Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Alexander Aring]

Hi Randy,

On Sat, Jan 22, 2022 at 2:19 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
>
>
> On 1/22/22 10:28, Alexander Aring wrote:
> > Hi,
> >
> > On Fri, Jan 21, 2022 at 9:45 PM kernel test robot <lkp@intel.com> wrote:
> >>
> >> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >> head:   9b57f458985742bd1c585f4c7f36d04634ce1143
> >> commit: 658bd576f95ed597e519cdadf1c86ac87c17aea5 fs: dlm: move version conversion to compile time
> >> date:   3 months ago
> >> config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220122/202201221028.YKA8kSdm-lkp@intel.com/config)
> >> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> >> reproduce:
> >>         # apt-get install sparse
> >>         # sparse version: v0.6.4-dirty
> >>         # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=658bd576f95ed597e519cdadf1c86ac87c17aea5
> >>         git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >>         git fetch --no-tags linus master
> >>         git checkout 658bd576f95ed597e519cdadf1c86ac87c17aea5
> >>         # save the config file to linux build tree
> >>         mkdir build_dir
> >>         make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash
> >>
> >> If you fix the issue, kindly add following tag as appropriate
> >
> > I have it on my list but it isn't easy to make sparse happy here...
> > this is the second time the robot reported this issue. Is there a way
> > to turn the robot off in that case?
> >
> > Maybe some human who reads that knows the answer?
>
> Hi Alex,
>
> In the source file, you can surround the "offending" line(s) with
>
> #ifndef __CHECKER__
>  ...
> #endif
>
> to prevent sparse from checking those lines.
> Sometimes there needs to be an "#else" branch also.
>
> 'git grep __CHECKER__' will show you quite a few other examples.
>

Thanks, I will try that to stop the false positives and endianness
warnings (temporarily).

I see also:

fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
'__srcu_struct_nodes_srcu' was not declared. Should it be static?

For a macro which contains the "static" keyword, it's
"DEFINE_STATIC_SRCU()". Either sparse should be teached to deal with
that or everybody should use "static DEFINE_SRCU()" instead of using
"DEFINE_STATIC_SRCU()".
I would like to see the second way because everybody can see "static"
is being used, that means maybe we should remove
"DEFINE_STATIC_SRCU()"? The alternative by doing nothing would be to
use "#ifndef __CHECKER__".

I cc here rcu@vger.kernel.org and linux-sparse@vger.kernel.org, maybe
this can be discussed?

- Alex

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Dan Carpenter]

On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> 
> I see also:
> 
> fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> 

