On 2025/9/5 00:15, Mark Tinguely wrote: > On 9/4/25 10:42 AM, Mateusz Guzik wrote: >> This postpones the writeout to ocfs2_evict_inode(), which I'm told is >> fine (tm). >> >> The intent is to retire the I_WILL_FREE flag. >> >> Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx> >> --- >> >> ACHTUNG: only compile-time tested. Need an ocfs2 person to ack it. >> >> btw grep shows comments referencing ocfs2_drop_inode() which are already >> stale on the stock kernel, I opted to not touch them. >> >> This ties into an effort to remove the I_WILL_FREE flag, unblocking >> other work. If accepted would be probably best taken through vfs >> branches with said work, see https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/log/?h=vfs-6.18.inode.refcount.preliminaries__;!!ACWV5N9M2RV99hQ!OLwk8DVo7uvC-Pd6XVTiUCgP6MUDMKBMEyuV27h_yPGXOjaq078-kMdC9ILFoYQh-4WX93yb0nMfBDFFY_0$ >> >> fs/ocfs2/inode.c | 23 ++--------------------- >> fs/ocfs2/inode.h | 1 - >> fs/ocfs2/ocfs2_trace.h | 2 -- >> fs/ocfs2/super.c | 2 +- >> 4 files changed, 3 insertions(+), 25 deletions(-) >> >> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c >> index 6c4f78f473fb..5f4a2cbc505d 100644 >> --- a/fs/ocfs2/inode.c >> +++ b/fs/ocfs2/inode.c >> @@ -1290,6 +1290,8 @@ static void ocfs2_clear_inode(struct inode *inode) >> void ocfs2_evict_inode(struct inode *inode) >> { >> + write_inode_now(inode, 1); >> + >> if (!inode->i_nlink || >> (OCFS2_I(inode)->ip_flags & OCFS2_INODE_MAYBE_ORPHANED)) { >> ocfs2_delete_inode(inode); >> @@ -1299,27 +1301,6 @@ void ocfs2_evict_inode(struct inode *inode) >> ocfs2_clear_inode(inode); >> } >> -/* Called under inode_lock, with no more references on the >> - * struct inode, so it's safe here to check the flags field >> - * and to manipulate i_nlink without any other locks. */ >> -int ocfs2_drop_inode(struct inode *inode) >> -{ >> - struct ocfs2_inode_info *oi = OCFS2_I(inode); >> - >> - trace_ocfs2_drop_inode((unsigned long long)oi->ip_blkno, >> - inode->i_nlink, oi->ip_flags); >> - >> - assert_spin_locked(&inode->i_lock); >> - inode->i_state |= I_WILL_FREE; >> - spin_unlock(&inode->i_lock); >> - write_inode_now(inode, 1); >> - spin_lock(&inode->i_lock); >> - WARN_ON(inode->i_state & I_NEW); >> - inode->i_state &= ~I_WILL_FREE; >> - >> - return 1; >> -} >> - >> /* >> * This is called from our getattr. >> */ >> diff --git a/fs/ocfs2/inode.h b/fs/ocfs2/inode.h >> index accf03d4765e..07bd838e7843 100644 >> --- a/fs/ocfs2/inode.h >> +++ b/fs/ocfs2/inode.h >> @@ -116,7 +116,6 @@ static inline struct ocfs2_caching_info *INODE_CACHE(struct inode *inode) >> } >> void ocfs2_evict_inode(struct inode *inode); >> -int ocfs2_drop_inode(struct inode *inode); >> /* Flags for ocfs2_iget() */ >> #define OCFS2_FI_FLAG_SYSFILE 0x1 >> diff --git a/fs/ocfs2/ocfs2_trace.h b/fs/ocfs2/ocfs2_trace.h >> index 54ed1495de9a..4b32fb5658ad 100644 >> --- a/fs/ocfs2/ocfs2_trace.h >> +++ b/fs/ocfs2/ocfs2_trace.h >> @@ -1569,8 +1569,6 @@ DEFINE_OCFS2_ULL_ULL_UINT_EVENT(ocfs2_delete_inode); >> DEFINE_OCFS2_ULL_UINT_EVENT(ocfs2_clear_inode); >> -DEFINE_OCFS2_ULL_UINT_UINT_EVENT(ocfs2_drop_inode); >> - >> TRACE_EVENT(ocfs2_inode_revalidate, >> TP_PROTO(void *inode, unsigned long long ino, >> unsigned int flags), >> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c >> index 53daa4482406..e4b0d25f4869 100644 >> --- a/fs/ocfs2/super.c >> +++ b/fs/ocfs2/super.c >> @@ -129,7 +129,7 @@ static const struct super_operations ocfs2_sops = { >> .statfs = ocfs2_statfs, >> .alloc_inode = ocfs2_alloc_inode, >> .free_inode = ocfs2_free_inode, >> - .drop_inode = ocfs2_drop_inode, >> + .drop_inode = generic_delete_inode, >> .evict_inode = ocfs2_evict_inode, >> .sync_fs = ocfs2_sync_fs, >> .put_super = ocfs2_put_super, > > > I agree, fileystems should not use I_FREEING/I_WILL_FREE. > Doing the sync write_inode_now() should be fine in ocfs_evict_inode(). > > Question is ocfs_drop_inode. In commit 513e2dae9422: > ocfs2: flush inode data to disk and free inode when i_count becomes zero > the return of 1 drops immediate to fix a memory caching issue. > Shouldn't .drop_inode() still return 1? > I think commit 513e2dae9422 only expected the write_inode_now() is determinately called in iput_final(), no matter drop_inode() return 0 or 1. Thanks, Joseph
