compaction.h: undefined CONFIG_ZONE_HIGHMEM_

compaction.h: undefined CONFIG_ZONE_HIGHMEM_

[Valentin Rothberg]

Hi Vlastimil,

your commit 1434c81a47e3 ("mm, compaction: export tracepoints zone names
to userspace") has shown up in todays linux-next tree (i.e., 20150923)
adding the following lines of code:

--- a/include/trace/events/compaction.h
+++ b/include/trace/events/compaction.h
@@ -18,6 +18,31 @@
[...]
+#ifdef CONFIG_ZONE_HIGHMEM_
+#define IFDEF_ZONE_HIGHMEM(X) X
+#else
+#define IFDEF_ZONE_HIGHMEM(X)
+#endif

At the current state, the #ifdef block will not see a compiler since
CONFIG_ZONE_HIGHMEM_ is not defined anywhere.  At first I thought it's a
typo as it ends with '_', but even ZONE_HIGHMEM isn't defined in
Kconfig.  Is there a patch queued somewhere to fix the issue?

I detected the issue with undertaker-checkpatch from [1].  We run a bot
daily on linux-next to detect some Kconfig related issues and bugs.

Kind regards,
 Valentin

[1] https://undertaker.cs.fau.de

Re: compaction.h: undefined CONFIG_ZONE_HIGHMEM_

[Vlastimil Babka]

On 09/23/2015 10:43 AM, Valentin Rothberg wrote:
> Hi Vlastimil,
> 
> your commit 1434c81a47e3 ("mm, compaction: export tracepoints zone names
> to userspace") has shown up in todays linux-next tree (i.e., 20150923)
> adding the following lines of code:
> 
> --- a/include/trace/events/compaction.h
> +++ b/include/trace/events/compaction.h
> @@ -18,6 +18,31 @@
> [...]
> +#ifdef CONFIG_ZONE_HIGHMEM_
> +#define IFDEF_ZONE_HIGHMEM(X) X
> +#else
> +#define IFDEF_ZONE_HIGHMEM(X)
> +#endif
> 
> At the current state, the #ifdef block will not see a compiler since
> CONFIG_ZONE_HIGHMEM_ is not defined anywhere. 

Ah damn, that's why a simple compile test won't catch this typo.

> At first I thought it's a
> typo as it ends with '_', but even ZONE_HIGHMEM isn't defined in
> Kconfig.  Is there a patch queued somewhere to fix the issue?

It's actually just CONFIG_HIGHMEM (who needs consistency anyway?). Patch
below. I didn't rename the IFDEF_ZONE_HIGHMEM as it's internal.

When looking at zone_names I've noticed a CONFIG_ZONE_DEVICE. I assume
compaction can't be run on this one so I'll ignore it. At worst the
tracepoint string would be missing.

> I detected the issue with undertaker-checkpatch from [1].  We run a
> bot daily on linux-next to detect some Kconfig related issues and
> bugs.

Yeah I've heard about it on last year's Plumbers. Thanks for catching this!

> Kind regards, Valentin
> 
> [1] https://undertaker.cs.fau.de

------8<------
From: Vlastimil Babka <vbabka@suse.cz>
Date: Wed, 23 Sep 2015 11:31:10 +0200
Subject: [PATCH] mm, compaction: export tracepoints zone names to
 userspace-fix

Through undertaker-checkpatch it was reported that HighMem would be missing
in the tracepoint output due to checking CONFIG_ZONE_HIGHMEM_ instead of
CONFIG_HIGHMEM. Fix it.

Reported-by: Valentin Rothberg <valentinrothberg@gmail.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/trace/events/compaction.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/trace/events/compaction.h b/include/trace/events/compaction.h
index 5604994..c92d1e1 100644
--- a/include/trace/events/compaction.h
+++ b/include/trace/events/compaction.h
@@ -31,7 +31,7 @@
 #define IFDEF_ZONE_DMA32(X)
 #endif
 
-#ifdef CONFIG_ZONE_HIGHMEM_
+#ifdef CONFIG_HIGHMEM
 #define IFDEF_ZONE_HIGHMEM(X) X
 #else
 #define IFDEF_ZONE_HIGHMEM(X)
-- 
2.5.1



 

Re: compaction.h: undefined CONFIG_ZONE_HIGHMEM_

[Valentin Rothberg]

On Sep 23 '15 11:34, Vlastimil Babka wrote:
> On 09/23/2015 10:43 AM, Valentin Rothberg wrote:
> > Hi Vlastimil,
> > 
> > your commit 1434c81a47e3 ("mm, compaction: export tracepoints zone names
> > to userspace") has shown up in todays linux-next tree (i.e., 20150923)
> > adding the following lines of code:
> > 
> > --- a/include/trace/events/compaction.h
> > +++ b/include/trace/events/compaction.h
> > @@ -18,6 +18,31 @@
> > [...]
> > +#ifdef CONFIG_ZONE_HIGHMEM_
> > +#define IFDEF_ZONE_HIGHMEM(X) X
> > +#else
> > +#define IFDEF_ZONE_HIGHMEM(X)
> > +#endif
> > 
> > At the current state, the #ifdef block will not see a compiler since
> > CONFIG_ZONE_HIGHMEM_ is not defined anywhere. 
> 
> Ah damn, that's why a simple compile test won't catch this typo.
> 
> > At first I thought it's a
> > typo as it ends with '_', but even ZONE_HIGHMEM isn't defined in
> > Kconfig.  Is there a patch queued somewhere to fix the issue?
> 
> It's actually just CONFIG_HIGHMEM (who needs consistency anyway?). Patch
> below. I didn't rename the IFDEF_ZONE_HIGHMEM as it's internal.
> 
> When looking at zone_names I've noticed a CONFIG_ZONE_DEVICE. I assume
> compaction can't be run on this one so I'll ignore it. At worst the
> tracepoint string would be missing.
> 
> > I detected the issue with undertaker-checkpatch from [1].  We run a
> > bot daily on linux-next to detect some Kconfig related issues and
> > bugs.
> 
> Yeah I've heard about it on last year's Plumbers. Thanks for catching this!

Cool, happy to hear when people know our tools!

We put parts of the symbolic analysis such as the "CONFIG_ZONE_HIGHMEM_"
case into scripts/checkkconfigsymbols.py.  It's quite funny that
checkkconfigsymbols did not catch your commit since it requires Kconfig
symbols to end with something alphanumerical, if not they will be
ignored - an everlasting fight with false postives and false negatives :)

Thanks,
  Valentin