Why not just do this?  (Untested.  Maybe I don't understand?)

diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index cb1f4351e8ba..a164089abec4 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -121,7 +121,7 @@ struct srcu_struct {
 #ifdef MODULE
 # define __DEFINE_SRCU(name, is_static)					\
 	is_static struct srcu_struct name;				\
-	struct srcu_struct * const __srcu_struct_##name			\
+	is_static struct srcu_struct * const __srcu_struct_##name	\
 		__section("___srcu_struct_ptrs") = &name
 #else
 # define __DEFINE_SRCU(name, is_static)					\

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Alexander Aring]

Hi,

On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> >
> > I see also:
> >
> > fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> > '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> >
>
> Why not just do this?  (Untested.  Maybe I don't understand?)
>
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index cb1f4351e8ba..a164089abec4 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -121,7 +121,7 @@ struct srcu_struct {
>  #ifdef MODULE
>  # define __DEFINE_SRCU(name, is_static)                                        \
>         is_static struct srcu_struct name;                              \
> -       struct srcu_struct * const __srcu_struct_##name                 \
> +       is_static struct srcu_struct * const __srcu_struct_##name       \
>                 __section("___srcu_struct_ptrs") = &name
>  #else
>  # define __DEFINE_SRCU(name, is_static)                                        \
>

I tried it and yes it will fix the issue and introduce another one
about "is_static struct srcu_struct * const __srcu_struct_##name" is
unused ("-Wunused-const-variable").
I added a __maybe_unused after the introduced is_static and it seems
to fix the introduced issue, now it compiles and sparse is happy. I am
not sure if this is the right fix?

- Alex

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Alexander Aring]

Hi,

On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> > >
> > > I see also:
> > >
> > > fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> > > '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> > >
> >
> > Why not just do this?  (Untested.  Maybe I don't understand?)
> >
> > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > index cb1f4351e8ba..a164089abec4 100644
> > --- a/include/linux/srcutree.h
> > +++ b/include/linux/srcutree.h
> > @@ -121,7 +121,7 @@ struct srcu_struct {
> >  #ifdef MODULE
> >  # define __DEFINE_SRCU(name, is_static)                                        \
> >         is_static struct srcu_struct name;                              \
> > -       struct srcu_struct * const __srcu_struct_##name                 \
> > +       is_static struct srcu_struct * const __srcu_struct_##name       \
> >                 __section("___srcu_struct_ptrs") = &name
> >  #else
> >  # define __DEFINE_SRCU(name, is_static)                                        \
> >
>
> I tried it and yes it will fix the issue and introduce another one
> about "is_static struct srcu_struct * const __srcu_struct_##name" is
> unused ("-Wunused-const-variable").
> I added a __maybe_unused after the introduced is_static and it seems
> to fix the introduced issue, now it compiles and sparse is happy. I am
> not sure if this is the right fix?

it is obviously unused, but it has something to do with
"__section("___srcu_struct_ptrs")" and during module loading it, I
suppose, srcu tries to access it to find whatever needs to be
registered?

- Alex

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Alexander Aring]

Hi,

On Mon, Jan 24, 2022 at 12:36 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <aahringo@redhat.com> wrote:
> >
> > Hi,
> >
> > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > >
> > > On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> > > >
> > > > I see also:
> > > >
> > > > fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> > > > '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> > > >
> > >
> > > Why not just do this?  (Untested.  Maybe I don't understand?)
> > >
> > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > index cb1f4351e8ba..a164089abec4 100644
> > > --- a/include/linux/srcutree.h
> > > +++ b/include/linux/srcutree.h
> > > @@ -121,7 +121,7 @@ struct srcu_struct {
> > >  #ifdef MODULE
> > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > >         is_static struct srcu_struct name;                              \
> > > -       struct srcu_struct * const __srcu_struct_##name                 \
> > > +       is_static struct srcu_struct * const __srcu_struct_##name       \
> > >                 __section("___srcu_struct_ptrs") = &name
> > >  #else
> > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > >
> >
> > I tried it and yes it will fix the issue and introduce another one
> > about "is_static struct srcu_struct * const __srcu_struct_##name" is
> > unused ("-Wunused-const-variable").
> > I added a __maybe_unused after the introduced is_static and it seems
> > to fix the introduced issue, now it compiles and sparse is happy. I am
> > not sure if this is the right fix?
>
> it is obviously unused, but it has something to do with
> "__section("___srcu_struct_ptrs")" and during module loading it, I
> suppose, srcu tries to access it to find whatever needs to be
> registered?

Sorry, but if this is true then it can't be declared as static... and
we are at the beginning again.

- Alex

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Paul E. McKenney]

On Mon, Jan 24, 2022 at 12:41:04PM -0500, Alexander Aring wrote:
> Hi,
> 
> On Mon, Jan 24, 2022 at 12:36 PM Alexander Aring <aahringo@redhat.com> wrote:
> >
> > Hi,
> >
> > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <aahringo@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > >
> > > > On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> > > > >
> > > > > I see also:
> > > > >
> > > > > fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> > > > > '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> > > > >
> > > >
> > > > Why not just do this?  (Untested.  Maybe I don't understand?)
> > > >
> > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > index cb1f4351e8ba..a164089abec4 100644
> > > > --- a/include/linux/srcutree.h
> > > > +++ b/include/linux/srcutree.h
> > > > @@ -121,7 +121,7 @@ struct srcu_struct {
> > > >  #ifdef MODULE
> > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > >         is_static struct srcu_struct name;                              \
> > > > -       struct srcu_struct * const __srcu_struct_##name                 \
> > > > +       is_static struct srcu_struct * const __srcu_struct_##name       \
> > > >                 __section("___srcu_struct_ptrs") = &name
> > > >  #else
> > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > >
> > >
> > > I tried it and yes it will fix the issue and introduce another one
> > > about "is_static struct srcu_struct * const __srcu_struct_##name" is
> > > unused ("-Wunused-const-variable").
> > > I added a __maybe_unused after the introduced is_static and it seems
> > > to fix the introduced issue, now it compiles and sparse is happy. I am
> > > not sure if this is the right fix?
> >
> > it is obviously unused, but it has something to do with
> > "__section("___srcu_struct_ptrs")" and during module loading it, I
> > suppose, srcu tries to access it to find whatever needs to be
> > registered?
> 
> Sorry, but if this is true then it can't be declared as static... and
> we are at the beginning again.

Welcome to my world!!!  ;-)

More seriously, thank you for chasing this down.  But would it work to
add a declaration just before?

							Thanx, Paul

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Alexander Aring]

Hi,

On Mon, Jan 24, 2022 at 3:23 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Mon, Jan 24, 2022 at 12:41:04PM -0500, Alexander Aring wrote:
> > Hi,
> >
> > On Mon, Jan 24, 2022 at 12:36 PM Alexander Aring <aahringo@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <aahringo@redhat.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > > >
> > > > > On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> > > > > >
> > > > > > I see also:
> > > > > >
> > > > > > fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> > > > > > '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> > > > > >
> > > > >
> > > > > Why not just do this?  (Untested.  Maybe I don't understand?)
> > > > >
> > > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > > index cb1f4351e8ba..a164089abec4 100644
> > > > > --- a/include/linux/srcutree.h
> > > > > +++ b/include/linux/srcutree.h
> > > > > @@ -121,7 +121,7 @@ struct srcu_struct {
> > > > >  #ifdef MODULE
> > > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > > >         is_static struct srcu_struct name;                              \
> > > > > -       struct srcu_struct * const __srcu_struct_##name                 \
> > > > > +       is_static struct srcu_struct * const __srcu_struct_##name       \
> > > > >                 __section("___srcu_struct_ptrs") = &name
> > > > >  #else
> > > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > > >
> > > >
> > > > I tried it and yes it will fix the issue and introduce another one
> > > > about "is_static struct srcu_struct * const __srcu_struct_##name" is
> > > > unused ("-Wunused-const-variable").
> > > > I added a __maybe_unused after the introduced is_static and it seems
> > > > to fix the introduced issue, now it compiles and sparse is happy. I am
> > > > not sure if this is the right fix?
> > >
> > > it is obviously unused, but it has something to do with
> > > "__section("___srcu_struct_ptrs")" and during module loading it, I
> > > suppose, srcu tries to access it to find whatever needs to be
> > > registered?
> >
> > Sorry, but if this is true then it can't be declared as static... and
> > we are at the beginning again.
>
> Welcome to my world!!!  ;-)
>
> More seriously, thank you for chasing this down.  But would it work to
> add a declaration just before?
>

only if I add an "extern" in front of the declaration before, so it looks like:

extern struct srcu_struct * const __srcu_struct_##name;

(compile and sparse tested only)

- Alex

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Paul E. McKenney]

On Mon, Jan 24, 2022 at 04:36:55PM -0500, Alexander Aring wrote:
> Hi,
> 
> On Mon, Jan 24, 2022 at 3:23 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Mon, Jan 24, 2022 at 12:41:04PM -0500, Alexander Aring wrote:
> > > Hi,
> > >
> > > On Mon, Jan 24, 2022 at 12:36 PM Alexander Aring <aahringo@redhat.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <aahringo@redhat.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > > > >
> > > > > > On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> > > > > > >
> > > > > > > I see also:
> > > > > > >
> > > > > > > fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> > > > > > > '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> > > > > > >
> > > > > >
> > > > > > Why not just do this?  (Untested.  Maybe I don't understand?)
> > > > > >
> > > > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > > > index cb1f4351e8ba..a164089abec4 100644
> > > > > > --- a/include/linux/srcutree.h
> > > > > > +++ b/include/linux/srcutree.h
> > > > > > @@ -121,7 +121,7 @@ struct srcu_struct {
> > > > > >  #ifdef MODULE
> > > > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > > > >         is_static struct srcu_struct name;                              \
> > > > > > -       struct srcu_struct * const __srcu_struct_##name                 \
> > > > > > +       is_static struct srcu_struct * const __srcu_struct_##name       \
> > > > > >                 __section("___srcu_struct_ptrs") = &name
> > > > > >  #else
> > > > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > > > >
> > > > >
> > > > > I tried it and yes it will fix the issue and introduce another one
> > > > > about "is_static struct srcu_struct * const __srcu_struct_##name" is
> > > > > unused ("-Wunused-const-variable").
> > > > > I added a __maybe_unused after the introduced is_static and it seems
> > > > > to fix the introduced issue, now it compiles and sparse is happy. I am
> > > > > not sure if this is the right fix?
> > > >
> > > > it is obviously unused, but it has something to do with
> > > > "__section("___srcu_struct_ptrs")" and during module loading it, I
> > > > suppose, srcu tries to access it to find whatever needs to be
> > > > registered?
> > >
> > > Sorry, but if this is true then it can't be declared as static... and
> > > we are at the beginning again.
> >
> > Welcome to my world!!!  ;-)
> >
> > More seriously, thank you for chasing this down.  But would it work to
> > add a declaration just before?
> >
> 
> only if I add an "extern" in front of the declaration before, so it looks like:
> 
> extern struct srcu_struct * const __srcu_struct_##name;
> 
> (compile and sparse tested only)

If that works for everyone, it seems worth persuing.

One way to test this is as follows:

1.	Build a kernel with CONFIG_RCU_TORTURE_TEST=m.  Boot this and
	type "modprobe rcutorture torture_type=srcu".

	If you want to stop the torture test, type "rmmod rcutorture".

	This will test DEFINE_SRCU() for the module case.

2.	At the top-level directory of your Linux-kernel source tree,
	type "tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 3 --configs 'SRCU-N' --trust-make"

	This will test DEFINE_SRCU() for the non-module case.

If those both work, and if there are no other objections or complications,
please do send a patch.

							Thanx, Paul

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Alexander Aring]

Hi,

On Mon, Jan 24, 2022 at 5:14 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Mon, Jan 24, 2022 at 04:36:55PM -0500, Alexander Aring wrote:
> > Hi,
> >
> > On Mon, Jan 24, 2022 at 3:23 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Mon, Jan 24, 2022 at 12:41:04PM -0500, Alexander Aring wrote:
> > > > Hi,
> > > >
> > > > On Mon, Jan 24, 2022 at 12:36 PM Alexander Aring <aahringo@redhat.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <aahringo@redhat.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > > > > >
> > > > > > > On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> > > > > > > >
> > > > > > > > I see also:
> > > > > > > >
> > > > > > > > fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> > > > > > > > '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> > > > > > > >
> > > > > > >
> > > > > > > Why not just do this?  (Untested.  Maybe I don't understand?)
> > > > > > >
> > > > > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > > > > index cb1f4351e8ba..a164089abec4 100644
> > > > > > > --- a/include/linux/srcutree.h
> > > > > > > +++ b/include/linux/srcutree.h
> > > > > > > @@ -121,7 +121,7 @@ struct srcu_struct {
> > > > > > >  #ifdef MODULE
> > > > > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > > > > >         is_static struct srcu_struct name;                              \
> > > > > > > -       struct srcu_struct * const __srcu_struct_##name                 \
> > > > > > > +       is_static struct srcu_struct * const __srcu_struct_##name       \
> > > > > > >                 __section("___srcu_struct_ptrs") = &name
> > > > > > >  #else
> > > > > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > > > > >
> > > > > >
> > > > > > I tried it and yes it will fix the issue and introduce another one
> > > > > > about "is_static struct srcu_struct * const __srcu_struct_##name" is
> > > > > > unused ("-Wunused-const-variable").
> > > > > > I added a __maybe_unused after the introduced is_static and it seems
> > > > > > to fix the introduced issue, now it compiles and sparse is happy. I am
> > > > > > not sure if this is the right fix?
> > > > >
> > > > > it is obviously unused, but it has something to do with
> > > > > "__section("___srcu_struct_ptrs")" and during module loading it, I
> > > > > suppose, srcu tries to access it to find whatever needs to be
> > > > > registered?
> > > >
> > > > Sorry, but if this is true then it can't be declared as static... and
> > > > we are at the beginning again.
> > >
> > > Welcome to my world!!!  ;-)
> > >
> > > More seriously, thank you for chasing this down.  But would it work to
> > > add a declaration just before?
> > >
> >
> > only if I add an "extern" in front of the declaration before, so it looks like:
> >
> > extern struct srcu_struct * const __srcu_struct_##name;
> >
> > (compile and sparse tested only)
>
> If that works for everyone, it seems worth persuing.
>
> One way to test this is as follows:
>
> 1.      Build a kernel with CONFIG_RCU_TORTURE_TEST=m.  Boot this and
>         type "modprobe rcutorture torture_type=srcu".
>
>         If you want to stop the torture test, type "rmmod rcutorture".
>
>         This will test DEFINE_SRCU() for the module case.
>

I tested this case, I still need to do the 2. case. Sorry I am quite
busy with something else, but I am still working on it.

Thanks.

- Alex

Re: fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer

[Alexander Aring]

Hi,

On Tue, Jan 25, 2022 at 5:35 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Mon, Jan 24, 2022 at 5:14 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Mon, Jan 24, 2022 at 04:36:55PM -0500, Alexander Aring wrote:
> > > Hi,
> > >
> > > On Mon, Jan 24, 2022 at 3:23 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > On Mon, Jan 24, 2022 at 12:41:04PM -0500, Alexander Aring wrote:
> > > > > Hi,
> > > > >
> > > > > On Mon, Jan 24, 2022 at 12:36 PM Alexander Aring <aahringo@redhat.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <aahringo@redhat.com> wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > > > > > >
> > > > > > > > On Sun, Jan 23, 2022 at 01:41:52PM -0500, Alexander Aring wrote:
> > > > > > > > >
> > > > > > > > > I see also:
> > > > > > > > >
> > > > > > > > > fs/dlm/midcomms.c:213:1: sparse: sparse: symbol
> > > > > > > > > '__srcu_struct_nodes_srcu' was not declared. Should it be static?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Why not just do this?  (Untested.  Maybe I don't understand?)
> > > > > > > >
> > > > > > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > > > > > index cb1f4351e8ba..a164089abec4 100644
> > > > > > > > --- a/include/linux/srcutree.h
> > > > > > > > +++ b/include/linux/srcutree.h
> > > > > > > > @@ -121,7 +121,7 @@ struct srcu_struct {
> > > > > > > >  #ifdef MODULE
> > > > > > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > > > > > >         is_static struct srcu_struct name;                              \
> > > > > > > > -       struct srcu_struct * const __srcu_struct_##name                 \
> > > > > > > > +       is_static struct srcu_struct * const __srcu_struct_##name       \
> > > > > > > >                 __section("___srcu_struct_ptrs") = &name
> > > > > > > >  #else
> > > > > > > >  # define __DEFINE_SRCU(name, is_static)                                        \
> > > > > > > >
> > > > > > >
> > > > > > > I tried it and yes it will fix the issue and introduce another one
> > > > > > > about "is_static struct srcu_struct * const __srcu_struct_##name" is
> > > > > > > unused ("-Wunused-const-variable").
> > > > > > > I added a __maybe_unused after the introduced is_static and it seems
> > > > > > > to fix the introduced issue, now it compiles and sparse is happy. I am
> > > > > > > not sure if this is the right fix?
> > > > > >
> > > > > > it is obviously unused, but it has something to do with
> > > > > > "__section("___srcu_struct_ptrs")" and during module loading it, I
> > > > > > suppose, srcu tries to access it to find whatever needs to be
> > > > > > registered?
> > > > >
> > > > > Sorry, but if this is true then it can't be declared as static... and
> > > > > we are at the beginning again.
> > > >
> > > > Welcome to my world!!!  ;-)
> > > >
> > > > More seriously, thank you for chasing this down.  But would it work to
> > > > add a declaration just before?
> > > >
> > >
> > > only if I add an "extern" in front of the declaration before, so it looks like:
> > >
> > > extern struct srcu_struct * const __srcu_struct_##name;
> > >
> > > (compile and sparse tested only)
> >
> > If that works for everyone, it seems worth persuing.
> >
> > One way to test this is as follows:
> >
> > 1.      Build a kernel with CONFIG_RCU_TORTURE_TEST=m.  Boot this and
> >         type "modprobe rcutorture torture_type=srcu".
> >
> >         If you want to stop the torture test, type "rmmod rcutorture".
> >
> >         This will test DEFINE_SRCU() for the module case.
> >
>
> I tested this case, I still need to do the 2. case. Sorry I am quite
> busy with something else, but I am still working on it.
>

I did the 2. test successful...:
"tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 3
--configs 'SRCU-N' --trust-make"
but I was required to hack it because my "qemu-system-x86_64" did not
exist, it was "/usr/libexec/qemu-kvm" (it was able to run by just
doing a symlink). I think I already cc'd the right people to report
this issue (rcu subsystem)...

I will send a patch for the export declaration soon as possible.

- Alex