[PATCH -next] mm: MEMTEST depends on MEMBLOCK

[PATCH -next] mm: MEMTEST depends on MEMBLOCK

[Guenter Roeck]

Building alpha:allmodconfig fails with

mm/memtest.c: In function 'reserve_bad_mem':
mm/memtest.c:38:2: error: implicit declaration of function 'memblock_reserve'
mm/memtest.c: In function 'do_one_pass':
mm/memtest.c:77:2: error: implicit declaration of function 'for_each_free_mem_range'
mm/memtest.c:77:73: error: expected ';' before '{' token

because it depends on MEMBLOCK which is not defined for the alpha
architecture.

Fixes: 420c89e6185d ("mm: move memtest under mm")
Cc: Vladimir Murzin <vladimir.murzin@arm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 lib/Kconfig.debug | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8b4e69a..dc1e69c 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1797,6 +1797,7 @@ config TEST_UDELAY
 
 config MEMTEST
 	bool "Memtest"
+	depends on MEMBLOCK
 	---help---
 	  This option adds a kernel parameter 'memtest', which allows memtest
 	  to be set.
-- 
2.1.0

Re: [PATCH -next] mm: MEMTEST depends on MEMBLOCK

[Vladimir Murzin]

On 13/03/15 09:03, Guenter Roeck wrote:
> Building alpha:allmodconfig fails with
> 
> mm/memtest.c: In function 'reserve_bad_mem':
> mm/memtest.c:38:2: error: implicit declaration of function 'memblock_reserve'
> mm/memtest.c: In function 'do_one_pass':
> mm/memtest.c:77:2: error: implicit declaration of function 'for_each_free_mem_range'
> mm/memtest.c:77:73: error: expected ';' before '{' token
> 
> because it depends on MEMBLOCK which is not defined for the alpha
> architecture.
> 
> Fixes: 420c89e6185d ("mm: move memtest under mm")
> Cc: Vladimir Murzin <vladimir.murzin@arm.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  lib/Kconfig.debug | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 8b4e69a..dc1e69c 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1797,6 +1797,7 @@ config TEST_UDELAY
>  
>  config MEMTEST
>  	bool "Memtest"
> +	depends on MEMBLOCK
>  	---help---
>  	  This option adds a kernel parameter 'memtest', which allows memtest
>  	  to be set.
> 

Thanks for fixing it!

Acked-by: Vladimir Murzin <vladimir.murzin@arm.com>

Cheers
Vladimir

Re: [PATCH -next] mm: MEMTEST depends on MEMBLOCK

[Geert Uytterhoeven]

On Fri, Mar 13, 2015 at 10:03 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> Building alpha:allmodconfig fails with
>
> mm/memtest.c: In function 'reserve_bad_mem':
> mm/memtest.c:38:2: error: implicit declaration of function 'memblock_reserve'
> mm/memtest.c: In function 'do_one_pass':
> mm/memtest.c:77:2: error: implicit declaration of function 'for_each_free_mem_range'
> mm/memtest.c:77:73: error: expected ';' before '{' token
>
> because it depends on MEMBLOCK which is not defined for the alpha
> architecture.
>
> Fixes: 420c89e6185d ("mm: move memtest under mm")
> Cc: Vladimir Murzin <vladimir.murzin@arm.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

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 -next] mm: MEMTEST depends on MEMBLOCK

[Paul Bolle]

On Fri, 2015-03-13 at 02:03 -0700, Guenter Roeck wrote:
> Building alpha:allmodconfig fails with
> 
> mm/memtest.c: In function 'reserve_bad_mem':
> mm/memtest.c:38:2: error: implicit declaration of function 'memblock_reserve'
> mm/memtest.c: In function 'do_one_pass':
> mm/memtest.c:77:2: error: implicit declaration of function 'for_each_free_mem_range'
> mm/memtest.c:77:73: error: expected ';' before '{' token
> 
> because it depends on MEMBLOCK which is not defined for the alpha
> architecture.
> 
> Fixes: 420c89e6185d ("mm: move memtest under mm")
> Cc: Vladimir Murzin <vladimir.murzin@arm.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  lib/Kconfig.debug | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 8b4e69a..dc1e69c 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1797,6 +1797,7 @@ config TEST_UDELAY
>  
>  config MEMTEST
>  	bool "Memtest"
> +	depends on MEMBLOCK
>  	---help---
>  	  This option adds a kernel parameter 'memtest', which allows memtest
>  	  to be set.

(There's now a bot checking this stuff, but I couldn't resist reporting
it when my script spotted it.)

Please do
    $ git grep -n "config\s\+MEMBLOCK\b" next-20150320
    $ git grep -n "config\s\+MEMBLOCK\b" v4.0-rc4

I think this patch simply disables MEMTEST altogether. I'm guessing this
should have added
	depends on HAVE_MEMBLOCK


Paul Bolle

Re: [PATCH -next] mm: MEMTEST depends on MEMBLOCK

[Vladimir Murzin]

