Re: [PATCH 11/11] fs: Remove aops->writepage

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

Hi, Matthew,

On Tue, 2025-04-01 at 17:26 +0100, Matthew Wilcox wrote:
> On Tue, Mar 18, 2025 at 09:10:38AM +0100, Thomas Hellström wrote:
> > On Mon, 2025-03-17 at 22:30 +0000, Matthew Wilcox wrote:
> > > This patch fixes the compilation problem.  But I don't understand
> > > why
> > > it's messing with the reclaim flag.  Thomas, can you explain?
> > 
> > Hi, Sorry for not responding earlier. The patch that uses
> > writepage()
> > here has been around for quite some time waiting for reviews / acks
> > so
> > I failed to notice that it's going away.
> 
> My turn to be sorry for dropping this conversation ...

Once again my turn. This disappeared in the mail flood. Sorry about
that.

> 
> > Anyway the reclaim flag clearing follows that of pageout() in
> > vmscan.c
> > which was also the case for the i915_gem_shmem.c usage in
> > __shmem_writeback(). My understanding was that if the writeback was
> > already completed at that point, the reclaim flag was no longer
> > desirable.
> 
> I think the question is really why you're setting it in the first
> place.
> Setting the reclaim flag indicates that you want the folio removed
> from
> the page cache as soon as possible. 

So when the shmem swapout has been called, My understanding was that
the page had been moved from the page cache to the swap cache and now
written out to disc and this is all part of reclaim.

When TTM reaches this part of the code, it's always called from a
shrinker with the aim of freeing up memory as soon as possible.

So if this is incorrect or unsuitable usage of the reclaim flag, we
should of course remove the manipulation of it. (IIRC I was also a bit
confused as to why it didn't seem to be protected by a lock in the
callsites I looked at) 

__shmem_writeback() in i915_gem_shmem.c and
pageout() in mm/vmscan.c

Thanks,
Thomas



>  Other changes in flight are about
> to make this more aggressive --  instead of waiting for the folio to
> reach the end of the writeout queue, it'll be removed upon I/O
> completion.
> 
> It doesn't seem to me that this is what you actually want for TTM,
> but perhaps I've misunderstood the intent of the code.