[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <92E87FB0-4C98-4E03-A2CF-AE365237D29A@nvidia.com>
Date: Mon, 09 Feb 2026 21:32:00 -0500
From: Zi Yan <ziy@...dia.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>
Cc: "David Hildenbrand (Arm)" <david@...nel.org>,
Mikhail Gavrilov <mikhail.v.gavrilov@...il.com>, linux-mm@...ck.org,
akpm@...ux-foundation.org, vbabka@...e.cz, surenb@...gle.com,
mhocko@...e.com, jackmanb@...gle.com, hannes@...xchg.org, npiggin@...il.com,
linux-kernel@...r.kernel.org, kasong@...cent.com, hughd@...gle.com,
chrisl@...nel.org, ryncsn@...il.com, stable@...r.kernel.org,
willy@...radead.org
Subject: Re: [PATCH v3] mm/page_alloc: clear page->private in
free_pages_prepare()
On 9 Feb 2026, at 21:25, Baolin Wang wrote:
> On 2/10/26 10:12 AM, Zi Yan wrote:
>> On 9 Feb 2026, at 20:20, Baolin Wang wrote:
>>
>>> On 2/10/26 3:42 AM, Zi Yan wrote:
>>>> On 9 Feb 2026, at 14:39, David Hildenbrand (Arm) wrote:
>>>>
>>>>> On 2/9/26 18:44, Zi Yan wrote:
>>>>>> On 9 Feb 2026, at 12:36, David Hildenbrand (Arm) wrote:
>>>>>>
>>>>>>> On 2/9/26 17:33, Zi Yan wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> I agree. Silently fixing non zero ->private just moves the work/responsibility
>>>>>>>> from users to core mm. They could do better. :)
>>>>>>>>
>>>>>>>> We can have a patch or multiple patches to fix users do not zero ->private
>>>>>>>> when freeing a page and add the patch below.
>>>>>>>
>>>>>>> Do we know roughly which ones don't zero it out?
>>>>>>
>>>>>> So far based on [1], I found:
>>>>>>
>>>>>> 1. shmem_swapin_folio() in mm/shmem.c does not zero ->swap.val (overlapping
>>>>>> with private);
>>>
>>> After Kairui’s series [1], the shmem part looks good to me. As we no longer skip the swapcache now, we shouldn’t clear the ->swap.val of a swapcache folio if failed to swap-in.
>>
>> What do you mean by "after Kairui's series[1]"? Can you elaborate a little bit more?
>
> Sure. This patch [2] in Kairui's series will never skip the swapcache, which means the shmem folio we’re trying to swap-in must be in the swapcache.
>
> [2] https://lore.kernel.org/all/20251219195751.61328-1-ryncsn@gmail.com/T/#me242d9f77d2caa126124afd5a7731113e8f0346e
>
>> For the diff below, does the "folio_put(folio)" have different outcomes based on
>> skip_swapcache? Only if skip_swapcache is true, "folio_put(folio)" frees the folio?
>
> Please check the latest mm-stable branch. The skip_swapcache related logic has been removed by Kairui’s series [1].
>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index ec6c01378e9d..546e193ef993 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -2437,8 +2437,10 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
>> failed_nolock:
>> if (skip_swapcache)
>> swapcache_clear(si, folio->swap, folio_nr_pages(folio));
>> - if (folio)
>> + if (folio) {
>> + folio->swap.val = 0;
>> folio_put(folio);
>> + }
>> put_swap_device(si);
>>
>> return error;
>
> Without Kairui's series, this change is incorrect. Yes, only if skip_swapcache is true, the "folio_put(folio)" frees the folio. Otherwise the folio is in the swapcache, and we will not free it.
Got it. Thanks. I just realized that the above diff is on top of v6.19-rc7.
The fix to mm-new/mm-stable for shmem should be:
diff --git a/mm/shmem.c b/mm/shmem.c
index eaaeca8f6c39..a52eca656ade 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2447,8 +2447,10 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
if (folio)
folio_unlock(folio);
failed_nolock:
- if (folio)
+ if (folio) {
+ folio->swap.val = 0;
folio_put(folio);
+ }
put_swap_device(si);
return error;
Thank you for the explanation.
>
>>> [1]https://lore.kernel.org/all/20251219195751.61328-1-ryncsn@gmail.com/T/#mcba8a32e1021dc28ce1e824c9d042dca316a30d7
--
Best Regards,
Yan, Zi
Powered by blists - more mailing lists