Re: [PATCH] fs/gfs2: prevent premature evict in prune_icache_sb

[Andreas Gruenbacher]

On Thu, Sep 24, 2026 at 11:53 AM Jan Kara <jack@xxxxxxx> wrote:
> On Wed 23-09-26 23:01:52, Andreas Gruenbacher wrote:
> > On Tue, Sep 22, 2026 at 1:22 PM Jan Kara <jack@xxxxxxx> wrote:
> > > Hello Andreas!
> > >
> > > On Mon 21-09-26 23:17:11, Andreas Gruenbacher wrote:
> > > > Introduce a new I_NOPRUNE i_state flag for preventing prune_icache_sb()
> > > > from pruning specific inodes.
> > > >
> > > > prune_icache_sb() prunes clean inodes under memory pressure ("direct
> > > > reclaim").  An inode is considered clean when none of the i_state flags
> > > > are set; the assumption is that evicting inodes that don't have any
> > > > i_state flags set will be fairly cheap.
> > > >
> > > > Unfortunately, on gfs2, inodes can be clean in the sense that they won't
> > > > require writing back to disk, but they may still have outstanding
> > > > revokes (as indicated by the GLF_LFLUSH inode glock flag).  To evict one
> > > > of those inodes, those outstanding revokes need to be written out first.
> > > >
> > > > This requires flushing the log, which is already an expensive operation.
> > > > When in data=ordered mode, all the ordered data needs to be written out
> > > > before the log can be flushed, which makes things even worse.
> > > >
> > > > As previously discussed [*], we are currently also running into the
> > > > following warning in iomap_writepages() when flushing ordered data:
> > > >
> > > >     /*
> > > >      * Writeback from reclaim context should never happen except in the case
> > > >      * of a VM regression so warn about it and refuse to write the data.
> > > >      */
> > > >     if (WARN_ON_ONCE((current->flags & (PF_MEMALLOC | PF_KSWAPD)) ==
> > > >                     PF_MEMALLOC))
> > > >             return -EIO;
> > > >
> > > > So we need to prevent prune_icache_sb() from evicting inodes that have
> > > > any outstanding revokes.  This patch achieves that by introducing a new
> > > > I_NOPRUNE i_state flag.  When that new flag (or any other flag) is set,
> > > > prune_icache_sb() will skip those inodes.
> > > >
> > > > In the filesystem code, we set and clear the I_NOPRUNE flag in sync with
> > > > the GLF_LFLUSH inode glock flag.
> > > >
> > > > An alternative approach might be to allow filesystems to refuse evicting
> > > > inodes that prune_icache_sb() has already selected.  This could be
> > > > achieved by changing the ->evict_inode super operation to return a
> > > > lru_status code or similar.  prune_icache_sb() would then have to
> > > > resurrect inodes that were already marked I_FREEING and put them back
> > > > onto the lru list.  This approach doesn't seem obviously better than
> > > > introducing I_NOPRUNE, so I haven't pursued this any further.
> > >
> > > So for situations like this I've written a patch set to allow filesystem
> > > to mark inode for deferred reclaim (from a workqueue) [1]. Would that work
> > > for you? It will definitely solve your problems with warnings in memory
> > > allocator.
> > >
> > > I don't want to have two different mechanisms for the same issue so if my
> > > solution doesn't quite work for you, let's discuss how to make it better
> > > :).
> >
> > thanks, this would solve some of my problems. In the gfs2 case, inodes
> > are only in "deferred reclaim" state for a while (while they have
> > outstanding revokes) before becoming directly reclaimable again. So
> > I'd also need a way to clear the I_DEFER_RECLAIM flag again.
>
> Well, that's kind of the case for other filesystems as well. But since
> deferred reclaim is relatively cheap my current thinking was that if
> somebody did something expensive to the inode like allocating blocks
> (that's what triggers this for ext4, not sure what exactly triggers the
> state for GFS2), then we can afford also the additional small cost of
> offloading the reclaim to a workqueue (regardless whether it is really
> needed at the time reclaim actually happens). We just don't want to
> unconditionally offload because if someone is just statting millions of
> inodes on some filesystem, he's basically bound by the speed of reclaim on
> sufficiently fast storage and the additional delay of reclaim and the cost
> of offloading would become visible.

Fair enough, let's see if we can get away without clearing I_DEFER_RECLAIM.

> > The I_DEFER_RECLAIM flag setting and clearing code would be racy and
> > ugly though, so I'm wondering if we can change the ->evict_inode()
> > super operation to return a DIRECT / DEFERRED status code instead. The
> > filesystem could then look at the filesystem-internal state and return
> > how to proceed with the inode. For example, fs2_evict_inode() would
> > probably request a DEFERED evict when GLF_LFLUSH is set, PF_MEMALLOC
> > is set, and PF_KSWAPD is not set.
>
> Right, but I really want filesystems to mess with process reclaim flags as
> little as possible because that smells with layering violation and
> generally tends to be fragile. In some code paths such as iput_final()
> or evict_inodes() I really prefer to be sure that the filesystem just frees
> the inode without having to check details of all the implementations making
> sure they don't do something stupid and offload when they shouldn't.
>
> So if we do something like this, I'd probably prefer explicit flag to
> ->evict() telling that offloading is possible and in that case filesystem
> can return special value meaning it requests offloading and prune_icache()
> would handle that.
>
> Another problem I see with this scheme is that VFS is completely oblivious
> to the number of inodes for which reclaim will be deferred and so there's
> no chance for throttling places that create such inodes when the reclaim is
> getting overloaded.
>
> So overall I'd prefer we avoid all this complexity at least until we have a
> real-world case where the performance impact of unnecessary offloading is
> visible.

Works for me.

Thanks,
Andreas