[PATCH] resource, kunit: add dependency on SPARSEMEM

[PATCH] resource, kunit: add dependency on SPARSEMEM

[Guenter Roeck]

Building allmodconfig images on systems with SPARSEMEM=n results in
the following message.

WARNING: unmet direct dependencies detected for GET_FREE_REGION
  Depends on [n]: SPARSEMEM [=n]
  Selected by [m]:
  - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]

and the build ultimately fails.

GET_FREE_REGION depends on SPARSEMEM, so any configuration selecting it
also depends on SPARSEMEM. Add the missing dependency.

Effectively that means that RESOURCE_KUNIT_TEST is now restricted to
systems with SPARSEMEM=y, but that can not be helped.

Fixes: 99185c10d5d9 ("resource, kunit: add test case for region_intersects()")
Cc: Huang Ying <ying.huang@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 lib/Kconfig.debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index bc8faa4509e1..52184c51b6dc 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2635,7 +2635,7 @@ config HASH_KUNIT_TEST
 
 config RESOURCE_KUNIT_TEST
 	tristate "KUnit test for resource API" if !KUNIT_ALL_TESTS
-	depends on KUNIT
+	depends on KUNIT && SPARSEMEM
 	default KUNIT_ALL_TESTS
 	select GET_FREE_REGION
 	help
-- 
2.45.2

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Huang, Ying]

Guenter Roeck <linux@roeck-us.net> writes:

> Building allmodconfig images on systems with SPARSEMEM=n results in
> the following message.
>
> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>   Depends on [n]: SPARSEMEM [=n]
>   Selected by [m]:
>   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>
> and the build ultimately fails.
>
> GET_FREE_REGION depends on SPARSEMEM, so any configuration selecting it
> also depends on SPARSEMEM. Add the missing dependency.
>
> Effectively that means that RESOURCE_KUNIT_TEST is now restricted to
> systems with SPARSEMEM=y, but that can not be helped.
>
> Fixes: 99185c10d5d9 ("resource, kunit: add test case for region_intersects()")
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Thanks for fixing.  It's better to fix this via remove dependency of
SPARSEMEM from GET_FREE_REGION.  However, we need to sort out some merge
conflict before that.  So, I think this patch is good as a quick fix.

Acked-by: "Huang, Ying" <ying.huang@intel.com>

> ---
>  lib/Kconfig.debug | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index bc8faa4509e1..52184c51b6dc 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2635,7 +2635,7 @@ config HASH_KUNIT_TEST
>  
>  config RESOURCE_KUNIT_TEST
>  	tristate "KUnit test for resource API" if !KUNIT_ALL_TESTS
> -	depends on KUNIT
> +	depends on KUNIT && SPARSEMEM
>  	default KUNIT_ALL_TESTS
>  	select GET_FREE_REGION
>  	help

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Geert Uytterhoeven]

Hi Günter,

On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net> wrote:
> Building allmodconfig images on systems with SPARSEMEM=n results in
> the following message.
>
> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>   Depends on [n]: SPARSEMEM [=n]
>   Selected by [m]:
>   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>
> and the build ultimately fails.

Really? What's the build error?
It does build for me on m68k, after fixing:

    --- a/include/linux/mm.h
    +++ b/include/linux/mm.h
    @@ -101,7 +101,7 @@ extern int mmap_rnd_compat_bits __read_mostly;
     # ifdef MAX_PHYSMEM_BITS
     # define PHYSMEM_END   ((1ULL << MAX_PHYSMEM_BITS) - 1)
     # else
    -# define PHYSMEM_END   (-1ULL)
   +# define PHYSMEM_END   ((phys_addr_t)-1)
     # endif
     #endif

> GET_FREE_REGION depends on SPARSEMEM, so any configuration selecting it
> also depends on SPARSEMEM. Add the missing dependency.
>
> Effectively that means that RESOURCE_KUNIT_TEST is now restricted to
> systems with SPARSEMEM=y, but that can not be helped.

Perhaps the individual test(s) that do depend on GET_FREE_REGION should
be protected by #ifdef CONFIG_GET_FREE_REGION instead? However,
I have no idea which parts depend on that, as apparently all tests
succeed on m68k/ARAnyM, with CONFIG_SPARSEMEM=n:

    KTAP version 1
    1..1
        KTAP version 1
        # Subtest: resource
        # module: resource_kunit
        1..3
        ok 1 resource_test_union
        ok 2 resource_test_intersection
        ok 3 resource_test_region_intersects
    # resource: pass:3 fail:0 skip:0 total:3
    # Totals: pass:3 fail:0 skip:0 total:3
    ok 1 resource

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] resource, kunit: add dependency on SPARSEMEM

[Guenter Roeck]

