[v2] srcu: avoid escaped section names
diff mbox series

Message ID 20200929192549.501516-1-ndesaulniers@google.com
State In Next
Commit fe0d06f03320f5e6169101ef4af98a6d1edd0ef3
Headers show
Series
  • [v2] srcu: avoid escaped section names
Related show

Commit Message

Nick Desaulniers Sept. 29, 2020, 7:25 p.m. UTC
The stringification operator, `#`, in the preprocessor escapes strings.
For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
they treat section names that contain \".

The portable solution is to not use a string literal with the
preprocessor stringification operator.

Link: https://bugs.llvm.org/show_bug.cgi?id=42950
Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1->V2:
* drop unrelated Kconfig changes accidentally committed in v1.

 include/linux/srcutree.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kees Cook Sept. 29, 2020, 11:45 p.m. UTC | #1
On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> The stringification operator, `#`, in the preprocessor escapes strings.
> For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> they treat section names that contain \".
> 
> The portable solution is to not use a string literal with the
> preprocessor stringification operator.
> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Ah, ignore my earlier question about also fixing this instance. Here it
is! ;)

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
> Changes V1->V2:
> * drop unrelated Kconfig changes accidentally committed in v1.
> 
>  include/linux/srcutree.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index 9cfcc8a756ae..9de652f4e1bd 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -124,7 +124,7 @@ struct srcu_struct {
>  # define __DEFINE_SRCU(name, is_static)					\
>  	is_static struct srcu_struct name;				\
>  	struct srcu_struct * const __srcu_struct_##name			\
> -		__section("___srcu_struct_ptrs") = &name
> +		__section(___srcu_struct_ptrs) = &name
>  #else
>  # define __DEFINE_SRCU(name, is_static)					\
>  	static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);	\
> -- 
> 2.28.0.709.gb0816b6eb0-goog
>
Nathan Chancellor Sept. 30, 2020, 4:27 p.m. UTC | #2
On Tue, Sep 29, 2020 at 12:25:49PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> The stringification operator, `#`, in the preprocessor escapes strings.
> For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> they treat section names that contain \".
> 
> The portable solution is to not use a string literal with the
> preprocessor stringification operator.
> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
> Changes V1->V2:
> * drop unrelated Kconfig changes accidentally committed in v1.
> 
>  include/linux/srcutree.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index 9cfcc8a756ae..9de652f4e1bd 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -124,7 +124,7 @@ struct srcu_struct {
>  # define __DEFINE_SRCU(name, is_static)					\
>  	is_static struct srcu_struct name;				\
>  	struct srcu_struct * const __srcu_struct_##name			\
> -		__section("___srcu_struct_ptrs") = &name
> +		__section(___srcu_struct_ptrs) = &name
>  #else
>  # define __DEFINE_SRCU(name, is_static)					\
>  	static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);	\
> -- 
> 2.28.0.709.gb0816b6eb0-goog
>
Sedat Dilek Sept. 30, 2020, 4:41 p.m. UTC | #3
On Tue, Sep 29, 2020 at 9:25 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> The stringification operator, `#`, in the preprocessor escapes strings.
> For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> they treat section names that contain \".
>
> The portable solution is to not use a string literal with the
> preprocessor stringification operator.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Puh, remember one year ago an overnight bisecting to catch the root
cause for "escaped section names".

The two patches I see - this here and "export.h: fix section name for
CONFIG_TRIM_UNUSED_KSYMS for Clang" were new cases?

Do we have a check-script to catch/avoid such cases (Joe Perches?)?

Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>

- Sedat -

> ---
> Changes V1->V2:
> * drop unrelated Kconfig changes accidentally committed in v1.
>
>  include/linux/srcutree.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index 9cfcc8a756ae..9de652f4e1bd 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -124,7 +124,7 @@ struct srcu_struct {
>  # define __DEFINE_SRCU(name, is_static)                                        \
>         is_static struct srcu_struct name;                              \
>         struct srcu_struct * const __srcu_struct_##name                 \
> -               __section("___srcu_struct_ptrs") = &name
> +               __section(___srcu_struct_ptrs) = &name
>  #else
>  # define __DEFINE_SRCU(name, is_static)                                        \
>         static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);      \
> --
> 2.28.0.709.gb0816b6eb0-goog
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200929192549.501516-1-ndesaulniers%40google.com.
Joe Perches Sept. 30, 2020, 6:57 p.m. UTC | #4
On Wed, 2020-09-30 at 18:41 +0200, Sedat Dilek wrote:
> On Tue, Sep 29, 2020 at 9:25 PM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> > The stringification operator, `#`, in the preprocessor escapes strings.
> > For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> > they treat section names that contain \".
> > 
> > The portable solution is to not use a string literal with the
> > preprocessor stringification operator.
> > 
> > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> Puh, remember one year ago an overnight bisecting to catch the root
> cause for "escaped section names".
> 
> The two patches I see - this here and "export.h: fix section name for
> CONFIG_TRIM_UNUSED_KSYMS for Clang" were new cases?
> 
> Do we have a check-script to catch/avoid such cases (Joe Perches?)?

Try the script that removes #S from #define __section(S)

https://lore.kernel.org/lkml/0e582a7f5144a33f465978d97701f9b3dcc377f3.camel@perches.com/
Paul E. McKenney Sept. 30, 2020, 8:40 p.m. UTC | #5
On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> The stringification operator, `#`, in the preprocessor escapes strings.
> For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> they treat section names that contain \".
> 
> The portable solution is to not use a string literal with the
> preprocessor stringification operator.
> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

I am guessing that this needs to go up with other patches.  If so:

Acked-by: Paul E. McKenney <paulmck@kernel.org>

If not, let me know and I will queue it.

							Thanx, Paul

> ---
> Changes V1->V2:
> * drop unrelated Kconfig changes accidentally committed in v1.
> 
>  include/linux/srcutree.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index 9cfcc8a756ae..9de652f4e1bd 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -124,7 +124,7 @@ struct srcu_struct {
>  # define __DEFINE_SRCU(name, is_static)					\
>  	is_static struct srcu_struct name;				\
>  	struct srcu_struct * const __srcu_struct_##name			\
> -		__section("___srcu_struct_ptrs") = &name
> +		__section(___srcu_struct_ptrs) = &name
>  #else
>  # define __DEFINE_SRCU(name, is_static)					\
>  	static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);	\
> -- 
> 2.28.0.709.gb0816b6eb0-goog
>
Nick Desaulniers Sept. 30, 2020, 8:55 p.m. UTC | #6
On Wed, Sep 30, 2020 at 1:40 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> > The stringification operator, `#`, in the preprocessor escapes strings.
> > For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> > they treat section names that contain \".
> >
> > The portable solution is to not use a string literal with the
> > preprocessor stringification operator.
> >
> > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> I am guessing that this needs to go up with other patches.  If so:
>
> Acked-by: Paul E. McKenney <paulmck@kernel.org>
>
> If not, let me know and I will queue it.

I could have bundled them up as a series.  I think you can pick it up,
and I'll owe you a beer?

>
>                                                         Thanx, Paul
>
> > ---
> > Changes V1->V2:
> > * drop unrelated Kconfig changes accidentally committed in v1.
> >
> >  include/linux/srcutree.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > index 9cfcc8a756ae..9de652f4e1bd 100644
> > --- a/include/linux/srcutree.h
> > +++ b/include/linux/srcutree.h
> > @@ -124,7 +124,7 @@ struct srcu_struct {
> >  # define __DEFINE_SRCU(name, is_static)                                      \
> >       is_static struct srcu_struct name;                              \
> >       struct srcu_struct * const __srcu_struct_##name                 \
> > -             __section("___srcu_struct_ptrs") = &name
> > +             __section(___srcu_struct_ptrs) = &name
> >  #else
> >  # define __DEFINE_SRCU(name, is_static)                                      \
> >       static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);      \
> > --
> > 2.28.0.709.gb0816b6eb0-goog
> >
Paul E. McKenney Oct. 2, 2020, 8:51 p.m. UTC | #7
On Wed, Sep 30, 2020 at 01:55:48PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 30, 2020 at 1:40 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> > > they treat section names that contain \".
> > >
> > > The portable solution is to not use a string literal with the
> > > preprocessor stringification operator.
> > >
> > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> >
> > I am guessing that this needs to go up with other patches.  If so:
> >
> > Acked-by: Paul E. McKenney <paulmck@kernel.org>
> >
> > If not, let me know and I will queue it.
> 
> I could have bundled them up as a series.  I think you can pick it up,
> and I'll owe you a beer?

