Re: [PATCH] mm: memory: fix truncation removing mapped folios

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 10/10/26 1:01 AM, Pedro Falcato wrote:
> On Fri, Oct 09, 2026 at 09:53:55PM +0800, Baolin Wang wrote:
>>
>>
>> On 10/9/26 9:36 PM, Pedro Falcato wrote:
>>> On Fri, Oct 09, 2026 at 01:00:52PM +0100, Lorenzo Stoakes (ARM) wrote:
>>>> On Fri, Oct 09, 2026 at 06:12:03PM +0800, Baolin Wang wrote:
>>>>> diff --git a/mm/memory.c b/mm/memory.c
>>>>> index 1f5d5f7d39cd..0272217ad7b0 100644
>>>>> --- a/mm/memory.c
>>>>> +++ b/mm/memory.c
>>>>
>>>>> @@ -2044,16 +2053,10 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
>>>>>    	 * to ensure they are still none, thereby preventing the pte entries
>>>>>    	 * from being repopulated by another thread.
>>>>>    	 */
>>>>> -	if (can_reclaim_pt && direct_reclaim && addr == end)
>>>>> +	if (can_reclaim_pt && direct_reclaim && addr == end) {
>>>>> +		/* rmap changes need to be observed before e.g PTEs get zapped. */
>>>>> +		smp_wmb();
>>>>
>>>> What does this pair with? You should always say what in the comment.
>>>
>>> Yep, this is missing a rmb to pair with. The diff I posted at
>>> https://lore.kernel.org/linux-mm/asigKktcrSY80A_2@pedro-suse.tail5790ac.ts.net/
>>> had one in zap_pmd_range(), which admittedly isn't the greatest. I'm not sure if
>>> there's a better way to do this.
>>
>> I don't think we need an rmb to pair with. This is because the
>> folio_mapped() check in __filemap_remove_folio() cannot be reordered
>> (guarded by spinlock) with the pmd_none() check in zap_pmd_range().
> 
> Why? spin_lock has ACQUIRE semantics, which are one-way permeable. Earlier
> loads and stores do not necessarily happen-before anything after the ACQUIRE.
> Unless you mean something else. 
A simplified view of the memory access logic on CPU 0 is as follows:

down_read() -> Acquire A1
pmd_none(): read pmd
up_read() -> Release R1

spin_lock() -> Acquire A2
folio_mapped(): read mapcount
spin_unlock -> Release R2

Initially I thought the read to folio_mapped() could not be reordered 
into the preceding acquire/release section, but after re-reading the 
memory-barriers.txt documentation, I realized that it actually can.

“
  (3) ACQUIRE vs ACQUIRE implication:
      All ACQUIRE operations issued before another ACQUIRE operation will be
      completed before that ACQUIRE operation.

  (4) ACQUIRE vs RELEASE implication:
      All ACQUIRE operations issued before a RELEASE operation will be
      completed before the RELEASE operation.
”

A possible out-of-order access sequence is:
A1 → A2 → read mapcount → read pmd → R1 → R2

So yes, you are right. I'll add an rmb barrier in v2.

Thanks to both of you for the review.