[PATCH v2] ocfs2: retire ocfs2_drop_inode() and I_WILL_FREE usage

[PATCH v2] ocfs2: retire ocfs2_drop_inode() and I_WILL_FREE usage

[Mateusz Guzik]

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.

Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---

v2:
- rebase -- generic_delete_inode -> inode_just_drop

The original posting got derailed and then this got lost in the shuffle,
see: https://lore.kernel.org/linux-fsdevel/20250904154245.644875-1-mjguzik@gmail.com/

This is the only filesystem using the flag. The only other spot is in
iput_final().

I have a wip patch to sort out the writeback vs iput situation a little
bit and need this out of the way.

Even if said patch does not go in, this clearly pushes things forward by
removing flag usage.

 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 fcc89856ab95..84115bf8b464 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..2c7ba1480f7a 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	= inode_just_drop,
 	.evict_inode	= ocfs2_evict_inode,
 	.sync_fs	= ocfs2_sync_fs,
 	.put_super	= ocfs2_put_super,
-- 
2.43.0

Re: [External] : [PATCH v2] ocfs2: retire ocfs2_drop_inode() and I_WILL_FREE usage

[Mark Tinguely]

On 10/2/25 9:36 PM, 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.
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> ---
>

I agree this is safe (evict() is done with the I_WILLFREE i_state flag)
and smart to remove I_WILL_FREE i_state from ocfs2.

Reviewed-by: Mark Tinguely <amrk.tinguely@oracle.com>
  
> v2:
> - rebase -- generic_delete_inode -> inode_just_drop
> 
> The original posting got derailed and then this got lost in the shuffle,
> see: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20250904154245.644875-1-mjguzik@gmail.com/__;!!ACWV5N9M2RV99hQ!Khsiz-7t3Akh8BY068JIkK4bVNulsK7SAgJlWzCE-T7Ry_ddsK-Omj0NiJuBy66vtlGZrDLaR33VeeHw1N0$
> 
> This is the only filesystem using the flag. The only other spot is in
> iput_final().
> 
> I have a wip patch to sort out the writeback vs iput situation a little
> bit and need this out of the way.
> 
> Even if said patch does not go in, this clearly pushes things forward by
> removing flag usage.
> 
>   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 fcc89856ab95..84115bf8b464 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..2c7ba1480f7a 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	= inode_just_drop,
>   	.evict_inode	= ocfs2_evict_inode,
>   	.sync_fs	= ocfs2_sync_fs,
>   	.put_super	= ocfs2_put_super,

Re: [PATCH v2] ocfs2: retire ocfs2_drop_inode() and I_WILL_FREE usage

[Joel Becker]

Since this is in `iput_final()`, it's outside of OCFS2 cluster locking.
The only work `ocfs_drop_inode()` does is to juggle the spinlock and
state while writing the inode.  `evict()` does this just a little later
in `iput_final()`, and there's no real way the flow gets interrupted, so
it is not even moving the writeout that far.

Reviewed-by: Joel Becker <jlbec@evilplan.org>

On Fri, Oct 03, 2025 at 04:36:52AM +0200, 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.
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> ---
> 
> v2:
> - rebase -- generic_delete_inode -> inode_just_drop
> 
> The original posting got derailed and then this got lost in the shuffle,
> see: https://lore.kernel.org/linux-fsdevel/20250904154245.644875-1-mjguzik@gmail.com/
> 
> This is the only filesystem using the flag. The only other spot is in
> iput_final().
> 
> I have a wip patch to sort out the writeback vs iput situation a little
> bit and need this out of the way.
> 
> Even if said patch does not go in, this clearly pushes things forward by
> removing flag usage.
> 
>  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 fcc89856ab95..84115bf8b464 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..2c7ba1480f7a 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	= inode_just_drop,
>  	.evict_inode	= ocfs2_evict_inode,
>  	.sync_fs	= ocfs2_sync_fs,
>  	.put_super	= ocfs2_put_super,
> -- 
> 2.43.0
> 
> 

-- 

"Hell is oneself, hell is alone, the other figures in it, merely projections."
        - T. S. Eliot

			http://www.jlbec.org/
			jlbec@evilplan.org

Re: [PATCH v2] ocfs2: retire ocfs2_drop_inode() and I_WILL_FREE usage

[Christian Brauner]

On Fri, 03 Oct 2025 04:36:52 +0200, 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.
> 
> 

Applied to the vfs-6.19.inode branch of the vfs/vfs.git tree.
Patches in the vfs-6.19.inode branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.19.inode

[1/1] ocfs2: retire ocfs2_drop_inode() and I_WILL_FREE usage
      https://git.kernel.org/vfs/vfs/c/20bca90a05e3