lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250825-zellteilung-investieren-90b030b10963@brauner>
Date: Mon, 25 Aug 2025 13:53:23 +0200
From: Christian Brauner <brauner@...nel.org>
To: Josef Bacik <josef@...icpanda.com>
Cc: linux-fsdevel@...r.kernel.org, linux-btrfs@...r.kernel.org, 
	kernel-team@...com, linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org, 
	viro@...iv.linux.org.uk
Subject: Re: [PATCH 49/50] fs: remove I_FREEING|I_WILL_FREE

On Thu, Aug 21, 2025 at 04:19:00PM -0400, Josef Bacik wrote:
> Now that we're using the i_count reference count as the ultimate arbiter
> of whether or not an inode is life we can remove the I_FREEING and
> I_WILL_FREE flags.
> 
> Signed-off-by: Josef Bacik <josef@...icpanda.com>
> ---

Very nice.

>  fs/inode.c                       |  8 ++------
>  include/linux/fs.h               | 32 +++++++++++---------------------
>  include/trace/events/writeback.h |  2 --
>  3 files changed, 13 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index f715504778d2..1bb528405b3d 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -878,7 +878,7 @@ void clear_inode(struct inode *inode)
>  	BUG_ON(inode->i_state & I_CLEAR);
>  	BUG_ON(!list_empty(&inode->i_wb_list));
>  	/* don't need i_lock here, no concurrent mods to i_state */
> -	inode->i_state = I_FREEING | I_CLEAR;
> +	inode->i_state = I_CLEAR;
>  }
>  EXPORT_SYMBOL(clear_inode);
>  
> @@ -942,7 +942,7 @@ static void evict(struct inode *inode)
>  	 * This also means we don't need any fences for the call below.
>  	 */
>  	inode_wake_up_bit(inode, __I_NEW);
> -	BUG_ON(inode->i_state != (I_FREEING | I_CLEAR));
> +	BUG_ON(inode->i_state != I_CLEAR);
>  }
>  
>  static void iput_evict(struct inode *inode);
> @@ -1975,7 +1975,6 @@ static void iput_final(struct inode *inode, bool drop)
>  
>  	state = inode->i_state;
>  	if (!drop) {
> -		WRITE_ONCE(inode->i_state, state | I_WILL_FREE);
>  		spin_unlock(&inode->i_lock);
>  
>  		write_inode_now(inode, 1);
> @@ -1983,10 +1982,7 @@ static void iput_final(struct inode *inode, bool drop)
>  		spin_lock(&inode->i_lock);
>  		state = inode->i_state;
>  		WARN_ON(state & I_NEW);
> -		state &= ~I_WILL_FREE;
>  	}
> -
> -	WRITE_ONCE(inode->i_state, state | I_FREEING);
>  	spin_unlock(&inode->i_lock);
>  
>  	evict(inode);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9d9acbea6433..0599faef0d6a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -672,8 +672,8 @@ is_uncached_acl(struct posix_acl *acl)
>   * I_DIRTY_DATASYNC, I_DIRTY_PAGES, and I_DIRTY_TIME.
>   *
>   * Four bits define the lifetime of an inode.  Initially, inodes are I_NEW,
> - * until that flag is cleared.  I_WILL_FREE, I_FREEING and I_CLEAR are set at
> - * various stages of removing an inode.
> + * until that flag is cleared.  I_CLEAR is set when the inode is clean and ready
> + * to be freed.
>   *
>   * Two bits are used for locking and completion notification, I_NEW and I_SYNC.
>   *
> @@ -697,24 +697,18 @@ is_uncached_acl(struct posix_acl *acl)
>   *			New inodes set I_NEW.  If two processes both create
>   *			the same inode, one of them will release its inode and
>   *			wait for I_NEW to be released before returning.
> - *			Inodes in I_WILL_FREE, I_FREEING or I_CLEAR state can
> - *			also cause waiting on I_NEW, without I_NEW actually
> - *			being set.  find_inode() uses this to prevent returning
> + *			Inodes with an i_count == 0 or I_CLEAR state can also
> + *			cause waiting on I_NEW, without I_NEW actually being
> + *			set.  find_inode() uses this to prevent returning
>   *			nearly-dead inodes.
> - * I_WILL_FREE		Must be set when calling write_inode_now() if i_count
> - *			is zero.  I_FREEING must be set when I_WILL_FREE is
> - *			cleared.
> - * I_FREEING		Set when inode is about to be freed but still has dirty
> - *			pages or buffers attached or the inode itself is still
> - *			dirty.
>   * I_CLEAR		Added by clear_inode().  In this state the inode is
> - *			clean and can be destroyed.  Inode keeps I_FREEING.
> + *			clean and can be destroyed.
>   *
> - *			Inodes that are I_WILL_FREE, I_FREEING or I_CLEAR are
> - *			prohibited for many purposes.  iget() must wait for
> - *			the inode to be completely released, then create it
> - *			anew.  Other functions will just ignore such inodes,
> - *			if appropriate.  I_NEW is used for waiting.
> + *			Inodes that have i_count == 0 or I_CLEAR are prohibited
> + *			for many purposes.  iget() must wait for the inode to be
> + *			completely released, then create it anew.  Other
> + *			functions will just ignore such inodes, if appropriate.
> + *			I_NEW is used for waiting.
>   *
>   * I_SYNC		Writeback of inode is running. The bit is set during
>   *			data writeback, and cleared with a wakeup on the bit
> @@ -752,8 +746,6 @@ is_uncached_acl(struct posix_acl *acl)
>   * I_CACHED_LRU		Inode is cached because it is dirty or isn't shrinkable,
>   *			and thus is on the s_cached_inode_lru list.
>   *
> - * Q: What is the difference between I_WILL_FREE and I_FREEING?
> - *
>   * __I_{SYNC,NEW,LRU_ISOLATING} are used to derive unique addresses to wait
>   * upon. There's one free address left.
>   */
> @@ -776,8 +768,6 @@ enum inode_state_bits {
>  	INODE_BIT(I_DIRTY_SYNC),
>  	INODE_BIT(I_DIRTY_DATASYNC),
>  	INODE_BIT(I_DIRTY_PAGES),
> -	INODE_BIT(I_WILL_FREE),
> -	INODE_BIT(I_FREEING),
>  	INODE_BIT(I_CLEAR),
>  	INODE_BIT(I_REFERENCED),
>  	INODE_BIT(I_LINKABLE),
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index 6949329c744a..58ee61f3d91d 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -15,8 +15,6 @@
>  		{I_DIRTY_DATASYNC,	"I_DIRTY_DATASYNC"},	\
>  		{I_DIRTY_PAGES,		"I_DIRTY_PAGES"},	\
>  		{I_NEW,			"I_NEW"},		\
> -		{I_WILL_FREE,		"I_WILL_FREE"},		\
> -		{I_FREEING,		"I_FREEING"},		\
>  		{I_CLEAR,		"I_CLEAR"},		\
>  		{I_SYNC,		"I_SYNC"},		\
>  		{I_DIRTY_TIME,		"I_DIRTY_TIME"},	\
> -- 
> 2.49.0
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