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

[Hugh Dickins <hughd@google.com>]

On Thu, 10 Jul 2025, Brian Foster wrote:
> On Wed, Jul 09, 2025 at 12:57:35AM -0700, 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.
> > 
> > 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.
> > 
> 
> So I was reading through some of this code yesterday and playing around
> with forcing swapout of an EOF folio, and I had the same observation as
> noted in Baolin's followup: it looks like large folios are always split
> across EOF, at which point post-eof folios are dropped because generally
> the pagecache doesn't track post-eof folios.

Generally yes, but as in reply to Baolin, not in the fallocend case.

> 
> A quick experiment to map write to a 2MB folio in a 1MB sized file shows
> that the folio is indeed split on swapout. It looks like the large folio
> is not immediately reconstructed on swapin, but rather something in the
> background reconstitutes it such that once the post-eof range is
> accessible again, it is effectively zeroed. I'm assuming this is not due
> to explicit zeroing, but rather a side effect of those post-eof folios
> being tossed (vs. swapped) and reallocated, but I could certainly be
> wrong about that.

Sounds right: that will have been khugepaged reconstituting it.

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

I have to confess that in the meantime I've grown rather to think I was
too obsessed without doing it at swapout, and this "ugly" solution better.
But I expect you'll try it out (in mind or in code) one way and the other,
and make your own decision which way is better.

A realization which pushed me in this direction, not decisive but a push:
there can be other reasons for the huge page getting split, not just
swapout and swapin.  Notably (only?) hole-punch somewhere in that
EOF-spanning huge page.  Could be before the EOF or after: in either
case the huge page is (likely to be) split into small pages, and
shmem_zero_eof()'s folio_size(folio) give too small an estimate of
what might need zeroing.

That could be fixed with a further shmem_zero_eof() call somewhere
in the hole-punching path; but that won't be necessary if
shmem_zero_eof() knows to go beyond small folio size (in the
fallocend case only? perhaps, but I haven't thought it through).

> > 
> > The solution I've had in mind (and pursue in comments below) is to do
> > the EOF zeroing in shmem_writeout() before it splits; and then avoid
> > swapin in shmem_zero_eof() when i_size is raised.
> > 
> 
> Indeed, this is similar to traditional writeback behavior in that fully
> post-eof folios are skipped (presumed to be racing with a truncate) and
> a straddling EOF folio is partially zeroed at writeback time.
> 
> I actually observed this difference in behavior when first looking into
> this issue on tmpfs, but I didn't have enough context to draw the
> parallel to swapout, so thanks for bringing this up.
> 
> > That solution partly inspired by the shmem symlink uninit-value bug
> > https://lore.kernel.org/linux-mm/670793eb.050a0220.8109b.0003.GAE@google.com/
> > which I haven't rushed to fix, but ought to be fixed along with this one
> > (by "along with" I don't mean that both have to be fixed in one single
> > patch, but it makes sense to consider them together).  I was inclined
> > not to zero the whole page in shmem_symlink(), but zero before swapout.
> > 
> 
> I'll have to take a closer look at that one..

And I've grown towards thinking (as I expect everybody else would) that
we should simply zero the rest of the page at shmem_symlink() time.
"An abundance of caution" makes me afraid to add the overhead there,
but maybe the right thing to do is the obvious thing, and make it
more complicated if/when anyone notices and complains.

> 
> > It worries me that an EOF page might be swapped out and in a thousand
> > times, but i_size set only once: I'm going for a solution which memsets
> > a thousand times rather than once?  But if that actually shows up as an
> > issue in any workload, then we can add a shmem inode flag to say whether
> > the EOF folio has been exposed via mmap (or symlink) so may need zeroing.
> > 
> > What's your preference?  My comments below assume the latter solution,
> > but that may be wrong.
> > 
> 
> I actually think swapout time zeroing is a nice functional improvement
> over this current patch. I'd be less inclined to think that frequent
> swap cycles are more problematic than swapping folios in (and presumably
> back out) just for partial zeroing due to file changes that don't even
> necessarily need the associated data. This is also generally more
> consistent with traditional fs/pagecache behavior, which IMO is a good
> thing.
> 
> So I suppose what I'm saying is that I like the prospective approach to
> also zero at shmem_writeout() and instead not swap in folios purely for
> partial eof zeroing purposes, just perhaps not for the exact reasons
> stated here. Of course the advantage could be that the code looks the
> same regardless, so if that folio splitting behavior ever changed in the
> future, the zeroing code would hopefully continue to do the right thing.
> Thoughts?

WHereas I liked the way you do it at well-defined user-call times,
rather than when the kernel behind-the-scenes decides it might want
to swap out the page.  But I did check with ext4, and verified there
that the post-EOF non-data vanishes without user-call intervention,
I assume on writeback as expected.  So although I like your choice
of user-call times, it's not a functional requirement at all.

Looks like we've neatly exchanged positions.
After you, Alphonse!

Hugh