mm/huge_memory: update file PMD counter before folio_put()

[PATCH] mm/huge_memory: update file PMD counter before folio_put()

Posted by Yin Tirui 1 week, 6 days ago

__split_huge_pmd_locked() updates the file/shmem RSS counter after
dropping the PMD mapping's folio reference. If folio_put() drops the
last reference, mm_counter_file() can later read freed folio state via
folio_test_swapbacked().

Move the counter update before folio_put().

Fixes: fadae2953072 ("thp: use mm_file_counter to determine update which rss counter")
Cc: <stable@vger.kernel.org>
Signed-off-by: Yin Tirui <yintirui@huawei.com>
---
 mm/huge_memory.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 0135c29a4372..a5f4a48b7b77 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3145,7 +3145,9 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
 			if (!folio_test_referenced(folio) && pmd_young(old_pmd))
 				folio_set_referenced(folio);
 			folio_remove_rmap_pmd(folio, page, vma);
+			add_mm_counter(mm, mm_counter_file(folio), -HPAGE_PMD_NR);
 			folio_put(folio);
+			return;
 		}
 		add_mm_counter(mm, mm_counter_file(folio), -HPAGE_PMD_NR);
 		return;
-- 
2.43.0

Re: [PATCH] mm/huge_memory: update file PMD counter before folio_put()

Posted by Dev Jain 1 week, 5 days ago


On 26/05/26 3:43 pm, Yin Tirui wrote:
> __split_huge_pmd_locked() updates the file/shmem RSS counter after
> dropping the PMD mapping's folio reference. If folio_put() drops the
> last reference, mm_counter_file() can later read freed folio state via
> folio_test_swapbacked().
> 
> Move the counter update before folio_put().
> 
> Fixes: fadae2953072 ("thp: use mm_file_counter to determine update which rss counter")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Yin Tirui <yintirui@huawei.com>
> ---

Reviewed-by: Dev Jain <dev.jain@arm.com>

>  mm/huge_memory.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 0135c29a4372..a5f4a48b7b77 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3145,7 +3145,9 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
>  			if (!folio_test_referenced(folio) && pmd_young(old_pmd))
>  				folio_set_referenced(folio);
>  			folio_remove_rmap_pmd(folio, page, vma);
> +			add_mm_counter(mm, mm_counter_file(folio), -HPAGE_PMD_NR);
>  			folio_put(folio);
> +			return;
>  		}
>  		add_mm_counter(mm, mm_counter_file(folio), -HPAGE_PMD_NR);
>  		return;

Re: [PATCH] mm/huge_memory: update file PMD counter before folio_put()

Posted by Lance Yang 1 week, 6 days ago

On Tue, May 26, 2026 at 06:13:37PM +0800, Yin Tirui wrote:
>__split_huge_pmd_locked() updates the file/shmem RSS counter after
>dropping the PMD mapping's folio reference. If folio_put() drops the
>last reference, mm_counter_file() can later read freed folio state via
>folio_test_swapbacked().
>
>Move the counter update before folio_put().
>
>Fixes: fadae2953072 ("thp: use mm_file_counter to determine update which rss counter")
>Cc: <stable@vger.kernel.org>
>Signed-off-by: Yin Tirui <yintirui@huawei.com>
>---

Thanks! Feel free to add:
Reviewed-by: Lance Yang <lance.yang@linux.dev>

Re: [PATCH] mm/huge_memory: update file PMD counter before folio_put()

Posted by Lorenzo Stoakes 1 week, 6 days ago

On Tue, May 26, 2026 at 06:13:37PM +0800, Yin Tirui wrote:
> __split_huge_pmd_locked() updates the file/shmem RSS counter after
> dropping the PMD mapping's folio reference. If folio_put() drops the
> last reference, mm_counter_file() can later read freed folio state via
> folio_test_swapbacked().
>
> Move the counter update before folio_put().
>
> Fixes: fadae2953072 ("thp: use mm_file_counter to determine update which rss counter")

That's an old commit :) I mean I suspect we're probably not actually ever
dropping the folio ref to 0 here since we never had a report since ~2018.

The page cache keeping a reference I guess?

But doesn't mean we shouldn't fix this on principal/there being some way
this could happen.

> Cc: <stable@vger.kernel.org>
> Signed-off-by: Yin Tirui <yintirui@huawei.com>

LGTM, so:

Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>

> ---
>  mm/huge_memory.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 0135c29a4372..a5f4a48b7b77 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3145,7 +3145,9 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
>  			if (!folio_test_referenced(folio) && pmd_young(old_pmd))
>  				folio_set_referenced(folio);
>  			folio_remove_rmap_pmd(folio, page, vma);
> +			add_mm_counter(mm, mm_counter_file(folio), -HPAGE_PMD_NR);
>  			folio_put(folio);
> +			return;

Hmm, sucks to duplicate like this, but for purposes of backport and getting
this resolved fine, we can clean it up later.

>  		}
>  		add_mm_counter(mm, mm_counter_file(folio), -HPAGE_PMD_NR);
>  		return;
> --
> 2.43.0
>

Cheers, Lorenzo

Re: [PATCH] mm/huge_memory: update file PMD counter before folio_put()