On 9/23/24 05:58, Geert Uytterhoeven wrote:
> Hi Günter,
> 
> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net> wrote:
>> Building allmodconfig images on systems with SPARSEMEM=n results in
>> the following message.
>>
>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>>    Depends on [n]: SPARSEMEM [=n]
>>    Selected by [m]:
>>    - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>>
>> and the build ultimately fails.
> 
> Really? What's the build error?

I saw it on hexagon, and I didn't bother writing down the actual build error
message. But it turns out you are correct, the m68k machine does build with
CONFIG_RESOURCE_KUNIT_TEST=y even though SPARSEMEM and with it GET_FREE_REGION
are not set. Never mind, I don't really want or have time to argue. I'll just
disable CONFIG_RESOURCE_KUNIT_TEST when building hexagon images and wherever
else I see the problem.

> It does build for me on m68k, after fixing:
> 
>      --- a/include/linux/mm.h
>      +++ b/include/linux/mm.h
>      @@ -101,7 +101,7 @@ extern int mmap_rnd_compat_bits __read_mostly;
>       # ifdef MAX_PHYSMEM_BITS
>       # define PHYSMEM_END   ((1ULL << MAX_PHYSMEM_BITS) - 1)
>       # else
>      -# define PHYSMEM_END   (-1ULL)
>     +# define PHYSMEM_END   ((phys_addr_t)-1)
>       # endif
>       #endif
> 
>> GET_FREE_REGION depends on SPARSEMEM, so any configuration selecting it
>> also depends on SPARSEMEM. Add the missing dependency.
>>
>> Effectively that means that RESOURCE_KUNIT_TEST is now restricted to
>> systems with SPARSEMEM=y, but that can not be helped.
> 
> Perhaps the individual test(s) that do depend on GET_FREE_REGION should
> be protected by #ifdef CONFIG_GET_FREE_REGION instead? However,
> I have no idea which parts depend on that, as apparently all tests
> succeed on m68k/ARAnyM, with CONFIG_SPARSEMEM=n:
> 
>      KTAP version 1
>      1..1
>          KTAP version 1
>          # Subtest: resource
>          # module: resource_kunit
>          1..3
>          ok 1 resource_test_union
>          ok 2 resource_test_intersection
>          ok 3 resource_test_region_intersects
>      # resource: pass:3 fail:0 skip:0 total:3
>      # Totals: pass:3 fail:0 skip:0 total:3
>      ok 1 resource
> 

Interesting that you get that to boot. The q800 machine crashes for me
when trying to boot it in qemu with the latest upstream kernel, in function
__pte_offset_map_lock(). It bisects to commit 394290cba966 ("mm: turn
USE_SPLIT_PTE_PTLOCKS / USE_SPLIT_PTE_PTLOCKS into Kconfig options").
Reverting that patch fixes the crash for me. I guess you are not seeing that ?

Thanks,
Guenter

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Geert Uytterhoeven]

Hi Günter,

On Mon, Sep 23, 2024 at 3:39 PM Guenter Roeck <linux@roeck-us.net> wrote:
> Interesting that you get that to boot. The q800 machine crashes for me
> when trying to boot it in qemu with the latest upstream kernel, in function
> __pte_offset_map_lock(). It bisects to commit 394290cba966 ("mm: turn
> USE_SPLIT_PTE_PTLOCKS / USE_SPLIT_PTE_PTLOCKS into Kconfig options").
> Reverting that patch fixes the crash for me. I guess you are not seeing that ?

I never used qemu -M q800.
I have just verified that -M virt boots fine?

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] resource, kunit: add dependency on SPARSEMEM

[Guenter Roeck]

On 9/23/24 06:47, Geert Uytterhoeven wrote:
> Hi Günter,
> 
> On Mon, Sep 23, 2024 at 3:39 PM Guenter Roeck <linux@roeck-us.net> wrote:
>> Interesting that you get that to boot. The q800 machine crashes for me
>> when trying to boot it in qemu with the latest upstream kernel, in function
>> __pte_offset_map_lock(). It bisects to commit 394290cba966 ("mm: turn
>> USE_SPLIT_PTE_PTLOCKS / USE_SPLIT_PTE_PTLOCKS into Kconfig options").
>> Reverting that patch fixes the crash for me. I guess you are not seeing that ?
> 
> I never used qemu -M q800.
> I have just verified that -M virt boots fine?
> 

m68k doesn't define a NR_CPUs configuration option. The new "config
SPLIT_PTE_PTLOCKS" depends on "NR_CPUS >= 4" but for some reason that
evaluates to true if there is no NR_CPUS configuration option.

It is interesting that this does not affect the 'virt' machine.

Guenter

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Guenter Roeck]

