[v1] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION

[PATCH -v2] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION

Posted by Huang Ying 1 month, 1 week ago

We want to use the functions (get_free_mem_region()) configured via
GET_FREE_REGION in resource kunit tests.  However, GET_FREE_REGION
depends on SPARSEMEM now.  This makes resource kunit tests cannot be
built on some architectures lacking SPARSEMEM, or causes config
warning as follows,

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

When get_free_mem_region() was introduced the only consumers were
those looking to pass the address range to memremap_pages(). That
address range needed to be mindful of the maximum addressable platform
physical address which at the time only SPARSMEM defined via
MAX_PHYSMEM_BITS.

Given that memremap_pages() also depended on SPARSEMEM via
ZONE_DEVICE, it was easier to just depend on that definition than
invent a general MAX_PHYSMEM_BITS concept outside of SPARSEMEM.

Turns out that decision was buggy and did not account for KASAN
consumption of physical address space. That problem was resolved
recently with commit ea72ce5da228 ("x86/kaslr: Expose and use the end
of the physical memory address space"), and GET_FREE_REGION dropped
its MAX_PHYSMEM_BITS dependency.

Then commit 99185c10d5d9 ("resource, kunit: add test case for
region_intersects()"), went ahead and fixed up the only remaining
dependency on SPARSEMEM which was usage of the PA_SECTION_SHIFT macro
for setting the default alignment. A PAGE_SIZE fallback is fine in the
SPARSEMEM=n case.

With those build dependencies gone GET_FREE_REGION no longer depends
on SPARSEMEM.  So, the patch removes dependency on SPARSEMEM from
GET_FREE_REGION to fix the build issues.

Link: https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
Fixes: 99185c10d5d9 ("resource, kunit: add test case for region_intersects()")
Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Co-developed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David Hildenbrand <david@redhat.com>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
---
 mm/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 4c9f5ea13271..33fa51d608dc 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1085,7 +1085,6 @@ config HMM_MIRROR
 	depends on MMU
 
 config GET_FREE_REGION
-	depends on SPARSEMEM
 	bool
 
 config DEVICE_PRIVATE
-- 
2.39.2

Re: [PATCH -v2] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION

Posted by Nathan Chancellor 1 month, 1 week ago

On Wed, Oct 16, 2024 at 09:47:30AM +0800, Huang Ying wrote:
> We want to use the functions (get_free_mem_region()) configured via
> GET_FREE_REGION in resource kunit tests.  However, GET_FREE_REGION
> depends on SPARSEMEM now.  This makes resource kunit tests cannot be
> built on some architectures lacking SPARSEMEM, or causes config
> warning as follows,
> 
>   WARNING: unmet direct dependencies detected for GET_FREE_REGION
>   Depends on [n]: SPARSEMEM [=n]
>   Selected by [y]:
>   - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y]
> 
> When get_free_mem_region() was introduced the only consumers were
> those looking to pass the address range to memremap_pages(). That
> address range needed to be mindful of the maximum addressable platform
> physical address which at the time only SPARSMEM defined via
> MAX_PHYSMEM_BITS.
> 
> Given that memremap_pages() also depended on SPARSEMEM via
> ZONE_DEVICE, it was easier to just depend on that definition than
> invent a general MAX_PHYSMEM_BITS concept outside of SPARSEMEM.
> 
> Turns out that decision was buggy and did not account for KASAN
> consumption of physical address space. That problem was resolved
> recently with commit ea72ce5da228 ("x86/kaslr: Expose and use the end
> of the physical memory address space"), and GET_FREE_REGION dropped
> its MAX_PHYSMEM_BITS dependency.
> 
> Then commit 99185c10d5d9 ("resource, kunit: add test case for
> region_intersects()"), went ahead and fixed up the only remaining
> dependency on SPARSEMEM which was usage of the PA_SECTION_SHIFT macro
> for setting the default alignment. A PAGE_SIZE fallback is fine in the
> SPARSEMEM=n case.
> 
> With those build dependencies gone GET_FREE_REGION no longer depends
> on SPARSEMEM.  So, the patch removes dependency on SPARSEMEM from
> GET_FREE_REGION to fix the build issues.
> 
> Link: https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
> Fixes: 99185c10d5d9 ("resource, kunit: add test case for region_intersects()")
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Tested-by: Guenter Roeck <linux@roeck-us.net>

Tested-by: Nathan Chancellor <nathan@kernel.org> # build

Thanks for adding the Fixes tag, as this should go via -hotfixes.

> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
>  mm/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 4c9f5ea13271..33fa51d608dc 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -1085,7 +1085,6 @@ config HMM_MIRROR
>  	depends on MMU
>  
>  config GET_FREE_REGION
> -	depends on SPARSEMEM
>  	bool
>  
>  config DEVICE_PRIVATE
> -- 
> 2.39.2
>

Re: [PATCH -v2] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION

Posted by David Hildenbrand 1 month, 1 week ago

On 16.10.24 03:47, Huang Ying wrote:
> We want to use the functions (get_free_mem_region()) configured via
> GET_FREE_REGION in resource kunit tests.  However, GET_FREE_REGION
> depends on SPARSEMEM now.  This makes resource kunit tests cannot be
> built on some architectures lacking SPARSEMEM, or causes config
> warning as follows,
> 
>    WARNING: unmet direct dependencies detected for GET_FREE_REGION
>    Depends on [n]: SPARSEMEM [=n]
>    Selected by [y]:
>    - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y]
> 
> When get_free_mem_region() was introduced the only consumers were
> those looking to pass the address range to memremap_pages(). That
> address range needed to be mindful of the maximum addressable platform
> physical address which at the time only SPARSMEM defined via
> MAX_PHYSMEM_BITS.
> 
> Given that memremap_pages() also depended on SPARSEMEM via
> ZONE_DEVICE, it was easier to just depend on that definition than
> invent a general MAX_PHYSMEM_BITS concept outside of SPARSEMEM.
> 
> Turns out that decision was buggy and did not account for KASAN
> consumption of physical address space. That problem was resolved
> recently with commit ea72ce5da228 ("x86/kaslr: Expose and use the end
> of the physical memory address space"), and GET_FREE_REGION dropped
> its MAX_PHYSMEM_BITS dependency.
> 
> Then commit 99185c10d5d9 ("resource, kunit: add test case for
> region_intersects()"), went ahead and fixed up the only remaining
> dependency on SPARSEMEM which was usage of the PA_SECTION_SHIFT macro
> for setting the default alignment. A PAGE_SIZE fallback is fine in the
> SPARSEMEM=n case.
> 
> With those build dependencies gone GET_FREE_REGION no longer depends
> on SPARSEMEM.  So, the patch removes dependency on SPARSEMEM from
> GET_FREE_REGION to fix the build issues.
> 
> Link: https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
> Fixes: 99185c10d5d9 ("resource, kunit: add test case for region_intersects()")
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
>   mm/Kconfig | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 4c9f5ea13271..33fa51d608dc 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -1085,7 +1085,6 @@ config HMM_MIRROR
>   	depends on MMU
>   
>   config GET_FREE_REGION
> -	depends on SPARSEMEM
>   	bool
>   
>   config DEVICE_PRIVATE

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb