2022-08-21 12:04:54

by Conor Dooley

[permalink] [raw]
Subject: [PATCH 0/6] Add an asm-generic cpuinfo_op declaration

From: Conor Dooley <[email protected]>

RISC-V is missing a prototype for cpuinfo_op. Rather than adding yet
another `extern const struct seq_operations cpuinfo_op;` to an arch
specific header file, create an asm-generic variant and migrate the
existing arch variants there too. Obv. there are other archs that use
cpuinfo_op but don't declare it and surely also have the same warning?
I went for the minimum change here, but would be perfectly happy to
extend the change to all archs if this change is worthwhile. Or just
make a header in arch/riscv, any of the three work for me!

If this isn't the approach I should've gone for, any direction would
be great :) I tried pushing this last weekend to get LKP to test it but
I got neither a build success nor a build failure email from it, so
I figured I may as well just send the patches..

I wasn't too sure if this could be a single patch, so I split it out
into a patch fixing the issue on RISC-V & copy-paste patches for each
arch that I moved.

Thanks,
Conor.

Conor Dooley (6):
asm-generic: add a cpuinfo_ops definition in shared code
microblaze: use the asm-generic version of cpuinfo_op
s390: use the asm-generic version of cpuinfo_op
sh: use the asm-generic version of cpuinfo_op
sparc: use the asm-generic version of cpuinfo_op
x86: use the asm-generic version of cpuinfo_op

arch/microblaze/include/asm/processor.h | 2 +-
arch/riscv/include/asm/processor.h | 1 +
arch/s390/include/asm/processor.h | 2 +-
arch/sh/include/asm/processor.h | 2 +-
arch/sparc/include/asm/cpudata.h | 3 +--
arch/x86/include/asm/processor.h | 2 +-
include/asm-generic/processor.h | 7 +++++++
7 files changed, 13 insertions(+), 6 deletions(-)
create mode 100644 include/asm-generic/processor.h

--
2.37.1


2022-08-21 12:12:40

by Conor Dooley

[permalink] [raw]
Subject: [PATCH 1/6] asm-generic: add a cpuinfo_ops definition in shared code

From: Conor Dooley <[email protected]>

On RISC-V sparse complains that:
arch/riscv/kernel/cpu.c:204:29: warning: symbol 'cpuinfo_op' was not declared. Should it be static?

Sure, it could be dumped into asm/processor.h like other archs have
done, but putting it in an asm-generic header seems to be a saner
strategy.

Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code")
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/include/asm/processor.h | 1 +
include/asm-generic/processor.h | 7 +++++++
2 files changed, 8 insertions(+)
create mode 100644 include/asm-generic/processor.h

diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 19eedd4af4cd..dd2c9a382192 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -9,6 +9,7 @@
#include <linux/const.h>

#include <vdso/processor.h>
+#include <asm-generic/processor.h>

#include <asm/ptrace.h>

diff --git a/include/asm-generic/processor.h b/include/asm-generic/processor.h
new file mode 100644
index 000000000000..2ec9af562e9b
--- /dev/null
+++ b/include/asm-generic/processor.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_PROCESSOR_H
+#define __ASM_PROCESSOR_H
+
+extern const struct seq_operations cpuinfo_op;
+
+#endif /* __ASM_PROCESSOR_H */
--
2.37.1

2022-08-22 10:43:25

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add an asm-generic cpuinfo_op declaration

Hi Conor,

On Sun, Aug 21, 2022 at 1:36 PM Conor Dooley <[email protected]> wrote:
> From: Conor Dooley <[email protected]>
>
> RISC-V is missing a prototype for cpuinfo_op. Rather than adding yet
> another `extern const struct seq_operations cpuinfo_op;` to an arch
> specific header file, create an asm-generic variant and migrate the
> existing arch variants there too. Obv. there are other archs that use
> cpuinfo_op but don't declare it and surely also have the same warning?
> I went for the minimum change here, but would be perfectly happy to
> extend the change to all archs if this change is worthwhile. Or just
> make a header in arch/riscv, any of the three work for me!
>
> If this isn't the approach I should've gone for, any direction would
> be great :) I tried pushing this last weekend to get LKP to test it but
> I got neither a build success nor a build failure email from it, so
> I figured I may as well just send the patches..
>
> I wasn't too sure if this could be a single patch, so I split it out
> into a patch fixing the issue on RISC-V & copy-paste patches for each
> arch that I moved.

