linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
@ 2024-10-15  5:15 Huang Ying
  2024-10-15  7:07 ` David Hildenbrand
  0 siblings, 1 reply; 6+ messages in thread
From: Huang Ying @ 2024-10-15  5:15 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Huang Ying, Guenter Roeck,
	Nathan Chancellor, Arnd Bergmann, Dan Williams, David Hildenbrand,
	Jonathan Cameron

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.

Link: https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dan Williams <dan.j.williams@intel.com>
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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
  2024-10-15  5:15 [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION Huang Ying
@ 2024-10-15  7:07 ` David Hildenbrand
  2024-10-15  8:03   ` Huang, Ying
  0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2024-10-15  7:07 UTC (permalink / raw)
  To: Huang Ying, Andrew Morton
  Cc: linux-mm, linux-kernel, Guenter Roeck, Nathan Chancellor,
	Arnd Bergmann, Dan Williams, Jonathan Cameron

On 15.10.24 07:15, Huang Ying wrote:
> 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.
> 
> Link: https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dan Williams <dan.j.williams@intel.com>
> 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

Added by

commit 14b80582c43e4f550acfd93c2b2cadbe36ea0874
Author: Dan Williams <dan.j.williams@intel.com>
Date:   Fri May 20 13:41:24 2022 -0700

     resource: Introduce alloc_free_mem_region()

@Dan, any insight why that dependency was added?

-- 
Cheers,

David / dhildenb



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
  2024-10-15  7:07 ` David Hildenbrand
@ 2024-10-15  8:03   ` Huang, Ying
  2024-10-15 10:33     ` David Hildenbrand
  0 siblings, 1 reply; 6+ messages in thread
From: Huang, Ying @ 2024-10-15  8:03 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Andrew Morton, linux-mm, linux-kernel, Guenter Roeck,
	Nathan Chancellor, Arnd Bergmann, Dan Williams, Jonathan Cameron

Hi, David,

David Hildenbrand <david@redhat.com> writes:

> On 15.10.24 07:15, Huang Ying wrote:
>> 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.
>> Link:
>> https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> Tested-by: Guenter Roeck <linux@roeck-us.net>
>> Cc: Nathan Chancellor <nathan@kernel.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> 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
>
> Added by
>
> commit 14b80582c43e4f550acfd93c2b2cadbe36ea0874
> Author: Dan Williams <dan.j.williams@intel.com>
> Date:   Fri May 20 13:41:24 2022 -0700
>
>     resource: Introduce alloc_free_mem_region()
>
> @Dan, any insight why that dependency was added?

Dan has explain it some what in the following email,

https://lore.kernel.org/lkml/66f5abd431dce_964f2294b9@dwillia2-xfh.jf.intel.com.notmuch/

This is reachable from the "Link:" tag in the patch.

--
Best Regards,
Huang, Ying


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
  2024-10-15  8:03   ` Huang, Ying
@ 2024-10-15 10:33     ` David Hildenbrand
  2024-10-16  0:00       ` Dan Williams
  0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2024-10-15 10:33 UTC (permalink / raw)
  To: Huang, Ying
  Cc: Andrew Morton, linux-mm, linux-kernel, Guenter Roeck,
	Nathan Chancellor, Arnd Bergmann, Dan Williams, Jonathan Cameron

On 15.10.24 10:03, Huang, Ying wrote:
> Hi, David,
> 
> David Hildenbrand <david@redhat.com> writes:
> 
>> On 15.10.24 07:15, Huang Ying wrote:
>>> 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.
>>> Link:
>>> https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
>>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>>> Tested-by: Guenter Roeck <linux@roeck-us.net>
>>> Cc: Nathan Chancellor <nathan@kernel.org>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: Dan Williams <dan.j.williams@intel.com>
>>> 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
>>
>> Added by
>>
>> commit 14b80582c43e4f550acfd93c2b2cadbe36ea0874
>> Author: Dan Williams <dan.j.williams@intel.com>
>> Date:   Fri May 20 13:41:24 2022 -0700
>>
>>      resource: Introduce alloc_free_mem_region()
>>
>> @Dan, any insight why that dependency was added?
> 
> Dan has explain it some what in the following email,
> 
> https://lore.kernel.org/lkml/66f5abd431dce_964f2294b9@dwillia2-xfh.jf.intel.com.notmuch/
> 
> This is reachable from the "Link:" tag in the patch.

That should be part of the patch description then :)

-- 
Cheers,

David / dhildenb



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
  2024-10-15 10:33     ` David Hildenbrand