It is queued, thank you!

When does it need to hit mainline?  (Your default is the v5.11 merge
window, that is, the one following the upcoming merge window.)


                                                        Thanx, Paul

> > > ---
> > > Changes V1->V2:
> > > * drop unrelated Kconfig changes accidentally committed in v1.
> > >
> > >  include/linux/srcutree.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > index 9cfcc8a756ae..9de652f4e1bd 100644
> > > --- a/include/linux/srcutree.h
> > > +++ b/include/linux/srcutree.h
> > > @@ -124,7 +124,7 @@ struct srcu_struct {
> > >  # define __DEFINE_SRCU(name, is_static)                                      \
> > >       is_static struct srcu_struct name;                              \
> > >       struct srcu_struct * const __srcu_struct_##name                 \
> > > -             __section("___srcu_struct_ptrs") = &name
> > > +             __section(___srcu_struct_ptrs) = &name
> > >  #else
> > >  # define __DEFINE_SRCU(name, is_static)                                      \
> > >       static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);      \
> > > --
> > > 2.28.0.709.gb0816b6eb0-goog
> > >
> 
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers
Nick Desaulniers Oct. 5, 2020, 6:29 p.m. UTC | #8
On Fri, Oct 2, 2020 at 1:51 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Wed, Sep 30, 2020 at 01:55:48PM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 30, 2020 at 1:40 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> > > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > > For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> > > > they treat section names that contain \".
> > > >
> > > > The portable solution is to not use a string literal with the
> > > > preprocessor stringification operator.
> > > >
> > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > > Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > >
> > > I am guessing that this needs to go up with other patches.  If so:
> > >
> > > Acked-by: Paul E. McKenney <paulmck@kernel.org>
> > >
> > > If not, let me know and I will queue it.
> >
> > I could have bundled them up as a series.  I think you can pick it up,
> > and I'll owe you a beer?
>
> It is queued, thank you!
>
> When does it need to hit mainline?  (Your default is the v5.11 merge
> window, that is, the one following the upcoming merge window.)

No rush, this patch wasn't blocking any known issue, just a cleanup
while I was in the neighborhood.  100 years ago, I was an Eagle scout.
Pretty sure there was a motto about "leaving things better than you
found them."  Thanks for help resolving the merge conflict reported in
-next related to it.

>
>
>                                                         Thanx, Paul
>
> > > > ---
> > > > Changes V1->V2:
> > > > * drop unrelated Kconfig changes accidentally committed in v1.
> > > >
> > > >  include/linux/srcutree.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > index 9cfcc8a756ae..9de652f4e1bd 100644
> > > > --- a/include/linux/srcutree.h
> > > > +++ b/include/linux/srcutree.h
> > > > @@ -124,7 +124,7 @@ struct srcu_struct {
> > > >  # define __DEFINE_SRCU(name, is_static)                                      \
> > > >       is_static struct srcu_struct name;                              \
> > > >       struct srcu_struct * const __srcu_struct_##name                 \
> > > > -             __section("___srcu_struct_ptrs") = &name
> > > > +             __section(___srcu_struct_ptrs) = &name
> > > >  #else
> > > >  # define __DEFINE_SRCU(name, is_static)                                      \
> > > >       static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);      \
> > > > --
> > > > 2.28.0.709.gb0816b6eb0-goog
> > > >
> >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
Sedat Dilek Oct. 5, 2020, 6:38 p.m. UTC | #9
On Mon, Oct 5, 2020 at 8:29 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Fri, Oct 2, 2020 at 1:51 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Wed, Sep 30, 2020 at 01:55:48PM -0700, Nick Desaulniers wrote:
> > > On Wed, Sep 30, 2020 at 1:40 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> > > > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > > > For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> > > > > they treat section names that contain \".
> > > > >
> > > > > The portable solution is to not use a string literal with the
> > > > > preprocessor stringification operator.
> > > > >
> > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > > > Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > >
> > > > I am guessing that this needs to go up with other patches.  If so:
> > > >
> > > > Acked-by: Paul E. McKenney <paulmck@kernel.org>
> > > >
> > > > If not, let me know and I will queue it.
> > >
> > > I could have bundled them up as a series.  I think you can pick it up,
> > > and I'll owe you a beer?
> >
> > It is queued, thank you!
> >
> > When does it need to hit mainline?  (Your default is the v5.11 merge
> > window, that is, the one following the upcoming merge window.)
>
> No rush, this patch wasn't blocking any known issue, just a cleanup
> while I was in the neighborhood.  100 years ago, I was an Eagle scout.
> Pretty sure there was a motto about "leaving things better than you
> found them."  Thanks for help resolving the merge conflict reported in
> -next related to it.
>

Wasn't there a problem with your "Fixes:" tag (Fixes: *drop word
"commit"* commit_hashid ("...")?

- Sedat -

> >
> >
> >                                                         Thanx, Paul
> >
> > > > > ---
> > > > > Changes V1->V2:
> > > > > * drop unrelated Kconfig changes accidentally committed in v1.
> > > > >
> > > > >  include/linux/srcutree.h | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > > index 9cfcc8a756ae..9de652f4e1bd 100644
> > > > > --- a/include/linux/srcutree.h
> > > > > +++ b/include/linux/srcutree.h
> > > > > @@ -124,7 +124,7 @@ struct srcu_struct {
> > > > >  # define __DEFINE_SRCU(name, is_static)                                      \
> > > > >       is_static struct srcu_struct name;                              \
> > > > >       struct srcu_struct * const __srcu_struct_##name                 \
> > > > > -             __section("___srcu_struct_ptrs") = &name
> > > > > +             __section(___srcu_struct_ptrs) = &name
> > > > >  #else
> > > > >  # define __DEFINE_SRCU(name, is_static)                                      \
> > > > >       static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);      \
> > > > > --
> > > > > 2.28.0.709.gb0816b6eb0-goog
> > > > >
> > >
> > >
> > >
> > > --
> > > Thanks,
> > > ~Nick Desaulniers
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdkPMSwQneMLFNg3ihM5zHorFy%2BuGvzAL7y70%2Bhu_1q24w%40mail.gmail.com.
Paul E. McKenney Oct. 5, 2020, 6:49 p.m. UTC | #10
On Mon, Oct 05, 2020 at 08:38:42PM +0200, Sedat Dilek wrote:
> On Mon, Oct 5, 2020 at 8:29 PM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > On Fri, Oct 2, 2020 at 1:51 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Wed, Sep 30, 2020 at 01:55:48PM -0700, Nick Desaulniers wrote:
> > > > On Wed, Sep 30, 2020 at 1:40 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > >
> > > > > On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> > > > > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > > > > For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> > > > > > they treat section names that contain \".
> > > > > >
> > > > > > The portable solution is to not use a string literal with the
> > > > > > preprocessor stringification operator.
> > > > > >
> > > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > > > > Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> > > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > >
> > > > > I am guessing that this needs to go up with other patches.  If so:
> > > > >
> > > > > Acked-by: Paul E. McKenney <paulmck@kernel.org>
> > > > >
> > > > > If not, let me know and I will queue it.
> > > >
> > > > I could have bundled them up as a series.  I think you can pick it up,
> > > > and I'll owe you a beer?
> > >
> > > It is queued, thank you!
> > >
> > > When does it need to hit mainline?  (Your default is the v5.11 merge
> > > window, that is, the one following the upcoming merge window.)
> >
> > No rush, this patch wasn't blocking any known issue, just a cleanup
> > while I was in the neighborhood.  100 years ago, I was an Eagle scout.
> > Pretty sure there was a motto about "leaving things better than you
> > found them."  Thanks for help resolving the merge conflict reported in
> > -next related to it.
> 
> Wasn't there a problem with your "Fixes:" tag (Fixes: *drop word
> "commit"* commit_hashid ("...")?

Indeed there was, and I have it noted to be fixed on my next rebase.

Perhaps another reason not to rush to mainline though.  ;-)

							Thanx, Paul