Posted by David Hildenbrand (Arm) 1 week, 6 days ago

On 5/26/26 13:05, Lorenzo Stoakes wrote:
> On Tue, May 26, 2026 at 06:13:37PM +0800, Yin Tirui wrote:
>> __split_huge_pmd_locked() updates the file/shmem RSS counter after
>> dropping the PMD mapping's folio reference. If folio_put() drops the
>> last reference, mm_counter_file() can later read freed folio state via
>> folio_test_swapbacked().
>>
>> Move the counter update before folio_put().
>>
>> Fixes: fadae2953072 ("thp: use mm_file_counter to determine update which rss counter")
> 
> That's an old commit :) I mean I suspect we're probably not actually ever
> dropping the folio ref to 0 here since we never had a report since ~2018.
> 
> The page cache keeping a reference I guess?

I assume we could be racing with truncation.

Truncation would have to trigger unmap itself before we do the
folio_remove_rmap_pmd().

While the race could happen in theory I think, I do assume this would be rather
hard to trigger.

> 
> But doesn't mean we shouldn't fix this on principal/there being some way
> this could happen.
> 
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Yin Tirui <yintirui@huawei.com>
> 
> LGTM, so:
> 
> Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>

Acked-by: David Hildenbrand (arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH] mm/huge_memory: update file PMD counter before folio_put()

Posted by Lorenzo Stoakes 1 week, 5 days ago

On Tue, May 26, 2026 at 02:25:35PM +0200, David Hildenbrand (Arm) wrote:
> On 5/26/26 13:05, Lorenzo Stoakes wrote:
> > On Tue, May 26, 2026 at 06:13:37PM +0800, Yin Tirui wrote:
> >> __split_huge_pmd_locked() updates the file/shmem RSS counter after
> >> dropping the PMD mapping's folio reference. If folio_put() drops the
> >> last reference, mm_counter_file() can later read freed folio state via
> >> folio_test_swapbacked().
> >>
> >> Move the counter update before folio_put().
> >>
> >> Fixes: fadae2953072 ("thp: use mm_file_counter to determine update which rss counter")
> >
> > That's an old commit :) I mean I suspect we're probably not actually ever
> > dropping the folio ref to 0 here since we never had a report since ~2018.
> >
> > The page cache keeping a reference I guess?
>
> I assume we could be racing with truncation.
>
> Truncation would have to trigger unmap itself before we do the
> folio_remove_rmap_pmd().
>
> While the race could happen in theory I think, I do assume this would be rather
> hard to trigger.

Yeah, I mean unless we missed it somehow it seems like any such race if it
exists is very tiny.

But obviously we really do need to fix this! :)

>
> >
> > But doesn't mean we shouldn't fix this on principal/there being some way
> > this could happen.
> >
> >> Cc: <stable@vger.kernel.org>
> >> Signed-off-by: Yin Tirui <yintirui@huawei.com>
> >
> > LGTM, so:
> >
> > Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
>
> Acked-by: David Hildenbrand (arm) <david@kernel.org>
>
> --
> Cheers,
>
> David

Cheers, Lorenzo

Re: [PATCH] mm/huge_memory: update file PMD counter before folio_put()

Posted by Yin Tirui 1 week, 6 days ago

On 5/26/2026 7:05 PM, Lorenzo Stoakes wrote:
> On Tue, May 26, 2026 at 06:13:37PM +0800, Yin Tirui wrote:
>> __split_huge_pmd_locked() updates the file/shmem RSS counter after
>> dropping the PMD mapping's folio reference. If folio_put() drops the
>> last reference, mm_counter_file() can later read freed folio state via
>> folio_test_swapbacked().
>>
>> Move the counter update before folio_put().
>>
>> Fixes: fadae2953072 ("thp: use mm_file_counter to determine update which rss counter")
> That's an old commit :) I mean I suspect we're probably not actually ever
> dropping the folio ref to 0 here since we never had a report since ~2018.
>
> The page cache keeping a reference I guess?
>
> But doesn't mean we shouldn't fix this on principal/there being some way
> this could happen.
Yes, agreed.
>
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Yin Tirui <yintirui@huawei.com>
> LGTM, so:
>
> Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
Thanks!
>> ---
>>   mm/huge_memory.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 0135c29a4372..a5f4a48b7b77 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3145,7 +3145,9 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
>>   			if (!folio_test_referenced(folio) && pmd_young(old_pmd))
>>   				folio_set_referenced(folio);
>>   			folio_remove_rmap_pmd(folio, page, vma);
>> +			add_mm_counter(mm, mm_counter_file(folio), -HPAGE_PMD_NR);
>>   			folio_put(folio);
>> +			return;
> Hmm, sucks to duplicate like this, but for purposes of backport and getting
> this resolved fine, we can clean it up later.
Yes, that was my intention as well: keep this patch minimal for backport.
I will post the related refactoring patch soon.
>>   		}
>>   		add_mm_counter(mm, mm_counter_file(folio), -HPAGE_PMD_NR);
>>   		return;
>> --
>> 2.43.0
>>
> Cheers, Lorenzo
>
-- 
Yin Tirui