Re: [PATCH 1/5] hugetlb: Make folio_test_hugetlb safer to call

[David Hildenbrand <david@redhat.com>]

On 01.03.24 22:47, Matthew Wilcox (Oracle) wrote:
> At least two places (memory failure and page migration) need to call
> folio_test_hugetlb() without a reference on the folio.  This can currently
> result in false positives (returning true when the folio doesn't belong
> to hugetlb) and more commonly in VM_BUG_ON() when a folio is split.
> 
> The new way to distinguish a hugetlb folio is to see if (1) the page
> is compound (or the folio is large) and (2) page[1].mapping is set to
> the address of hugetlb_lock.  If the folio is (or has been) large then
> page[1] is guaranteed to exist.  If the folio is split between the two
> tests, page[1].mapping will be set to something which definitely isn't
> the address of hugetlb_lock.
> 
> Because we shift around the layout of struct folio a bit, we now use
> page[1].private, which means we need to adjust __split_huge_page_tail()
> a little.  We also need to annoy the vmcore_info people again.  Sorry.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/linux/mm_types.h   |  4 +++-
>   include/linux/page-flags.h | 25 ++++---------------------
>   kernel/vmcore_info.c       |  3 ++-
>   mm/huge_memory.c           | 10 ++--------
>   mm/hugetlb.c               | 24 ++++++++++++++++++++++++
>   5 files changed, 35 insertions(+), 31 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index a7223ba3ea1e..fd80bf8b5d8a 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -289,6 +289,7 @@ typedef struct {
>    * @virtual: Virtual address in the kernel direct map.
>    * @_last_cpupid: IDs of last CPU and last process that accessed the folio.
>    * @_entire_mapcount: Do not use directly, call folio_entire_mapcount().
> + * @large_id: May identify the type of a large folio.
>    * @_nr_pages_mapped: Do not use directly, call folio_mapcount().
>    * @_pincount: Do not use directly, call folio_maybe_dma_pinned().
>    * @_folio_nr_pages: Do not use directly, call folio_nr_pages().
> @@ -348,9 +349,9 @@ struct folio {
>   		struct {
>   			unsigned long _flags_1;
>   			unsigned long _head_1;
> -			unsigned long _folio_avail;
>   	/* public: */
>   			atomic_t _entire_mapcount;
> +			void *large_id;
>   			atomic_t _nr_pages_mapped;
>   			atomic_t _pincount;

Hm, I wanted that for the total mapcount. :(

Great that hugetlb makes things once again worse for everybody ... gah.

-- 
Cheers,

David / dhildenb