> - Sedat -
> 
> > >
> > >
> > >                                                         Thanx, Paul
> > >
> > > > > > ---
> > > > > > Changes V1->V2:
> > > > > > * drop unrelated Kconfig changes accidentally committed in v1.
> > > > > >
> > > > > >  include/linux/srcutree.h | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > > > index 9cfcc8a756ae..9de652f4e1bd 100644
> > > > > > --- a/include/linux/srcutree.h
> > > > > > +++ b/include/linux/srcutree.h
> > > > > > @@ -124,7 +124,7 @@ struct srcu_struct {
> > > > > >  # define __DEFINE_SRCU(name, is_static)                                      \
> > > > > >       is_static struct srcu_struct name;                              \
> > > > > >       struct srcu_struct * const __srcu_struct_##name                 \
> > > > > > -             __section("___srcu_struct_ptrs") = &name
> > > > > > +             __section(___srcu_struct_ptrs) = &name
> > > > > >  #else
> > > > > >  # define __DEFINE_SRCU(name, is_static)                                      \
> > > > > >       static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);      \
> > > > > > --
> > > > > > 2.28.0.709.gb0816b6eb0-goog
> > > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Thanks,
> > > > ~Nick Desaulniers
> >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdkPMSwQneMLFNg3ihM5zHorFy%2BuGvzAL7y70%2Bhu_1q24w%40mail.gmail.com.
Nathan Chancellor Oct. 6, 2020, 6:56 a.m. UTC | #11
On Mon, Oct 05, 2020 at 11:49:10AM -0700, Paul E. McKenney wrote:
> On Mon, Oct 05, 2020 at 08:38:42PM +0200, Sedat Dilek wrote:
> > On Mon, Oct 5, 2020 at 8:29 PM 'Nick Desaulniers' via Clang Built
> > Linux <clang-built-linux@googlegroups.com> wrote:
> > >
> > > On Fri, Oct 2, 2020 at 1:51 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > On Wed, Sep 30, 2020 at 01:55:48PM -0700, Nick Desaulniers wrote:
> > > > > On Wed, Sep 30, 2020 at 1:40 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> > > > > > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > > > > > For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> > > > > > > they treat section names that contain \".
> > > > > > >
> > > > > > > The portable solution is to not use a string literal with the
> > > > > > > preprocessor stringification operator.
> > > > > > >
> > > > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > > > > > Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> > > > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > > >
> > > > > > I am guessing that this needs to go up with other patches.  If so:
> > > > > >
> > > > > > Acked-by: Paul E. McKenney <paulmck@kernel.org>
> > > > > >
> > > > > > If not, let me know and I will queue it.
> > > > >
> > > > > I could have bundled them up as a series.  I think you can pick it up,
> > > > > and I'll owe you a beer?
> > > >
> > > > It is queued, thank you!
> > > >
> > > > When does it need to hit mainline?  (Your default is the v5.11 merge
> > > > window, that is, the one following the upcoming merge window.)
> > >
> > > No rush, this patch wasn't blocking any known issue, just a cleanup
> > > while I was in the neighborhood.  100 years ago, I was an Eagle scout.
> > > Pretty sure there was a motto about "leaving things better than you
> > > found them."  Thanks for help resolving the merge conflict reported in
> > > -next related to it.
> > 
> > Wasn't there a problem with your "Fixes:" tag (Fixes: *drop word
> > "commit"* commit_hashid ("...")?
> 
> Indeed there was, and I have it noted to be fixed on my next rebase.
> 
> Perhaps another reason not to rush to mainline though.  ;-)
> 
> 							Thanx, Paul

I am replying here as well so that the relevant parties are in the know
but I believe this patch should be fast tracked with a cc stable tag as
this appears to be the root cause of the issue that Nick reported a few
weeks ago:

https://lore.kernel.org/rcu/CAKwvOdm4AQhobdkKT08bjPGb15N58QN79XWxEaQt-P5Dk4+avQ@mail.gmail.com/
https://github.com/ClangBuiltLinux/linux/issues/1081

I can reproduce the issue on next-20201002 on my Raspberry Pi 4 just by
booting it up. As soon as I apply this patch, all warnings disappear. I
asked the original reporters to test if the patch resolves the issue for
them but I figured more visibility on this, the sooner. The commit
message might need to be revised if this turns out to be the case to
make it more apparent that it has a user visible issue, rather than just
a QoL fix.

Additionally, it seems like the patch is missing some reviewed by tags
from Kees, Sedat, and myself. Feel free to add a

Tested-by: Nathan Chancellor <natechancellor@gmail.com>

as well.

Cheers,
Nathan
Paul E. McKenney Oct. 7, 2020, 9:05 p.m. UTC | #12
On Mon, Oct 05, 2020 at 11:56:23PM -0700, Nathan Chancellor wrote:
> On Mon, Oct 05, 2020 at 11:49:10AM -0700, Paul E. McKenney wrote:
> > On Mon, Oct 05, 2020 at 08:38:42PM +0200, Sedat Dilek wrote:
> > > On Mon, Oct 5, 2020 at 8:29 PM 'Nick Desaulniers' via Clang Built
> > > Linux <clang-built-linux@googlegroups.com> wrote:
> > > >
> > > > On Fri, Oct 2, 2020 at 1:51 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > >
> > > > > On Wed, Sep 30, 2020 at 01:55:48PM -0700, Nick Desaulniers wrote:
> > > > > > On Wed, Sep 30, 2020 at 1:40 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > > > >
> > > > > > > On Tue, Sep 29, 2020 at 12:25:49PM -0700, Nick Desaulniers wrote:
> > > > > > > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > > > > > > For example, `# "foo"` becomes `"\"foo\""`.  GCC and Clang differ in how
> > > > > > > > they treat section names that contain \".
> > > > > > > >
> > > > > > > > The portable solution is to not use a string literal with the
> > > > > > > > preprocessor stringification operator.
> > > > > > > >
> > > > > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > > > > > > Fixes: commit fe15b50cdeee ("srcu: Allocate per-CPU data for DEFINE_SRCU() in modules")
> > > > > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > > > >
> > > > > > > I am guessing that this needs to go up with other patches.  If so:
> > > > > > >
> > > > > > > Acked-by: Paul E. McKenney <paulmck@kernel.org>
> > > > > > >
> > > > > > > If not, let me know and I will queue it.
> > > > > >
> > > > > > I could have bundled them up as a series.  I think you can pick it up,
> > > > > > and I'll owe you a beer?
> > > > >
> > > > > It is queued, thank you!
> > > > >
> > > > > When does it need to hit mainline?  (Your default is the v5.11 merge
> > > > > window, that is, the one following the upcoming merge window.)
> > > >
> > > > No rush, this patch wasn't blocking any known issue, just a cleanup
> > > > while I was in the neighborhood.  100 years ago, I was an Eagle scout.
> > > > Pretty sure there was a motto about "leaving things better than you
> > > > found them."  Thanks for help resolving the merge conflict reported in
> > > > -next related to it.
> > > 
> > > Wasn't there a problem with your "Fixes:" tag (Fixes: *drop word
> > > "commit"* commit_hashid ("...")?
> > 
> > Indeed there was, and I have it noted to be fixed on my next rebase.
> > 
> > Perhaps another reason not to rush to mainline though.  ;-)
> > 
> > 							Thanx, Paul
> 
> I am replying here as well so that the relevant parties are in the know
> but I believe this patch should be fast tracked with a cc stable tag as
> this appears to be the root cause of the issue that Nick reported a few
> weeks ago:
> 
> https://lore.kernel.org/rcu/CAKwvOdm4AQhobdkKT08bjPGb15N58QN79XWxEaQt-P5Dk4+avQ@mail.gmail.com/
> https://github.com/ClangBuiltLinux/linux/issues/1081
> 
> I can reproduce the issue on next-20201002 on my Raspberry Pi 4 just by
> booting it up. As soon as I apply this patch, all warnings disappear. I
> asked the original reporters to test if the patch resolves the issue for
> them but I figured more visibility on this, the sooner. The commit
> message might need to be revised if this turns out to be the case to
> make it more apparent that it has a user visible issue, rather than just
> a QoL fix.
> 
> Additionally, it seems like the patch is missing some reviewed by tags
> from Kees, Sedat, and myself. Feel free to add a
> 
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> 
> as well.

Good catch, and apologies to all concerned!  I have removed the stray
"commit" and, based posts earlier in this thread, I have also added:

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>

Thank you all!

							Thanx, Paul

Patch
diff mbox series

diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index 9cfcc8a756ae..9de652f4e1bd 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -124,7 +124,7 @@  struct srcu_struct {
 # define __DEFINE_SRCU(name, is_static)					\
 	is_static struct srcu_struct name;				\
 	struct srcu_struct * const __srcu_struct_##name			\
-		__section("___srcu_struct_ptrs") = &name
+		__section(___srcu_struct_ptrs) = &name
 #else
 # define __DEFINE_SRCU(name, is_static)					\
 	static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);	\