Re: [PATCH 02/11] readahead: push invalidate_lock out of page_cache_ra_unbounded

[Christoph Hellwig <hch@lst.de>]

On Mon, Feb 02, 2026 at 03:11:45PM +0000, Matthew Wilcox wrote:
> On Mon, Feb 02, 2026 at 07:06:31AM +0100, Christoph Hellwig wrote:
> > +++ b/fs/f2fs/file.c
> > @@ -4418,7 +4418,9 @@ static int redirty_blocks(struct inode *inode, pgoff_t page_idx, int len)
> >  	pgoff_t redirty_idx = page_idx;
> >  	int page_len = 0, ret = 0;
> >  
> > +	filemap_invalidate_lock_shared(mapping);
> >  	page_cache_ra_unbounded(&ractl, len, 0);
> > +	filemap_invalidate_unlock_shared(mapping);
> 
> Why is f2fs calling page_cache_ra_unbounded() here?

From tracing the callers is seems to be able to be called from the
garbage collector, which might have to move fsverity files.  Not sure if
that was the reason or is incidental.

(using the pagecache for GC is generally a very bad idea, and there is
at least one academic paper showing it is a huge performance problem in
f2fs, and my initial attempts at using the pagecache for GC in zoned XFS
also showed horrible results)

> >  	unsigned int nofs = memalloc_nofs_save();
> >  
> > +	lockdep_assert_held_read(&mapping->invalidate_lock);
> 
> Hm, why are we asserting that it's not write-locked?  For the
> purposes of this function, I'd think we want to just
> lockdep_assert_held()?

Fine with me.

> In the tree I'm looking at, there are also calls to
> page_cache_ra_unbounded() in fs/ext4/verity.c and fs/f2fs/verity.c
> which probably need the lock taken too?

I consolidated those into the single call in fs/verity/pagecache.c
in the previous iteration of this series, and Eric merged the
first few patches including that one into the fsverity tree.