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/[email protected]/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
Reported-by: kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
fs/dlm/midcomms.c:213:1: sparse: sparse: symbol '__srcu_struct_nodes_srcu' was not declared. Should it be static?
fs/dlm/midcomms.c:570:25: sparse: sparse: cast to restricted __le32
fs/dlm/midcomms.c:678:19: sparse: sparse: cast to restricted __le16
fs/dlm/midcomms.c:680:16: sparse: sparse: cast to restricted __le16
fs/dlm/midcomms.c:718:27: sparse: sparse: cast to restricted __le16
fs/dlm/midcomms.c:737:25: sparse: sparse: cast to restricted __le32
fs/dlm/midcomms.c:747:25: sparse: sparse: cast to restricted __le32
fs/dlm/midcomms.c:756:23: sparse: sparse: cast to restricted __le32
fs/dlm/midcomms.c:766:42: sparse: sparse: cast to restricted __le16
fs/dlm/midcomms.c:769:26: sparse: sparse: cast to restricted __le16
fs/dlm/midcomms.c:804:23: sparse: sparse: cast to restricted __le32
fs/dlm/midcomms.c:838:27: sparse: sparse: cast to restricted __le16
fs/dlm/midcomms.c:898:26: sparse: sparse: cast to restricted __le16
fs/dlm/midcomms.c:920:25: sparse: sparse: cast to restricted __le32
>> fs/dlm/midcomms.c:913:22: sparse: sparse: restricted __le32 degrades to integer
fs/dlm/midcomms.c:916:22: sparse: sparse: restricted __le32 degrades to integer
fs/dlm/midcomms.c:1056:20: sparse: sparse: context imbalance in 'dlm_midcomms_get_mhandle' - wrong count at exit
fs/dlm/midcomms.c: note: in included file (through include/linux/notifier.h, arch/x86/include/asm/uprobes.h, include/linux/uprobes.h, ...):
include/linux/srcu.h:188:9: sparse: sparse: context imbalance in 'dlm_midcomms_commit_mhandle' - unexpected unlock
vim +913 fs/dlm/midcomms.c
871
872 /*
873 * Called from the low-level comms layer to process a buffer of
874 * commands.
875 */
876
877 int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int len)
878 {
879 const unsigned char *ptr = buf;
880 const struct dlm_header *hd;
881 uint16_t msglen;
882 int ret = 0;
883
884 while (len >= sizeof(struct dlm_header)) {
885 hd = (struct dlm_header *)ptr;
886
887 /* no message should be more than DLM_MAX_SOCKET_BUFSIZE or
888 * less than dlm_header size.
889 *
890 * Some messages does not have a 8 byte length boundary yet
891 * which can occur in a unaligned memory access of some dlm
892 * messages. However this problem need to be fixed at the
893 * sending side, for now it seems nobody run into architecture
894 * related issues yet but it slows down some processing.
895 * Fixing this issue should be scheduled in future by doing
896 * the next major version bump.
897 */
898 msglen = le16_to_cpu(hd->h_length);
899 if (msglen > DLM_MAX_SOCKET_BUFSIZE ||
900 msglen < sizeof(struct dlm_header)) {
901 log_print("received invalid length header: %u from node %d, will abort message parsing",
902 msglen, nodeid);
903 return -EBADMSG;
904 }
905
906 /* caller will take care that leftover
907 * will be parsed next call with more data
908 */
909 if (msglen > len)
910 break;
911
912 switch (hd->h_version) {
> 913 case cpu_to_le32(DLM_VERSION_3_1):
914 dlm_midcomms_receive_buffer_3_1((union dlm_packet *)ptr, nodeid);
915 break;
916 case cpu_to_le32(DLM_VERSION_3_2):
917 dlm_midcomms_receive_buffer_3_2((union dlm_packet *)ptr, nodeid);
918 break;
919 default:
920 log_print("received invalid version header: %u from node %d, will skip this message",
921 le32_to_cpu(hd->h_version), nodeid);
922 break;
923 }
924
925 ret += msglen;
926 len -= msglen;
927 ptr += msglen;
928 }
929
930 return ret;
931 }
932
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi,
On Fri, Jan 21, 2022 at 9:45 PM kernel test robot <[email protected]> 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/[email protected]/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?
Thanks.
- Alex
On 1/22/22 10:28, Alexander Aring wrote:
> Hi,
>
> On Fri, Jan 21, 2022 at 9:45 PM kernel test robot <[email protected]> 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/[email protected]/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.
kernel test robot might have a different way to disable this test
for a specific source file. I dunno.
--
~Randy
Hi Randy,
On Sat, Jan 22, 2022 at 2:19 PM Randy Dunlap <[email protected]> wrote:
>
>
>
> On 1/22/22 10:28, Alexander Aring wrote:
> > Hi,
> >
> > On Fri, Jan 21, 2022 at 9:45 PM kernel test robot <[email protected]> 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/[email protected]/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 [email protected] and [email protected], maybe
this can be discussed?
- Alex
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) \
Hi,
On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <[email protected]> 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
Hi,
On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <[email protected]> wrote:
>
> Hi,
>
> On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <[email protected]> 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
Hi,
On Mon, Jan 24, 2022 at 12:36 PM Alexander Aring <[email protected]> wrote:
>
> Hi,
>
> On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <[email protected]> wrote:
> >
> > Hi,
> >
> > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <[email protected]> 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
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 <[email protected]> wrote:
> >
> > Hi,
> >
> > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <[email protected]> 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
Hi,
On Mon, Jan 24, 2022 at 3:23 PM Paul E. McKenney <[email protected]> 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 <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <[email protected]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <[email protected]> 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
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 <[email protected]> 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 <[email protected]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <[email protected]> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <[email protected]> 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
Hi,
On Mon, Jan 24, 2022 at 5:14 PM Paul E. McKenney <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <[email protected]> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <[email protected]> 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
Hi,
On Tue, Jan 25, 2022 at 5:35 PM Alexander Aring <[email protected]> wrote:
>
> Hi,
>
> On Mon, Jan 24, 2022 at 5:14 PM Paul E. McKenney <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, Jan 24, 2022 at 12:21 PM Alexander Aring <[email protected]> wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Mon, Jan 24, 2022 at 7:46 AM Dan Carpenter <[email protected]> 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
On Sat, Jan 22, 2022 at 01:28:20PM -0500, Alexander Aring wrote:
> Hi,
>
> On Fri, Jan 21, 2022 at 9:45 PM kernel test robot <[email protected]> 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/[email protected]/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?
Frankly, these "convert in place" functions (dlm_message_in(), etc.) are
asking for trouble. IOW, it's a genuinely fishy code.
Saner approach is to keep them in little-endian through the entire thing,
but it's quite a bit of massage.
Hi,
On Mon, Feb 14, 2022 at 12:33 AM Al Viro <[email protected]> wrote:
>
> On Sat, Jan 22, 2022 at 01:28:20PM -0500, Alexander Aring wrote:
> > Hi,
> >
> > On Fri, Jan 21, 2022 at 9:45 PM kernel test robot <[email protected]> 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/[email protected]/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?
>
> Frankly, these "convert in place" functions (dlm_message_in(), etc.) are
> asking for trouble. IOW, it's a genuinely fishy code.
>
> Saner approach is to keep them in little-endian through the entire thing,
> but it's quite a bit of massage.
Yes, I agree. That is also what "/net" does, "mark it stored" the way
as the wire handles it.
- Alex