On 20/03/15 10:23, Paul Bolle wrote:
> On Fri, 2015-03-13 at 02:03 -0700, Guenter Roeck wrote:
>> Building alpha:allmodconfig fails with
>>
>> mm/memtest.c: In function 'reserve_bad_mem':
>> mm/memtest.c:38:2: error: implicit declaration of function 'memblock_reserve'
>> mm/memtest.c: In function 'do_one_pass':
>> mm/memtest.c:77:2: error: implicit declaration of function 'for_each_free_mem_range'
>> mm/memtest.c:77:73: error: expected ';' before '{' token
>>
>> because it depends on MEMBLOCK which is not defined for the alpha
>> architecture.
>>
>> Fixes: 420c89e6185d ("mm: move memtest under mm")
>> Cc: Vladimir Murzin <vladimir.murzin@arm.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>  lib/Kconfig.debug | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 8b4e69a..dc1e69c 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1797,6 +1797,7 @@ config TEST_UDELAY
>>  
>>  config MEMTEST
>>  	bool "Memtest"
>> +	depends on MEMBLOCK
>>  	---help---
>>  	  This option adds a kernel parameter 'memtest', which allows memtest
>>  	  to be set.
> 
> (There's now a bot checking this stuff, but I couldn't resist reporting
> it when my script spotted it.)
> 
> Please do
>     $ git grep -n "config\s\+MEMBLOCK\b" next-20150320
>     $ git grep -n "config\s\+MEMBLOCK\b" v4.0-rc4
> 
> I think this patch simply disables MEMTEST altogether. I'm guessing this

I should have tested it before... yes, you (and bot) are right, after
patch applied memtest is not available at arm/arm64 which definitely has
memblock.

> should have added
> 	depends on HAVE_MEMBLOCK
> 

Could we fold this change or separate patch is needed?

Cheers
Vladimir

> 
> Paul Bolle
> 
> 

Re: [PATCH -next] mm: MEMTEST depends on MEMBLOCK

[Paul Bolle]

On Fri, 2015-03-20 at 10:39 +0000, Vladimir Murzin wrote:
> I should have tested it before...

Valentin proposed an update to scripts/checkkconfigsymbols.py in 
https://lkml.org/lkml/2015/3/16/190 . It (or something like it) should
have helped testing for typos like that:
    python scripts/checkkconfigsymbols.py --commit b47818820b392273da5470975a29aca4cba92cb4
    MEMBLOCK	lib/Kconfig.debug

Which is actually rather cool!

Valentin, what's the status of that update?


Paul Bolle

Re: [PATCH -next] mm: MEMTEST depends on MEMBLOCK

[Valentin Rothberg]

On Fri, Mar 20, 2015 at 11:52 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Fri, 2015-03-20 at 10:39 +0000, Vladimir Murzin wrote:
>> I should have tested it before...
>
> Valentin proposed an update to scripts/checkkconfigsymbols.py in
> https://lkml.org/lkml/2015/3/16/190 . It (or something like it) should
> have helped testing for typos like that:
>     python scripts/checkkconfigsymbols.py --commit b47818820b392273da5470975a29aca4cba92cb4
>     MEMBLOCK    lib/Kconfig.debug
>
> Which is actually rather cool!
>
> Valentin, what's the status of that update?

Greg applied it to his misc tree, so it should arrive very soon in linux-next.

Kind regards,
 Valentin

Re: [PATCH -next] mm: MEMTEST depends on MEMBLOCK

[Stefan Hengelein]

Would it be reasonable to add an entry (or expand entry 8) in
Documentation/SubmitChecklist to point to the script?
"If your patch has changed conditions in Kconfig, run "python
scripts/checkkconfigsymbols.py --commit $your_commit"" ?

2015-03-20 11:52 GMT+01:00 Paul Bolle <pebolle@tiscali.nl>:
> On Fri, 2015-03-20 at 10:39 +0000, Vladimir Murzin wrote:
>> I should have tested it before...
>
> Valentin proposed an update to scripts/checkkconfigsymbols.py in
> https://lkml.org/lkml/2015/3/16/190 . It (or something like it) should
> have helped testing for typos like that:
>     python scripts/checkkconfigsymbols.py --commit b47818820b392273da5470975a29aca4cba92cb4
>     MEMBLOCK    lib/Kconfig.debug
>
> Which is actually rather cool!
>
> Valentin, what's the status of that update?
>
>
> Paul Bolle
>

Re: [PATCH -next] mm: MEMTEST depends on MEMBLOCK

[Paul Bolle]

On Fri, 2015-03-20 at 15:34 +0100, Stefan Hengelein wrote:
> 2015-03-20 11:52 GMT+01:00 Paul Bolle <pebolle@tiscali.nl>:
> > Valentin proposed an update to scripts/checkkconfigsymbols.py in
> > https://lkml.org/lkml/2015/3/16/190 . It (or something like it) should
> > have helped testing for typos like that:
> >     python scripts/checkkconfigsymbols.py --commit b47818820b392273da5470975a29aca4cba92cb4
> >     MEMBLOCK    lib/Kconfig.debug
> >
> > Which is actually rather cool!
> >
> Would it be reasonable to add an entry (or expand entry 8) in
> Documentation/SubmitChecklist to point to the script?
> "If your patch has changed conditions in Kconfig, run "python
> scripts/checkkconfigsymbols.py --commit $your_commit"" ?

That's not for me to decide. However, I would suggest you wait a bit
before adding it to that list (or any other piece of Documentation).
Just to be sure it works for most people.

Once that update has hit linux-next, you can point patch authors to it
every time the bot finds something the script would have caught too.
That should enlarge the pool of users. And, as complaints tend to be
voiced loud and clear, that in turn should help shake out any bugs or
ill considered features.


Paul Bolle