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

["David Hildenbrand (Arm)" <david@kernel.org>]

On 10/27/26 11:35, Baolin Wang wrote:
> 
> 
> On 10/27/26 5:33 PM, David Hildenbrand (Arm) wrote:
>> On 10/27/26 10:03, Baolin Wang wrote:
>>>
>>>
>>>
>>> No, keeping two pmd_none() checks cannot fix the race, as I replied before. [1]
>>>
>>
>> Best to wait to submit new versions if there are open discussions? Because now
>> we have the same discussion in two places.
> 
> Sure.
> 
>>
>> I think you're right about the race, the issue is that we check for
>> pmd_is_huge(), not pmd_present() which confused me.
>>
>> Clearest to me would be to not drop the second pmd_none() check but move it
>> where it belongs: if splitting zapped the PMD.
>>
>> @@ -2101,13 +2104,15 @@ static inline unsigned long zap_pmd_range(struct
>> mmu_gather *tlb,
>>                                  addr = next;
>>                                  continue;
>>                          }
>> +                       /* We already synchronized with concurrent zapping. */
>> +                       if (pmd_none(*pmd)) {
>> +                               addr = next;
>> +                               continue;
>> +                       }
>>                          /* fall through */
>> -               } else if (details && details->single_folio &&
>> -                          folio_test_pmd_mappable(details->single_folio) &&
>> -                          next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
>> -                       sync_with_folio_pmd_zap(tlb->mm, pmd);
>> -               }
>> -               if (pmd_none(*pmd)) {
>> +               } else if (pmd_none(*pmd)) {
>> +                       if (details && details->single_folio)
>> +                               sync_with_folio_pmd_zap(tlb->mm, pmd);
>>                          addr = next;
>>                          continue;
>>                  }
> 
> IMHO, I'm not a big fan of adding two pmd_none() checks just to avoid an
> uncommon concurrent sync. 

It's what we already had. We had to: one for handling pmd_none() directly and
one for handling zapping while splitting (file THPs).

We just managed to let the second also handle the final piece of the former.

Anyhow, let's get this fixed.

If you think the code is clearer your way (where I disagree), then keep it as is.

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

-- 
Cheers,

David