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

[Hugh Dickins <hughd@google.com>]

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).

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.

Those my initial reactions.
Hugh

> 
> 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))