On 9/23/24 06:47, Geert Uytterhoeven wrote:
> Hi Günter,
> 
> On Mon, Sep 23, 2024 at 3:39 PM Guenter Roeck <linux@roeck-us.net> wrote:
>> Interesting that you get that to boot. The q800 machine crashes for me
>> when trying to boot it in qemu with the latest upstream kernel, in function
>> __pte_offset_map_lock(). It bisects to commit 394290cba966 ("mm: turn
>> USE_SPLIT_PTE_PTLOCKS / USE_SPLIT_PTE_PTLOCKS into Kconfig options").
>> Reverting that patch fixes the crash for me. I guess you are not seeing that ?
> 
> I never used qemu -M q800.
> I have just verified that -M virt boots fine?
> 

Not for me :-(

Run /sbin/init as init process
Unable to handle kernel NULL pointer dereference at virtual address 00000014
Oops: 00000000
PC: [<000ca784>] __pte_offset_map_lock+0x36/0x7e

This is with virt_defconfig.

Guenter

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Geert Uytterhoeven]

Hi Günter.

On Mon, Sep 23, 2024 at 4:55 PM Guenter Roeck <linux@roeck-us.net> wrote:
> On 9/23/24 06:47, Geert Uytterhoeven wrote:
> > On Mon, Sep 23, 2024 at 3:39 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >> Interesting that you get that to boot. The q800 machine crashes for me
> >> when trying to boot it in qemu with the latest upstream kernel, in function
> >> __pte_offset_map_lock(). It bisects to commit 394290cba966 ("mm: turn
> >> USE_SPLIT_PTE_PTLOCKS / USE_SPLIT_PTE_PTLOCKS into Kconfig options").
> >> Reverting that patch fixes the crash for me. I guess you are not seeing that ?
> >
> > I never used qemu -M q800.
> > I have just verified that -M virt boots fine?
>
> Not for me :-(
>
> Run /sbin/init as init process
> Unable to handle kernel NULL pointer dereference at virtual address 00000014
> Oops: 00000000
> PC: [<000ca784>] __pte_offset_map_lock+0x36/0x7e
>
> This is with virt_defconfig.

Before, I was using my current development tree, which has lots of
local patches. So I retried with commit 394290cba966.
Boots fine into Debian:

root@debian:~# cat /etc/issue
Debian GNU/Linux bullseye/sid \n \l

root@debian:~# uname -r
6.11.0-rc6-virt-00033-g394290cba966
root@debian:~#

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] resource, kunit: add dependency on SPARSEMEM

[Guenter Roeck]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]

On 9/23/24 09:01, Geert Uytterhoeven wrote:
> Hi Günter.
> 
> On Mon, Sep 23, 2024 at 4:55 PM Guenter Roeck <linux@roeck-us.net> wrote:
>> On 9/23/24 06:47, Geert Uytterhoeven wrote:
>>> On Mon, Sep 23, 2024 at 3:39 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>>> Interesting that you get that to boot. The q800 machine crashes for me
>>>> when trying to boot it in qemu with the latest upstream kernel, in function
>>>> __pte_offset_map_lock(). It bisects to commit 394290cba966 ("mm: turn
>>>> USE_SPLIT_PTE_PTLOCKS / USE_SPLIT_PTE_PTLOCKS into Kconfig options").
>>>> Reverting that patch fixes the crash for me. I guess you are not seeing that ?
>>>
>>> I never used qemu -M q800.
>>> I have just verified that -M virt boots fine?
>>
>> Not for me :-(
>>
>> Run /sbin/init as init process
>> Unable to handle kernel NULL pointer dereference at virtual address 00000014
>> Oops: 00000000
>> PC: [<000ca784>] __pte_offset_map_lock+0x36/0x7e
>>
>> This is with virt_defconfig.
> 
> Before, I was using my current development tree, which has lots of
> local patches. So I retried with commit 394290cba966.
> Boots fine into Debian:
> 

Interesting. I have a lot of debug and kunit test options enabled. Turns out
I can boot cleanly with an unmodified virt_defconfig, but the crash is seen
if I enable the various debug and test options. I attached the defconfig
I used for reference.

Guenter

[-- Attachment #2: defconfig.debug.gz --]
[-- Type: application/gzip, Size: 1181 bytes --]

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Huang, Ying]

Guenter Roeck <linux@roeck-us.net> writes:

> On 9/23/24 05:58, Geert Uytterhoeven wrote:
>> Hi Günter,
>> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
>> wrote:
>>> Building allmodconfig images on systems with SPARSEMEM=n results in
>>> the following message.
>>>
>>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>>>    Depends on [n]: SPARSEMEM [=n]
>>>    Selected by [m]:
>>>    - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>>>
>>> and the build ultimately fails.
>> Really? What's the build error?
>
> I saw it on hexagon, and I didn't bother writing down the actual build error
> message. But it turns out you are correct, the m68k machine does build with
> CONFIG_RESOURCE_KUNIT_TEST=y even though SPARSEMEM and with it GET_FREE_REGION
> are not set. Never mind, I don't really want or have time to argue. I'll just
> disable CONFIG_RESOURCE_KUNIT_TEST when building hexagon images and wherever
> else I see the problem.
>
>> It does build for me on m68k, after fixing:
>>      --- a/include/linux/mm.h
>>      +++ b/include/linux/mm.h
>>      @@ -101,7 +101,7 @@ extern int mmap_rnd_compat_bits __read_mostly;
>>       # ifdef MAX_PHYSMEM_BITS
>>       # define PHYSMEM_END   ((1ULL << MAX_PHYSMEM_BITS) - 1)
>>       # else
>>      -# define PHYSMEM_END   (-1ULL)
>>     +# define PHYSMEM_END   ((phys_addr_t)-1)
>>       # endif
>>       #endif

After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to depend
on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
you check whether the following patch work for you on top of latest
upstream kernel (with Linus' fix).

------------------------8<-------------------------------
From ce1a930f74192a4a85c20564098470356f8c2ed4 Mon Sep 17 00:00:00 2001
From: Huang Ying <ying.huang@intel.com>
Date: Mon, 23 Sep 2024 09:24:03 +0800
Subject: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION

We want to use the functions configured via GET_FREE_REGION in
resource kunit tests.  However, GET_FREE_REGION depends on SPARSEMEM.
This makes resource kunit tests cannot be built on some architectures
lacking SPARSEMEM.  In fact, these functions doesn't depend on
SPARSEMEM now.  So, remove dependency on SPARSEMEM from
GET_FREE_REGION.

Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 mm/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index b72e7d040f78..f287b0d1c5fc 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1060,7 +1060,6 @@ config HMM_MIRROR
 	depends on MMU
 
 config GET_FREE_REGION
-	depends on SPARSEMEM
 	bool
 
 config DEVICE_PRIVATE
-- 
2.39.2

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Guenter Roeck]

On 9/23/24 18:21, Huang, Ying wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> 
>> On 9/23/24 05:58, Geert Uytterhoeven wrote:
>>> Hi Günter,
>>> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
>>> wrote:
>>>> Building allmodconfig images on systems with SPARSEMEM=n results in
>>>> the following message.
>>>>
>>>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>>>>     Depends on [n]: SPARSEMEM [=n]
>>>>     Selected by [m]:
>>>>     - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>>>>
>>>> and the build ultimately fails.
>>> Really? What's the build error?
>>
>> I saw it on hexagon, and I didn't bother writing down the actual build error
>> message. But it turns out you are correct, the m68k machine does build with
>> CONFIG_RESOURCE_KUNIT_TEST=y even though SPARSEMEM and with it GET_FREE_REGION
>> are not set. Never mind, I don't really want or have time to argue. I'll just
>> disable CONFIG_RESOURCE_KUNIT_TEST when building hexagon images and wherever
>> else I see the problem.
>>
>>> It does build for me on m68k, after fixing:
>>>       --- a/include/linux/mm.h
>>>       +++ b/include/linux/mm.h
>>>       @@ -101,7 +101,7 @@ extern int mmap_rnd_compat_bits __read_mostly;
>>>        # ifdef MAX_PHYSMEM_BITS
>>>        # define PHYSMEM_END   ((1ULL << MAX_PHYSMEM_BITS) - 1)
>>>        # else
>>>       -# define PHYSMEM_END   (-1ULL)
>>>      +# define PHYSMEM_END   ((phys_addr_t)-1)
>>>        # endif
>>>        #endif
> 
> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to depend
> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
> you check whether the following patch work for you on top of latest
> upstream kernel (with Linus' fix).
> 

It works for m68k. I'll run a complete test on all architectures/platforms
tonight.

Guenter

> ------------------------8<-------------------------------
>>From ce1a930f74192a4a85c20564098470356f8c2ed4 Mon Sep 17 00:00:00 2001
> From: Huang Ying <ying.huang@intel.com>
> Date: Mon, 23 Sep 2024 09:24:03 +0800
> Subject: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
> 
> We want to use the functions configured via GET_FREE_REGION in
> resource kunit tests.  However, GET_FREE_REGION depends on SPARSEMEM.
> This makes resource kunit tests cannot be built on some architectures
> lacking SPARSEMEM.  In fact, these functions doesn't depend on
> SPARSEMEM now.  So, remove dependency on SPARSEMEM from
> GET_FREE_REGION.
> 
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---
>   mm/Kconfig | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index b72e7d040f78..f287b0d1c5fc 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -1060,7 +1060,6 @@ config HMM_MIRROR
>   	depends on MMU
>   
>   config GET_FREE_REGION
> -	depends on SPARSEMEM
>   	bool
>   
>   config DEVICE_PRIVATE

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Huang, Ying]

Hi, Guenter,

Guenter Roeck <linux@roeck-us.net> writes:

> On 9/23/24 18:21, Huang, Ying wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:
>> 
>>> On 9/23/24 05:58, Geert Uytterhoeven wrote:
>>>> Hi Günter,
>>>> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
>>>> wrote:
>>>>> Building allmodconfig images on systems with SPARSEMEM=n results in
>>>>> the following message.
>>>>>
>>>>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>>>>>     Depends on [n]: SPARSEMEM [=n]
>>>>>     Selected by [m]:
>>>>>     - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>>>>>
>>>>> and the build ultimately fails.
>>>> Really? What's the build error?
>>>
>>> I saw it on hexagon, and I didn't bother writing down the actual build error
>>> message. But it turns out you are correct, the m68k machine does build with
>>> CONFIG_RESOURCE_KUNIT_TEST=y even though SPARSEMEM and with it GET_FREE_REGION
>>> are not set. Never mind, I don't really want or have time to argue. I'll just
>>> disable CONFIG_RESOURCE_KUNIT_TEST when building hexagon images and wherever
>>> else I see the problem.
>>>
>>>> It does build for me on m68k, after fixing:
>>>>       --- a/include/linux/mm.h
>>>>       +++ b/include/linux/mm.h
>>>>       @@ -101,7 +101,7 @@ extern int mmap_rnd_compat_bits __read_mostly;
>>>>        # ifdef MAX_PHYSMEM_BITS
>>>>        # define PHYSMEM_END   ((1ULL << MAX_PHYSMEM_BITS) - 1)
>>>>        # else
>>>>       -# define PHYSMEM_END   (-1ULL)
>>>>      +# define PHYSMEM_END   ((phys_addr_t)-1)
>>>>        # endif
>>>>        #endif
>> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to
>> depend
>> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
>> you check whether the following patch work for you on top of latest
>> upstream kernel (with Linus' fix).
>> 
>
> It works for m68k. I'll run a complete test on all architectures/platforms
> tonight.

Does it work in the complete test?

--
Thanks,
Huang, Ying

>> ------------------------8<-------------------------------
>>>From ce1a930f74192a4a85c20564098470356f8c2ed4 Mon Sep 17 00:00:00 2001
>> From: Huang Ying <ying.huang@intel.com>
>> Date: Mon, 23 Sep 2024 09:24:03 +0800
>> Subject: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
>> We want to use the functions configured via GET_FREE_REGION in
>> resource kunit tests.  However, GET_FREE_REGION depends on SPARSEMEM.
>> This makes resource kunit tests cannot be built on some architectures
>> lacking SPARSEMEM.  In fact, these functions doesn't depend on
>> SPARSEMEM now.  So, remove dependency on SPARSEMEM from
>> GET_FREE_REGION.
>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Nathan Chancellor <nathan@kernel.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> ---
>>   mm/Kconfig | 1 -
>>   1 file changed, 1 deletion(-)
>> diff --git a/mm/Kconfig b/mm/Kconfig
>> index b72e7d040f78..f287b0d1c5fc 100644
>> --- a/mm/Kconfig
>> +++ b/mm/Kconfig
>> @@ -1060,7 +1060,6 @@ config HMM_MIRROR
>>   	depends on MMU
>>     config GET_FREE_REGION
>> -	depends on SPARSEMEM
>>   	bool
>>     config DEVICE_PRIVATE

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Guenter Roeck]

On 10/9/24 23:40, Huang, Ying wrote:
> Hi, Guenter,
> 
> Guenter Roeck <linux@roeck-us.net> writes:
> 
>> On 9/23/24 18:21, Huang, Ying wrote:
>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>
>>>> On 9/23/24 05:58, Geert Uytterhoeven wrote:
>>>>> Hi Günter,
>>>>> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
>>>>> wrote:
>>>>>> Building allmodconfig images on systems with SPARSEMEM=n results in
>>>>>> the following message.
>>>>>>
>>>>>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>>>>>>      Depends on [n]: SPARSEMEM [=n]
>>>>>>      Selected by [m]:
>>>>>>      - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>>>>>>
>>>>>> and the build ultimately fails.
>>>>> Really? What's the build error?
>>>>
>>>> I saw it on hexagon, and I didn't bother writing down the actual build error
>>>> message. But it turns out you are correct, the m68k machine does build with
>>>> CONFIG_RESOURCE_KUNIT_TEST=y even though SPARSEMEM and with it GET_FREE_REGION
>>>> are not set. Never mind, I don't really want or have time to argue. I'll just
>>>> disable CONFIG_RESOURCE_KUNIT_TEST when building hexagon images and wherever
>>>> else I see the problem.
>>>>
>>>>> It does build for me on m68k, after fixing:
>>>>>        --- a/include/linux/mm.h
>>>>>        +++ b/include/linux/mm.h
>>>>>        @@ -101,7 +101,7 @@ extern int mmap_rnd_compat_bits __read_mostly;
>>>>>         # ifdef MAX_PHYSMEM_BITS
>>>>>         # define PHYSMEM_END   ((1ULL << MAX_PHYSMEM_BITS) - 1)
>>>>>         # else
>>>>>        -# define PHYSMEM_END   (-1ULL)
>>>>>       +# define PHYSMEM_END   ((phys_addr_t)-1)
>>>>>         # endif
>>>>>         #endif
>>> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to
>>> depend
>>> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
>>> you check whether the following patch work for you on top of latest
>>> upstream kernel (with Linus' fix).
>>>
>>
>> It works for m68k. I'll run a complete test on all architectures/platforms
>> tonight.
> 
> Does it work in the complete test?
> 

Sorry, I dropped that one. Yes, it does.

Thanks,
Guenter

> --
> Thanks,
> Huang, Ying
> 
>>> ------------------------8<-------------------------------
>>> >From ce1a930f74192a4a85c20564098470356f8c2ed4 Mon Sep 17 00:00:00 2001
>>> From: Huang Ying <ying.huang@intel.com>
>>> Date: Mon, 23 Sep 2024 09:24:03 +0800
>>> Subject: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
>>> We want to use the functions configured via GET_FREE_REGION in
>>> resource kunit tests.  However, GET_FREE_REGION depends on SPARSEMEM.
>>> This makes resource kunit tests cannot be built on some architectures
>>> lacking SPARSEMEM.  In fact, these functions doesn't depend on
>>> SPARSEMEM now.  So, remove dependency on SPARSEMEM from
>>> GET_FREE_REGION.
>>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>>> Cc: Nathan Chancellor <nathan@kernel.org>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>>    mm/Kconfig | 1 -
>>>    1 file changed, 1 deletion(-)
>>> diff --git a/mm/Kconfig b/mm/Kconfig
>>> index b72e7d040f78..f287b0d1c5fc 100644
>>> --- a/mm/Kconfig
>>> +++ b/mm/Kconfig
>>> @@ -1060,7 +1060,6 @@ config HMM_MIRROR
>>>    	depends on MMU
>>>      config GET_FREE_REGION
>>> -	depends on SPARSEMEM
>>>    	bool
>>>      config DEVICE_PRIVATE

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Huang, Ying]

Guenter Roeck <linux@roeck-us.net> writes:

> On 10/9/24 23:40, Huang, Ying wrote:
>> Hi, Guenter,
>> Guenter Roeck <linux@roeck-us.net> writes:
>> 
>>> On 9/23/24 18:21, Huang, Ying wrote:
>>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>>
>>>>> On 9/23/24 05:58, Geert Uytterhoeven wrote:
>>>>>> Hi Günter,
>>>>>> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
>>>>>> wrote:
>>>>>>> Building allmodconfig images on systems with SPARSEMEM=n results in
>>>>>>> the following message.
>>>>>>>
>>>>>>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>>>>>>>      Depends on [n]: SPARSEMEM [=n]
>>>>>>>      Selected by [m]:
>>>>>>>      - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>>>>>>>
>>>>>>> and the build ultimately fails.
>>>>>> Really? What's the build error?
>>>>>
>>>>> I saw it on hexagon, and I didn't bother writing down the actual build error
>>>>> message. But it turns out you are correct, the m68k machine does build with
>>>>> CONFIG_RESOURCE_KUNIT_TEST=y even though SPARSEMEM and with it GET_FREE_REGION
>>>>> are not set. Never mind, I don't really want or have time to argue. I'll just
>>>>> disable CONFIG_RESOURCE_KUNIT_TEST when building hexagon images and wherever
>>>>> else I see the problem.
>>>>>
>>>>>> It does build for me on m68k, after fixing:
>>>>>>        --- a/include/linux/mm.h
>>>>>>        +++ b/include/linux/mm.h
>>>>>>        @@ -101,7 +101,7 @@ extern int mmap_rnd_compat_bits __read_mostly;
>>>>>>         # ifdef MAX_PHYSMEM_BITS
>>>>>>         # define PHYSMEM_END   ((1ULL << MAX_PHYSMEM_BITS) - 1)
>>>>>>         # else
>>>>>>        -# define PHYSMEM_END   (-1ULL)
>>>>>>       +# define PHYSMEM_END   ((phys_addr_t)-1)
>>>>>>         # endif
>>>>>>         #endif
>>>> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to
>>>> depend
>>>> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
>>>> you check whether the following patch work for you on top of latest
>>>> upstream kernel (with Linus' fix).
>>>>
>>>
>>> It works for m68k. I'll run a complete test on all architectures/platforms
>>> tonight.
>> Does it work in the complete test?
>> 
>
> Sorry, I dropped that one. Yes, it does.
>

Thanks a lot!  Can I add your Tested-by in the formal version?

--
Best Regards,
Huang, Ying

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Guenter Roeck]

On 10/11/24 20:43, Huang, Ying wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> 
>> On 10/9/24 23:40, Huang, Ying wrote:
>>> Hi, Guenter,
>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>
>>>> On 9/23/24 18:21, Huang, Ying wrote:
>>>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>>>
>>>>>> On 9/23/24 05:58, Geert Uytterhoeven wrote:
>>>>>>> Hi Günter,
>>>>>>> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
>>>>>>> wrote:
>>>>>>>> Building allmodconfig images on systems with SPARSEMEM=n results in
>>>>>>>> the following message.
>>>>>>>>
>>>>>>>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>>>>>>>>       Depends on [n]: SPARSEMEM [=n]
>>>>>>>>       Selected by [m]:
>>>>>>>>       - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>>>>>>>>
>>>>>>>> and the build ultimately fails.
>>>>>>> Really? What's the build error?
>>>>>>
>>>>>> I saw it on hexagon, and I didn't bother writing down the actual build error
>>>>>> message. But it turns out you are correct, the m68k machine does build with
>>>>>> CONFIG_RESOURCE_KUNIT_TEST=y even though SPARSEMEM and with it GET_FREE_REGION
>>>>>> are not set. Never mind, I don't really want or have time to argue. I'll just
>>>>>> disable CONFIG_RESOURCE_KUNIT_TEST when building hexagon images and wherever
>>>>>> else I see the problem.
>>>>>>
>>>>>>> It does build for me on m68k, after fixing:
>>>>>>>         --- a/include/linux/mm.h
>>>>>>>         +++ b/include/linux/mm.h
>>>>>>>         @@ -101,7 +101,7 @@ extern int mmap_rnd_compat_bits __read_mostly;
>>>>>>>          # ifdef MAX_PHYSMEM_BITS
>>>>>>>          # define PHYSMEM_END   ((1ULL << MAX_PHYSMEM_BITS) - 1)
>>>>>>>          # else
>>>>>>>         -# define PHYSMEM_END   (-1ULL)
>>>>>>>        +# define PHYSMEM_END   ((phys_addr_t)-1)
>>>>>>>          # endif
>>>>>>>          #endif
>>>>> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to
>>>>> depend
>>>>> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
>>>>> you check whether the following patch work for you on top of latest
>>>>> upstream kernel (with Linus' fix).
>>>>>
>>>>
>>>> It works for m68k. I'll run a complete test on all architectures/platforms
>>>> tonight.
>>> Does it work in the complete test?
>>>
>>
>> Sorry, I dropped that one. Yes, it does.
>>
> 
> Thanks a lot!  Can I add your Tested-by in the formal version?
> 

Sure, go ahead.

Tested-by: Guenter Roeck <linux@roeck-us.net>

Guenter

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Geert Uytterhoeven]

Hi Huang,

On Tue, Sep 24, 2024 at 3:25 AM Huang, Ying <ying.huang@intel.com> wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> > On 9/23/24 05:58, Geert Uytterhoeven wrote:
> >> Hi Günter,
> >> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
> >> wrote:
> >>> Building allmodconfig images on systems with SPARSEMEM=n results in
> >>> the following message.
> >>>
> >>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
> >>>    Depends on [n]: SPARSEMEM [=n]
> >>>    Selected by [m]:
> >>>    - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]

> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to depend
> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
> you check whether the following patch work for you on top of latest
> upstream kernel (with Linus' fix).

Yes it does, thanks!

One remaining issue is that RESOURCE_KUNIT_TEST selects GET_FREE_REGION.
IMHO merely enabling a test should not enable extra functionality
in the kernel.  Can the individual test(s) that do depend on
GET_FREE_REGION be protected by #ifdef CONFIG_GET_FREE_REGION instead?

Thanks again!

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] resource, kunit: add dependency on SPARSEMEM

[Huang, Ying]

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

> Hi Huang,
>
> On Tue, Sep 24, 2024 at 3:25 AM Huang, Ying <ying.huang@intel.com> wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:
>> > On 9/23/24 05:58, Geert Uytterhoeven wrote:
>> >> Hi Günter,
>> >> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
>> >> wrote:
>> >>> Building allmodconfig images on systems with SPARSEMEM=n results in
>> >>> the following message.
>> >>>
>> >>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>> >>>    Depends on [n]: SPARSEMEM [=n]
>> >>>    Selected by [m]:
>> >>>    - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>
>> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to depend
>> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
>> you check whether the following patch work for you on top of latest
>> upstream kernel (with Linus' fix).
>
> Yes it does, thanks!
>
> One remaining issue is that RESOURCE_KUNIT_TEST selects GET_FREE_REGION.
> IMHO merely enabling a test should not enable extra functionality
> in the kernel.  Can the individual test(s) that do depend on
> GET_FREE_REGION be protected by #ifdef CONFIG_GET_FREE_REGION instead?

After checking GET_FREE_REGION, I don't think that it's a special
functionality.  I guess it's selectable because it depends on SPARSEMEM
and to reduce code size.

Hi, Dan, please correct me if I'm wrong here.

So, to reduce #ifdef in .c file as much as possible and make code
simpler, I prefer to select it for RESOURCE_KUNIT_TEST.

--
Best Regards,
Huang, Ying

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Dan Williams]

Huang, Ying wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> 
> > Hi Huang,
> >
> > On Tue, Sep 24, 2024 at 3:25 AM Huang, Ying <ying.huang@intel.com> wrote:
> >> Guenter Roeck <linux@roeck-us.net> writes:
> >> > On 9/23/24 05:58, Geert Uytterhoeven wrote:
> >> >> Hi Günter,
> >> >> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
> >> >> wrote:
> >> >>> Building allmodconfig images on systems with SPARSEMEM=n results in
> >> >>> the following message.
> >> >>>
> >> >>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
> >> >>>    Depends on [n]: SPARSEMEM [=n]
> >> >>>    Selected by [m]:
> >> >>>    - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
> >
> >> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to depend
> >> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
> >> you check whether the following patch work for you on top of latest
> >> upstream kernel (with Linus' fix).
> >
> > Yes it does, thanks!
> >
> > One remaining issue is that RESOURCE_KUNIT_TEST selects GET_FREE_REGION.
> > IMHO merely enabling a test should not enable extra functionality
> > in the kernel.  Can the individual test(s) that do depend on
> > GET_FREE_REGION be protected by #ifdef CONFIG_GET_FREE_REGION instead?
> 
> After checking GET_FREE_REGION, I don't think that it's a special
> functionality.  I guess it's selectable because it depends on SPARSEMEM
> and to reduce code size.
> 
> Hi, Dan, please correct me if I'm wrong here.

Right, the only reason it is selectable is just to be mindful is
micro-bloat in kernel/resource.c for the small number of drivers that
need that call.

> So, to reduce #ifdef in .c file as much as possible and make code
> simpler, I prefer to select it for RESOURCE_KUNIT_TEST.

I agree with the result, but for a different rationale.

RESOURCE_KUNIT_TEST is simply another "driver" that needs
GET_FREE_REGION. So while I agree with the general idea that "enabling a
test should not enable extra functionality", in this case the *test* is
the extra functionality and GET_FREE_REGION comes along for the ride.

Re: [PATCH] resource, kunit: add dependency on SPARSEMEM

[Huang, Ying]

Dan Williams <dan.j.williams@intel.com> writes:

> Huang, Ying wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> 
>> > Hi Huang,
>> >
>> > On Tue, Sep 24, 2024 at 3:25 AM Huang, Ying <ying.huang@intel.com> wrote:
>> >> Guenter Roeck <linux@roeck-us.net> writes:
>> >> > On 9/23/24 05:58, Geert Uytterhoeven wrote:
>> >> >> Hi Günter,
>> >> >> On Mon, Sep 23, 2024 at 12:50 AM Guenter Roeck <linux@roeck-us.net>
>> >> >> wrote:
>> >> >>> Building allmodconfig images on systems with SPARSEMEM=n results in
>> >> >>> the following message.
>> >> >>>
>> >> >>> WARNING: unmet direct dependencies detected for GET_FREE_REGION
>> >> >>>    Depends on [n]: SPARSEMEM [=n]
>> >> >>>    Selected by [m]:
>> >> >>>    - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>> >
>> >> After Linus' fix for PHYSMEM_END, GET_FREE_REGION doesn't need to depend
>> >> on SPARSEMEM anymore.  So, I think we can remove the dependency.  Can
>> >> you check whether the following patch work for you on top of latest
>> >> upstream kernel (with Linus' fix).
>> >
>> > Yes it does, thanks!
>> >
>> > One remaining issue is that RESOURCE_KUNIT_TEST selects GET_FREE_REGION.
>> > IMHO merely enabling a test should not enable extra functionality
>> > in the kernel.  Can the individual test(s) that do depend on
>> > GET_FREE_REGION be protected by #ifdef CONFIG_GET_FREE_REGION instead?
>> 
>> After checking GET_FREE_REGION, I don't think that it's a special
>> functionality.  I guess it's selectable because it depends on SPARSEMEM
>> and to reduce code size.
>> 
>> Hi, Dan, please correct me if I'm wrong here.
>
> Right, the only reason it is selectable is just to be mindful is
> micro-bloat in kernel/resource.c for the small number of drivers that
> need that call.

Thanks for explanation.

>> So, to reduce #ifdef in .c file as much as possible and make code
>> simpler, I prefer to select it for RESOURCE_KUNIT_TEST.
>
> I agree with the result, but for a different rationale.
>
> RESOURCE_KUNIT_TEST is simply another "driver" that needs
> GET_FREE_REGION. So while I agree with the general idea that "enabling a
> test should not enable extra functionality", in this case the *test* is
> the extra functionality and GET_FREE_REGION comes along for the ride.

Thanks, this sounds totally reasonable for me.

--
Best Regards,
Huang, Ying