Re: [PATCH] tmpfs: zero post-eof folio range on file extension

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

On 2025/7/11 15:50, Hugh Dickins wrote:
> On Fri, 11 Jul 2025, Baolin Wang wrote:
>> On 2025/7/11 06:20, Hugh Dickins wrote:
>>> On Thu, 10 Jul 2025, Baolin Wang wrote:
>>>> On 2025/7/9 15:57, Hugh Dickins wrote:
>>> ...
>>>>>
>>>>> The problem is with huge pages (or large folios) in shmem_writeout():
>>>>> what goes in as a large folio may there have to be split into small
>>>>> pages; or it may be swapped out as one large folio, but fragmentation
>>>>> at swapin time demand that it be split into small pages when swapped in.
>>>>
>>>> Good point.
>>>>
>>>>> So, if there has been swapout since the large folio was modified beyond
>>>>> EOF, the folio that shmem_zero_eof() brings in does not guarantee what
>>>>> length needs to be zeroed.
>>>>>
>>>>> We could set that aside as a deficiency to be fixed later on: that
>>>>> would not be unreasonable, but I'm guessing that won't satisfy you.
>>>>>
>>>>> We could zero the maximum (the remainder of PMD size I believe) in
>>>>> shmem_zero_eof(): looping over small folios within the range, skipping
>>>>> !uptodate ones (but we do force them uptodate when swapping out, in
>>>>> order to keep the space reservation).  TBH I've ignored that as a bad
>>>>> option, but it doesn't seem so bad to me now: ugly, but maybe not bad.
>>>>
>>>> However, IIUC, if the large folios are split in shmem_writeout(), and those
>>>> small folios which beyond EOF will be dropped and freed in
>>>> __split_unmapped_folio(), should we still consider them?
>>>
>>> You're absolutely right about the normal case, and thank you for making
>>> that point.  Had I forgotten that when writing?  Or was I already
>>> jumping ahead to the problem case?  I don't recall, but was certainly
>>> wrong for not mentioning it.
>>>
>>> The abnormal case is when there's a "fallocend" beyond i_size (or beyond
>>> the small page extent spanning i_size) i.e. fallocate() has promised to
>>> keep pages allocated beyond EOF.  In that case, __split_unmapped_folio()
>>> is keeping those pages.
>>
>> Ah, yes, you are right.
>>
>>> There could well be some optimization, involving fallocend, to avoid
>>> zeroing more than necessary; but I wouldn't want to say what in a hurry,
>>> it's quite confusing!
>>
>> Like you said, not only can a large folio split occur during swapout, but it
>> can also happen during a punch hole operation. Moreover, considering the
>> abnormal case of fallocate() you mentioned, we should find a more common
>> approach to mitigate the impact of fallocate().
>>
>> For instance, when splitting, we could clear the 'uptodate' flag for these EOF
>> small folios that are beyond 'i_size' but less than the 'fallocend', so that
>> these EOF small folios will be re-initialized if they are used again. What do
>> you think?
> 
> First impression: that's a great idea, much better than anything I was
> proposing.  Let's hope I don't perceive some drawback overnight and renege.
> 
> I don't love your patch below so much, we would probably want to gather
> the shmem_mapping() peculiarities together better (and seeing that
> repeated i_size_read(): IIRC 32-bit locking doesn't allow it in there).

Absolutely yes, the following code just shows my thought:)

> And there tends to be an assumption (don't ask me where) that a page once
> uptodate remains that way until it's freed: maybe no problem before it's
> inserted in the xarray (as you have), or maybe better before unfreezing,
> or maybe the page lock is already enough.

These EOF small folios are unmapped, frozen, locked, and not yet in the 
xarray. It seems there is no other way to get them (pfn walker could, 
but the page lock or freeze will abort the pfn walker process), so it 
appears to be safe to change the 'uptodate' flag. Anyway, let me still 
do some investigation.

> Those my initial reactions.

Thanks for comments.

>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index ce130225a8e5..2ccb442525d1 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3546,6 +3546,18 @@ static int __split_unmapped_folio(struct folio *folio,
>> int new_order,
>>                          lru_add_split_folio(origin_folio, release, lruvec,
>>                                          list);
>>
>> +                       /*
>> +                        * fallocate() will keep folios allocated beyond EOF,
>> we should
>> +                        * clear the uptodate flag for these folios to re-zero
>> them
>> +                        * if necessary.
>> +                        */
>> +                       if (shmem_mapping(mapping)) {
>> +                               loff_t i_size = i_size_read(mapping->host);
>> +
>> +                               if (i_size < end && release->index >= i_size)
>> +                                       folio_clear_uptodate(release);
>> +                       }
>> +
>>                          /* Some pages can be beyond EOF: drop them from cache
>> */
>>                          if (release->index >= end) {
>>                                  if (shmem_mapping(mapping))