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

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

[Oscar Salvador]

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.

Signed-off-by: Oscar Salvador <osalvador@suse.de>
---
Hi guys,

Although I can no longer reproduce the issue with this patch, I'm not entirely
sure this is the right way to fix the problem, so I'm open to
suggestions.
---
 mm/hugetlb_vmemmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
index 9a99dfa3c495..3d763182c834 100644
--- a/mm/hugetlb_vmemmap.c
+++ b/mm/hugetlb_vmemmap.c
@@ -498,6 +498,12 @@ static int __hugetlb_vmemmap_restore_folio(const struct hstate *h,
  */
 int hugetlb_vmemmap_restore_folio(const struct hstate *h, struct folio *folio)
 {
+	/*
+	 * Before restoring vmemmap, make sure to reset mapping to TAIL_MAPPING,
+	 * so tail pages that were reset will have the right thing after being
+	 * restored, and the checks in free_tail_page_prepare() will pass.
+	 */
+	set_hugetlb_cgroup(folio, TAIL_MAPPING);
 	return __hugetlb_vmemmap_restore_folio(h, folio, VMEMMAP_SYNCHRONIZE_RCU);
 }
 
-- 
2.49.0

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

[David Hildenbrand]

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.

-- 
Cheers,

David / dhildenb

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

[David Hildenbrand]

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?


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

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

[Muchun Song]

> On Apr 15, 2025, at 15:32, David Hildenbrand <david@redhat.com> 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

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

[Oscar Salvador]

On Tue, Apr 15, 2025 at 09:32:40AM +0200, David Hildenbrand wrote:
> Ah, reading about the HVO hackery in the comment above NR_RESET_STRUCT_PAGE, might the following fix it?

Yap, I was experimenting in parallel with that after sending the patch
and it also fixes the issue, and I think is nicer indeed.

I do not want to step on someone else's fet, so do you want me to send
the patch or will you?

thanks!


-- 
Oscar Salvador
SUSE Labs

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

[David Hildenbrand]

On 15.04.25 10:02, Oscar Salvador wrote:
> On Tue, Apr 15, 2025 at 09:32:40AM +0200, David Hildenbrand wrote:
>> Ah, reading about the HVO hackery in the comment above NR_RESET_STRUCT_PAGE, might the following fix it?
> 
> Yap, I was experimenting in parallel with that after sending the patch
> and it also fixes the issue, and I think is nicer indeed.
> 
> I do not want to step on someone else's fet, so do you want me to send
> the patch or will you?

Yes, please send it out.

-- 
Cheers,

David / dhildenb

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

[kernel test robot]

Hi Oscar,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Oscar-Salvador/mm-hugetlb-Reset-mapping-to-TAIL_MAPPING-before-restoring-vmemmap/20250415-134835
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20250415054705.370412-1-osalvador%40suse.de
patch subject: [PATCH] mm, hugetlb: Reset mapping to TAIL_MAPPING before restoring vmemmap
config: x86_64-buildonly-randconfig-003-20250416 (https://download.01.org/0day-ci/archive/20250416/202504161523.M2CmTjsj-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250416/202504161523.M2CmTjsj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504161523.M2CmTjsj-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/hugetlb_vmemmap.c: In function 'hugetlb_vmemmap_restore_folio':
>> mm/hugetlb_vmemmap.c:506:9: error: implicit declaration of function 'set_hugetlb_cgroup' [-Werror=implicit-function-declaration]
     506 |         set_hugetlb_cgroup(folio, TAIL_MAPPING);
         |         ^~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/set_hugetlb_cgroup +506 mm/hugetlb_vmemmap.c

   488	
   489	/**
   490	 * hugetlb_vmemmap_restore_folio - restore previously optimized (by
   491	 *				hugetlb_vmemmap_optimize_folio()) vmemmap pages which
   492	 *				will be reallocated and remapped.
   493	 * @h:		struct hstate.
   494	 * @folio:     the folio whose vmemmap pages will be restored.
   495	 *
   496	 * Return: %0 if @folio's vmemmap pages have been reallocated and remapped,
   497	 * negative error code otherwise.
   498	 */
   499	int hugetlb_vmemmap_restore_folio(const struct hstate *h, struct folio *folio)
   500	{
   501		/*
   502		 * Before restoring vmemmap, make sure to reset mapping to TAIL_MAPPING,
   503		 * so tail pages that were reset will have the right thing after being
   504		 * restored, and the checks in free_tail_page_prepare() will pass.
   505		 */
 > 506		set_hugetlb_cgroup(folio, TAIL_MAPPING);
   507		return __hugetlb_vmemmap_restore_folio(h, folio, VMEMMAP_SYNCHRONIZE_RCU);
   508	}
   509	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Oscar,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Oscar-Salvador/mm-hugetlb-Reset-mapping-to-TAIL_MAPPING-before-restoring-vmemmap/20250415-134835
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20250415054705.370412-1-osalvador%40suse.de
patch subject: [PATCH] mm, hugetlb: Reset mapping to TAIL_MAPPING before restoring vmemmap
config: x86_64-buildonly-randconfig-006-20250416 (https://download.01.org/0day-ci/archive/20250416/202504161522.S1qEFD4b-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250416/202504161522.S1qEFD4b-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504161522.S1qEFD4b-lkp@intel.com/

All errors (new ones prefixed by >>):

>> mm/hugetlb_vmemmap.c:506:2: error: call to undeclared function 'set_hugetlb_cgroup'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     506 |         set_hugetlb_cgroup(folio, TAIL_MAPPING);
         |         ^
   1 error generated.


vim +/set_hugetlb_cgroup +506 mm/hugetlb_vmemmap.c

   488	
   489	/**
   490	 * hugetlb_vmemmap_restore_folio - restore previously optimized (by
   491	 *				hugetlb_vmemmap_optimize_folio()) vmemmap pages which
   492	 *				will be reallocated and remapped.
   493	 * @h:		struct hstate.
   494	 * @folio:     the folio whose vmemmap pages will be restored.
   495	 *
   496	 * Return: %0 if @folio's vmemmap pages have been reallocated and remapped,
   497	 * negative error code otherwise.
   498	 */
   499	int hugetlb_vmemmap_restore_folio(const struct hstate *h, struct folio *folio)
   500	{
   501		/*
   502		 * Before restoring vmemmap, make sure to reset mapping to TAIL_MAPPING,
   503		 * so tail pages that were reset will have the right thing after being
   504		 * restored, and the checks in free_tail_page_prepare() will pass.
   505		 */
 > 506		set_hugetlb_cgroup(folio, TAIL_MAPPING);
   507		return __hugetlb_vmemmap_restore_folio(h, folio, VMEMMAP_SYNCHRONIZE_RCU);
   508	}
   509	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki