Re: [PATCH] mm, hugetlb: Reset mapping to TAIL_MAPPING before restoring vmemmap

[Muchun Song]

> On Apr 15, 2025, at 15:32, David Hildenbrand <david@xxxxxxxxxx> wrote:
> 
> On 15.04.25 09:23, David Hildenbrand wrote:
>> On 15.04.25 07:47, Oscar Salvador wrote:
>>> commit 4eeec8c89a0c ("mm: move hugetlb specific things in folio to page[3]")
>>> shifted hugetlb specific stuff, and now mapping overlaps _hugetlb_cgroup field.
>>> 
>>> _hugetlb_cgroup is set to NULL when preparing the hugetlb page in
>>> init_new_hugetlb_folio().
>>> For a better picture, this is page->mapping before and after the comming
>>> for the first three tail pages:
>>> 
>>> before:
>>> page: fffff51a44358040  0000000000000000
>>> page: fffff51a44358080  0000000000000000
>>> page: fffff51a443580c0  dead000000000400
>>> 
>>> after:
>>> page: fffff1f0042b0040  0000000000000000
>>> page: fffff1f0042b0080  fffff1f0042b0090
>>> page: fffff1f0042b00c0  0000000000000000
>>> 
>>> Tail#2 has fffff1f0042b0090 because of the _deferred_list initialization,
>>> which was also shifted, but that is not a problem.
>>> 
>>> For HVO, upon restoring that gets copied in some tail pages (reset_struct_pages)
>>> and so those tail pages will not have TAIL_MAPPING set and the check
>>> in free_tail_page_prepare() will fail:
>>> 
>>>   kernel: BUG: Bad page state in process kworker/0:3  pfn:10ac40
>>>   kernel: page does not match folio
>>>   kernel: page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x10ac40
>>>   kernel: flags: 0x17ffffc0000000(node=0|zone=2|lastcpupid=0x1fffff)
>>>   kernel: raw: 0017ffffc0000000 fffff1f0042b0000 0000000000000000 0000000000000000
>>>   kernel: raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
>>>   kernel: page dumped because: corrupted mapping in tail page
>>> 
>>> Reset _hugetlb_cgroup to TAIL_MAPPING before restoring so tail pages have the
>>> right value.
>> Hi,
>> To handle that for ordinary hugtlb alloc/free I added in that patch in free_tail_page_prepare():
>> case 3:
>> /* the third tail page: hugetlb specifics overlap ->mappings */
>> if (IS_ENABLED(CONFIG_HUGETLB_PAGE))
>> break;
>> fallthrough;
>> default:
>> if (page->mapping != TAIL_MAPPING) {
>> bad_page(page, "corrupted mapping in tail page");
>> goto out;
>> }
>> break;
>> }
>> Now I am confused why that check doesn't catch that?
>> Apparently only a problem with HVO? Because I recall testing the ordinary alloc/free.
> 
> Ah, reading about the HVO hackery in the comment above NR_RESET_STRUCT_PAGE, might the following fix it?

Yes. And the Fixes tag should be

  Fixes: 4eeec8c89a0c ("mm: move hugetlb specific things in folio to page[3]")

Thanks.

> 
> 
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 9a99dfa3c4958..27245e86df250 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -238,11 +238,11 @@ static void vmemmap_remap_pte(pte_t *pte, unsigned long addr,
>  * struct page, the special metadata (e.g. page->flags or page->mapping)
>  * cannot copy to the tail struct page structs. The invalid value will be
>  * checked in the free_tail_page_prepare(). In order to avoid the message
> - * of "corrupted mapping in tail page". We need to reset at least 3 (one
> - * head struct page struct and two tail struct page structs) struct page
> + * of "corrupted mapping in tail page". We need to reset at least 4 (one
> + * head struct page struct and three tail struct page structs) struct page
>  * structs.
>  */
> -#define NR_RESET_STRUCT_PAGE           3
> +#define NR_RESET_STRUCT_PAGE           4
>  static inline void reset_struct_pages(struct page *start)
> {
> 
> -- 
> Cheers,
> 
> David / dhildenb