linux-kernel - Re: [PATCH] resource: Remove dependency on SPARSEMEM from GET_FREE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87wmi87uhw.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Wed, 16 Oct 2024 08:37:47 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Dan Williams <dan.j.williams@...el.com>
Cc: David Hildenbrand <david@...hat.com>,  Andrew Morton
 <akpm@...ux-foundation.org>,  linux-mm@...ck.org,
  linux-kernel@...r.kernel.org,  Guenter Roeck <linux@...ck-us.net>,
  Nathan Chancellor <nathan@...nel.org>,  Arnd Bergmann <arnd@...db.de>,
  Jonathan Cameron <jonathan.cameron@...wei.com>
Subject: Re: [PATCH] resource: Remove dependency on SPARSEMEM from
 GET_FREE_REGION

Hi, Dan,

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

> David Hildenbrand wrote:
>> On 15.10.24 10:03, Huang, Ying wrote:
>> > Hi, David,
>> > 
>> > David Hildenbrand <david@...hat.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@...el.com>
>> >>> Tested-by: Guenter Roeck <linux@...ck-us.net>
>> >>> Cc: Nathan Chancellor <nathan@...nel.org>
>> >>> Cc: Arnd Bergmann <arnd@...db.de>
>> >>> Cc: Dan Williams <dan.j.williams@...el.com>
>> >>> Cc: David Hildenbrand <david@...hat.com>
>> >>> Cc: Jonathan Cameron <jonathan.cameron@...wei.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@...el.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