Thanks for your series!

> Conor Dooley (6):
> asm-generic: add a cpuinfo_ops definition in shared code
> microblaze: use the asm-generic version of cpuinfo_op
> s390: use the asm-generic version of cpuinfo_op
> sh: use the asm-generic version of cpuinfo_op
> sparc: use the asm-generic version of cpuinfo_op
> x86: use the asm-generic version of cpuinfo_op
>
> arch/microblaze/include/asm/processor.h | 2 +-
> arch/riscv/include/asm/processor.h | 1 +
> arch/s390/include/asm/processor.h | 2 +-
> arch/sh/include/asm/processor.h | 2 +-
> arch/sparc/include/asm/cpudata.h | 3 +--
> arch/x86/include/asm/processor.h | 2 +-
> include/asm-generic/processor.h | 7 +++++++
> 7 files changed, 13 insertions(+), 6 deletions(-)
> create mode 100644 include/asm-generic/processor.h

I was a bit surprised not to find fs/proc/cpuinfo.c in the diffstat
above. That file already has an external declaration for cpuinfo_op,
and uses it rather unconditionally (that is, if CONFIG_PROC_FS=y)
on all architectures.

So I think you can just move that to include/linux/processor.h, include
the latter everywhere, and drop all architecture-specific copies.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-08-22 11:05:06

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add an asm-generic cpuinfo_op declaration

Hi Conor,

On Mon, Aug 22, 2022 at 12:05 PM <[email protected]> wrote:
> On 22/08/2022 10:36, Geert Uytterhoeven wrote:
> > On Sun, Aug 21, 2022 at 1:36 PM Conor Dooley <[email protected]> wrote:
> >> arch/microblaze/include/asm/processor.h | 2 +-
> >> arch/riscv/include/asm/processor.h | 1 +
> >> arch/s390/include/asm/processor.h | 2 +-
> >> arch/sh/include/asm/processor.h | 2 +-
> >> arch/sparc/include/asm/cpudata.h | 3 +--
> >> arch/x86/include/asm/processor.h | 2 +-
> >> include/asm-generic/processor.h | 7 +++++++
> >> 7 files changed, 13 insertions(+), 6 deletions(-)
> >> create mode 100644 include/asm-generic/processor.h
> >
> > I was a bit surprised not to find fs/proc/cpuinfo.c in the diffstat
> > above. That file already has an external declaration for cpuinfo_op,
> > and uses it rather unconditionally (that is, if CONFIG_PROC_FS=y)
> > on all architectures.
> >
> > So I think you can just move that to include/linux/processor.h, include
> > the latter everywhere, and drop all architecture-specific copies.
>
> This is the sort of thing I was really hoping to hear, so fine by
> me.. When you say "everywhere", I assume you mean in every arch
> and not just the ones listed here that already have it in an arch
> specific header?

Yes, above every user, to silence the sparse "foo was not
declared. Should it be static?" warnings.

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-08-22 11:06:05

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add an asm-generic cpuinfo_op declaration

On 22/08/2022 10:36, Geert Uytterhoeven wrote:
> On Sun, Aug 21, 2022 at 1:36 PM Conor Dooley <[email protected]> wrote:
>> arch/microblaze/include/asm/processor.h | 2 +-
>> arch/riscv/include/asm/processor.h | 1 +
>> arch/s390/include/asm/processor.h | 2 +-
>> arch/sh/include/asm/processor.h | 2 +-
>> arch/sparc/include/asm/cpudata.h | 3 +--
>> arch/x86/include/asm/processor.h | 2 +-
>> include/asm-generic/processor.h | 7 +++++++
>> 7 files changed, 13 insertions(+), 6 deletions(-)
>> create mode 100644 include/asm-generic/processor.h
>
> I was a bit surprised not to find fs/proc/cpuinfo.c in the diffstat
> above. That file already has an external declaration for cpuinfo_op,
> and uses it rather unconditionally (that is, if CONFIG_PROC_FS=y)
> on all architectures.
>
> So I think you can just move that to include/linux/processor.h, include
> the latter everywhere, and drop all architecture-specific copies.

Hey Geert,
This is the sort of thing I was really hoping to hear, so fine by
me.. When you say "everywhere", I assume you mean in every arch
and not just the ones listed here that already have it in an arch
specific header?

Thanks,
Conor.