@ 2024-10-16  0:00       ` Dan Williams
  2024-10-16  0:37         ` Huang, Ying
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Williams @ 2024-10-16  0:00 UTC (permalink / raw)
  To: David Hildenbrand, Huang, Ying
  Cc: Andrew Morton, linux-mm, linux-kernel, Guenter Roeck,
	Nathan Chancellor, Arnd Bergmann, Dan Williams, Jonathan Cameron

David Hildenbrand wrote:
> On 15.10.24 10:03, Huang, Ying wrote:
> > Hi, David,
> > 
> > David Hildenbrand <david@redhat.com> writes:
> > 
> >> On 15.10.24 07:15, Huang Ying wrote:
> >>> 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.
> >>> Link:
> >>> https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
> >>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> >>> Tested-by: Guenter Roeck <linux@roeck-us.net>
> >>> Cc: Nathan Chancellor <nathan@kernel.org>
> >>> Cc: Arnd Bergmann <arnd@arndb.de>
> >>> Cc: Dan Williams <dan.j.williams@intel.com>
> >>> 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
> >>
> >> Added by
> >>
> >> commit 14b80582c43e4f550acfd93c2b2cadbe36ea0874
> >> Author: Dan Williams <dan.j.williams@intel.com>
> >> Date:   Fri May 20 13:41:24 2022 -0700
> >>
> >>      resource: Introduce alloc_free_mem_region()
> >>
> >> @Dan, any insight why that dependency was added?
> > 
> > Dan has explain it some what in the following email,
> > 
> > https://lore.kernel.org/lkml/66f5abd431dce_964f2294b9@dwillia2-xfh.jf.intel.com.notmuch/
> > 
> > This is reachable from the "Link:" tag in the patch.
> 
> That should be part of the patch description then :)

That Link: does not really describe the history though...

The description I would add is:

---

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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION
  2024-10-16  0:00       ` Dan Williams
@ 2024-10-16  0:37         ` Huang, Ying
  0 siblings, 0 replies; 6+ messages in thread
From: Huang, Ying @ 2024-10-16  0:37 UTC (permalink / raw)
  To: Dan Williams
  Cc: David Hildenbrand, Andrew Morton, linux-mm, linux-kernel,
	Guenter Roeck, Nathan Chancellor, Arnd Bergmann, Jonathan Cameron

Hi, Dan,

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

> David Hildenbrand wrote:
>> On 15.10.24 10:03, Huang, Ying wrote:
>> > Hi, David,
>> > 
>> > David Hildenbrand <david@redhat.com> writes:
>> > 
>> >> On 15.10.24 07:15, Huang Ying wrote:
>> >>> 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.
>> >>> Link:
>> >>> https://lore.kernel.org/lkml/20240922225041.603186-1-linux@roeck-us.net/
>> >>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> >>> Tested-by: Guenter Roeck <linux@roeck-us.net>
>> >>> Cc: Nathan Chancellor <nathan@kernel.org>
>> >>> Cc: Arnd Bergmann <arnd@arndb.de>
>> >>> Cc: Dan Williams <dan.j.williams@intel.com>
>> >>> 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
>> >>
>> >> Added by
>> >>
>> >> commit 14b80582c43e4f550acfd93c2b2cadbe36ea0874
>> >> Author: Dan Williams <dan.j.williams@intel.com>
>> >> Date:   Fri May 20 13:41:24 2022 -0700
>> >>
>> >>      resource: Introduce alloc_free_mem_region()
>> >>
>> >> @Dan, any insight why that dependency was added?
>> > 
>> > Dan has explain it some what in the following email,
>> > 
>> > https://lore.kernel.org/lkml/66f5abd431dce_964f2294b9@dwillia2-xfh.jf.intel.com.notmuch/
>> > 
>> > This is reachable from the "Link:" tag in the patch.
>> 
>> That should be part of the patch description then :)
>
> That Link: does not really describe the history though...

Sorry.  I made a mistake here.

> The description I would add is:
>
> ---
>
> 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.

This looks great!  Will use this in the patch description in the next
version.

--
Best Regards,
Huang, Ying


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-10-16  0:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-15  5:15 [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE_REGION Huang Ying
2024-10-15  7:07 ` David Hildenbrand
2024-10-15  8:03   ` Huang, Ying
2024-10-15 10:33     ` David Hildenbrand
2024-10-16  0:00       ` Dan Williams
2024-10-16  0:37         ` Huang, Ying

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).