On Mon, Feb 02, 2026 at 04:17:55PM +0100, Christoph Hellwig wrote: > 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.
I changed both instances of lockdep_assert_held_read() to lockdep_assert_held() when applying. - Eric _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
