[PATCH 1/2] Don't let LATENCYTOP and LOCKDEP select KALLSYMS_ALL

[PATCH 1/2] Don't let LATENCYTOP and LOCKDEP select KALLSYMS_ALL

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

KALLSYMS_ALL enables including data variables into KALLSYMS.
With plain KALLSYMS only functions are included.

LATENCYTOP and LOCKDEP select KALLSYMS_ALL in addition to KALLSYMS.
It's unclear what they actually need _ALL for; they should
only need function backtraces and afaik never touch variables.

LTO currently does not support KALLSYMS_ALL, which prevents
LATENCYTOP and LOCKDEP from working and gives Kconfig errors.
Disable the requirement for KALLSYMS_ALL for them, just use
KALLSYMS.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 lib/Kconfig.debug | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 3e0289e..062165c 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1001,7 +1001,6 @@ config LOCKDEP
 	select STACKTRACE
 	select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !SCORE
 	select KALLSYMS
-	select KALLSYMS_ALL
 
 config LOCK_STAT
 	bool "Lock usage statistics"
@@ -1469,7 +1468,6 @@ config LATENCYTOP
 	depends on PROC_FS
 	select FRAME_POINTER if !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM_UNWIND && !ARC
 	select KALLSYMS
-	select KALLSYMS_ALL
 	select STACKTRACE
 	select SCHEDSTATS
 	select SCHED_DEBUG
-- 
2.3.3

[PATCH 2/2] test-hexdump.c: Fix initconst confusion

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

const char *...[]  is not const, but an array of pointer to const.
So these arrays cannot be __initconst, but must be __initdata

This fixes section conflicts with LTO.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 lib/test-hexdump.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/test-hexdump.c b/lib/test-hexdump.c
index daf29a39..9846ff7 100644
--- a/lib/test-hexdump.c
+++ b/lib/test-hexdump.c
@@ -18,26 +18,26 @@ static const unsigned char data_b[] = {
 
 static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
 
-static const char *test_data_1_le[] __initconst = {
+static const char * const test_data_1_le[] __initconst = {
 	"be", "32", "db", "7b", "0a", "18", "93", "b2",
 	"70", "ba", "c4", "24", "7d", "83", "34", "9b",
 	"a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
 	"4c", "d1", "19", "99", "43", "b1", "af", "0c",
 };
 
-static const char *test_data_2_le[] __initconst = {
+static const char *test_data_2_le[] __initdata = {
 	"32be", "7bdb", "180a", "b293",
 	"ba70", "24c4", "837d", "9b34",
 	"9ca6", "ad31", "0f9c", "e9ac",
 	"d14c", "9919", "b143", "0caf",
 };
 
-static const char *test_data_4_le[] __initconst = {
+static const char *test_data_4_le[] __initdata = {
 	"7bdb32be", "b293180a", "24c4ba70", "9b34837d",
 	"ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
 };
 
-static const char *test_data_8_le[] __initconst = {
+static const char *test_data_8_le[] __initdata = {
 	"b293180a7bdb32be", "9b34837d24c4ba70",
 	"e9ac0f9cad319ca6", "0cafb1439919d14c",
 };
-- 
2.3.3

Re: [PATCH 2/2] test-hexdump.c: Fix initconst confusion

[Andrew Morton]

On Wed,  8 Apr 2015 06:06:45 -0700 Andi Kleen <andi@firstfloor.org> wrote:

> From: Andi Kleen <ak@linux.intel.com>
> 
> const char *...[]  is not const, but an array of pointer to const.
> So these arrays cannot be __initconst, but must be __initdata
> 
> This fixes section conflicts with LTO.
> 
> --- a/lib/test-hexdump.c
> +++ b/lib/test-hexdump.c
> @@ -18,26 +18,26 @@ static const unsigned char data_b[] = {
>  
>  static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
>  
> -static const char *test_data_1_le[] __initconst = {
> +static const char * const test_data_1_le[] __initconst = {

const char * const __initconst

>  	"be", "32", "db", "7b", "0a", "18", "93", "b2",
>  	"70", "ba", "c4", "24", "7d", "83", "34", "9b",
>  	"a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
>  	"4c", "d1", "19", "99", "43", "b1", "af", "0c",
>  };
>  
> +static const char *test_data_2_le[] __initdata = {
> +static const char *test_data_4_le[] __initdata = {
> +static const char *test_data_8_le[] __initdata = {

const char * __initdata

Why is test_data_1_le[] different?

Can we make them all "const char * const __initconst"?  That would make
checkpatch happy ;)

Re: [PATCH 2/2] test-hexdump.c: Fix initconst confusion

[Andi Kleen]

On Wed, Apr 08, 2015 at 03:35:54PM -0700, Andrew Morton wrote:
> > 
> > --- a/lib/test-hexdump.c
> > +++ b/lib/test-hexdump.c
> > @@ -18,26 +18,26 @@ static const unsigned char data_b[] = {
> >  
> >  static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
> >  
> > -static const char *test_data_1_le[] __initconst = {
> > +static const char * const test_data_1_le[] __initconst = {
> 
> const char * const __initconst

This one didn't cause any warnings elsewhere.

> 
> >  	"be", "32", "db", "7b", "0a", "18", "93", "b2",
> >  	"70", "ba", "c4", "24", "7d", "83", "34", "9b",
> >  	"a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
> >  	"4c", "d1", "19", "99", "43", "b1", "af", "0c",
> >  };
> >  
> > +static const char *test_data_2_le[] __initdata = {
> > +static const char *test_data_4_le[] __initdata = {
> > +static const char *test_data_8_le[] __initdata = {
> 
> const char * __initdata
> 
> Why is test_data_1_le[] different?
> 
> Can we make them all "const char * const __initconst"?  That would make
> checkpatch happy ;)

I tried it, but it would have needed a lot more changes to shut up
warnings later in the code. This was the least intrusive.

checkpatch is a bit stupid about this, but then C declarations are
difficult to parse...

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

[PATCH] checkpatch: Add a test for const with __read_mostly uses

[Joe Perches]

const objects shouldn't be __read_mostly.  They are read-only.

Marking these objects as __read_mostly causes section conflicts
with LTO linking.

So add a test to try to avoid this issue.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9a8b2bd..199d81b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4777,6 +4777,16 @@ sub process {
 			}
 		}
 
+# check for __read_mostly with const non-pointer (should just be const)
+		if ($line =~ /\b__read_mostly\b/ &&
+		    $line =~ /($Type)\s*$Ident/ && $1 !~ /\*\s*$/ && $1 =~ /\bconst\b/) {
+			if (ERROR("CONST_READ_MOSTLY",
+				  "Invalid use of __read_mostly with const type\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$fixlinenr] =~ s/\s+__read_mostly\b//;
+			}
+		}
+
 # don't use __constant_<foo> functions outside of include/uapi/
 		if ($realfile !~ m@^include/uapi/@ &&
 		    $line =~ /(__constant_(?:htons|ntohs|[bl]e(?:16|32|64)_to_cpu|cpu_to_[bl]e(?:16|32|64)))\s*\(/) {

Re: [PATCH] checkpatch: Add a test for const with __read_mostly uses

[Andi Kleen]

On Wed, Apr 08, 2015 at 05:14:49PM -0700, Joe Perches wrote:
> const objects shouldn't be __read_mostly.  They are read-only.
> 
> Marking these objects as __read_mostly causes section conflicts
> with LTO linking.
> 
> So add a test to try to avoid this issue.

Thanks Joe. Looks good thanks. I suspect excluding * 
will miss quite a few cases, but there's no way around it.

-Andi

Re: [PATCH] checkpatch: Add a test for const with __read_mostly uses

[Joe Perches]

On Thu, 2015-04-09 at 02:28 +0200, Andi Kleen wrote:
> On Wed, Apr 08, 2015 at 05:14:49PM -0700, Joe Perches wrote:
> > const objects shouldn't be __read_mostly.  They are read-only.
> > 
> > Marking these objects as __read_mostly causes section conflicts
> > with LTO linking.
> > 
> > So add a test to try to avoid this issue.
> 
> I suspect excluding *
> will miss quite a few cases, but there's no way around it.

That excludes only "const <foo> *" not "const foo * const"
so I believe it shouldn't exclude anything that's not
pointer to const.

Re: [PATCH 2/2] test-hexdump.c: Fix initconst confusion

[Geert Uytterhoeven]

On Thu, Apr 9, 2015 at 1:52 AM, Andi Kleen <andi@firstfloor.org> wrote:
> On Wed, Apr 08, 2015 at 03:35:54PM -0700, Andrew Morton wrote:
>> >
>> > --- a/lib/test-hexdump.c
>> > +++ b/lib/test-hexdump.c
>> > @@ -18,26 +18,26 @@ static const unsigned char data_b[] = {
>> >
>> >  static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
>> >
>> > -static const char *test_data_1_le[] __initconst = {
>> > +static const char * const test_data_1_le[] __initconst = {
>>
>> const char * const __initconst
>
> This one didn't cause any warnings elsewhere.
>
>>
>> >     "be", "32", "db", "7b", "0a", "18", "93", "b2",
>> >     "70", "ba", "c4", "24", "7d", "83", "34", "9b",
>> >     "a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
>> >     "4c", "d1", "19", "99", "43", "b1", "af", "0c",
>> >  };
>> >
>> > +static const char *test_data_2_le[] __initdata = {
>> > +static const char *test_data_4_le[] __initdata = {
>> > +static const char *test_data_8_le[] __initdata = {
>>
>> const char * __initdata
>>
>> Why is test_data_1_le[] different?
>>
>> Can we make them all "const char * const __initconst"?  That would make
>> checkpatch happy ;)
>
> I tried it, but it would have needed a lot more changes to shut up
> warnings later in the code. This was the least intrusive.

"a lot more changes" is "one more change"? (patch sent)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH 2/2] test-hexdump.c: Fix initconst confusion

[Andi Kleen]

> > I tried it, but it would have needed a lot more changes to shut up
> > warnings later in the code. This was the least intrusive.
> 
> "a lot more changes" is "one more change"? (patch sent)

Thanks Geert. You're right of course.
-Andi