Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Jan Kara]

On Fri 19-06-26 14:24:51, Zhou, Yun wrote:
> On 6/18/2026 2:42 AM, Jan Kara wrote:
> > > +static void ext4_xattr_inode_array_free_deferred(struct super_block *sb,
> > > +                             struct ext4_xattr_inode_array *array)
> > 
> > The array of EA inodes used in xattr handling is just another mechanism
> > used for delaying iput() of EA inodes. It doesn't make sense to stack these
> > to one on top of another. Just completely replace the array mechanism with
> > always deferring iput of EA inode into the workqueue.
> > 
> I'm thinking that a complete replacement might be too large a change. Should
> we consider postponing this work, or perhaps appending a new patch to this
> series to handle it?

Do one patch to implement the new framework for delayed EA iputs. Then
maybe one patch to convert existing iputs() of EA inodes to the new
delaying framework and then one patch to convert users of the current
'array to release' mechanism to the new delaying framework.

> > Allocating ext4_ea_iput_entry for dropping each inode is somewhat wasteful.
> > I want to suggest another scheme (somewhat more involved but more efficient
> > scheme):
> > 
> > 1) Create a VFS helper bool iput_if_not_last(struct inode *inode) which
> > drops inode reference if it is not the last one (and returns true in that
> > case). Basically:
> > 
> > bool iput_if_not_last(struct inode *inode)
> > {
> >          return atomic_add_unless(&inode->i_count, -1, 1);
> > }
> > 
> > This needs to be a separate patch as it should get vetting from VFS
> > maintainers.
> > 
> > 2) Use iput_if_not_last() in ext4_put_ea_inode(). If it returns true, we
> > are done. Otherwise we know we were at least for a moment holders of the
> > last inode reference, so we link the inode to the list of inodes to drop
> > through llist_node embedded in ext4_inode_info. We cannot race with anybody
> > else trying to link the same inode into the list because we hold one inode
> > ref and so nobody else can hit this "I was holding the last ref" path.
> > I'd union this llist_node say with xattr_sem which is unused for EA inodes
> > to avoid growing ext4_inode_info.
> > 
> > This way we avoid offloading unless really necessary and we don't have to
> > do allocations just to drop EA inode ref.
> 
> Your idea makes a lot of sense. It greatly simplifies the current deferred
> iput logic and eliminates the risk of failing to allocate an entry during
> an OOM. However, as you mentioned, getting the VFS maintainers to agree
> might be quite challenging.

VFS maintainers don't bite, in the worst case they'd disagree :) For a fact
I'm one of VFS reviewers and I think iput_if_not_last() idea is an
acceptable one. So I think it's worth trying.

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