Messages in this thread | | | From | Nick Desaulniers <> | Date | Fri, 9 Aug 2019 16:04:03 -0700 | Subject | Re: checkpatch.pl should suggest __section |
| |
On Fri, Aug 9, 2019 at 3:58 PM Joe Perches <joe@perches.com> wrote: > > On Fri, 2019-08-09 at 15:21 -0700, Nick Desaulniers wrote: > > Hi Joe, > > While debugging: > > https://github.com/ClangBuiltLinux/linux/issues/619 > > we found a bunch of places where __section is not used but could be, > > and uses a string literal when it probably should not be. > > > > Just a thought that maybe checkpatch.pl could warn if > > `__attribute__((section` appeared in the added diff, and suggest > > __section? Then further warn to not use `""` for the section name? > > Hmm, that makes me wonder about the existing __section uses > _with_ a quote are actually in the proper sections. > > $ git grep -n -P '\b__section\s*\(\s*"' > arch/arm64/kernel/smp_spin_table.c:22:volatile unsigned long __section(".mmuoff.data.read") > arch/s390/boot/startup.c:49:static struct diag210 _diag210_tmp_dma __section(".dma.data"); > include/linux/compiler.h:27: __section("_ftrace_annotated_branch") \ > include/linux/compiler.h:63: __section("_ftrace_branch") \ > include/linux/compiler.h:121:#define __annotate_jump_table __section(".rodata..c_jump_table") > include/linux/compiler.h:158: __section("___kentry" "+" #sym ) \ > include/linux/compiler.h:301: static void * __section(".discard.addressable") __used \ > include/linux/export.h:107: static int __ksym_marker_##sym[0] __section(".discard.ksym") __used > include/linux/srcutree.h:127: __section("___srcu_struct_ptrs") = &name
I'm going through and fixing all of these now. Thinking about sending one treewide fix to akpm.
> > Maybe there should also be a __section("<foo>") test too.
I think so. Some of the trickier ones are ones that use the stringification C preprocessor operator. I need to think more about these.
> > Anyway, how about: > --- > scripts/checkpatch.pl | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 1cdacb4fd207..8e6693ca772c 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -5901,6 +5901,15 @@ sub process { > "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr); > } > > +# Check for __attribute__ section, prefer __section (without quotes) > + if ($realfile !~ m@\binclude/uapi/@ && > + $line =~ /\b__attribute__\s*\(\s*\(.*_*section_*\s*\(\s*("[^"]*")/) { > + my $old = substr($rawline, $-[1], $+[1] - $-[1]); > + my $new = substr($old, 1, -1); > + WARN("PREFER_SECTION", > + "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr); > + } > +
I can't read Perl, but this looks pretty good. Acked-by: Nick Desaulniers <ndesaulniers@google.com> -- Thanks, ~Nick Desaulniers
| |