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

[Jan Kara]

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

								Honza

[1] https://lore.kernel.org/all/20260911081309.14137-1-jack@xxxxxxx

-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR