linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Josef Bacik <josef@toxicpanda.com>
Cc: linux-fsdevel@vger.kernel.org, linux-btrfs@vger.kernel.org,
	 kernel-team@fb.com, linux-ext4@vger.kernel.org,
	linux-xfs@vger.kernel.org,  viro@zeniv.linux.org.uk,
	amir73il@gmail.com
Subject: Re: [PATCH v2 22/54] fs: convert i_count to refcount_t
Date: Thu, 28 Aug 2025 14:00:03 +0200	[thread overview]
Message-ID: <20250828-bergkette-umtriebe-921d8f3bdf4e@brauner> (raw)
In-Reply-To: <36a888e4cf47948c01f49ee24ede2b662075fc5e.1756222465.git.josef@toxicpanda.com>

On Tue, Aug 26, 2025 at 11:39:22AM -0400, Josef Bacik wrote:
> Now that we do not allow i_count to drop to 0 and be used we can convert
> it to a refcount_t and benefit from the protections those helpers add.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/inode.c   | 2 +-
>  fs/inode.c         | 9 +++++----
>  include/linux/fs.h | 6 +++---
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index e16df38e0eef..eb9496342346 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3418,7 +3418,7 @@ void btrfs_add_delayed_iput(struct btrfs_inode *inode)
>  	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>  	unsigned long flags;
>  
> -	if (atomic_add_unless(&inode->vfs_inode.i_count, -1, 1)) {
> +	if (refcount_dec_not_one(&inode->vfs_inode.i_count)) {

Now this is the only place outside core VFS where we open-access
i_count. Add a helper and reuse it iput() as well? icount_maybe_dec()?
icount_dec_not_one()?

>  		iobj_put(&inode->vfs_inode);
>  		return;
>  	}
> diff --git a/fs/inode.c b/fs/inode.c
> index 1992db5cd70a..0be1c137bf1e 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -236,7 +236,7 @@ int inode_init_always_gfp(struct super_block *sb, struct inode *inode, gfp_t gfp
>  	inode->i_state = 0;
>  	atomic64_set(&inode->i_sequence, 0);
>  	refcount_set(&inode->i_obj_count, 1);
> -	atomic_set(&inode->i_count, 1);
> +	refcount_set(&inode->i_count, 1);
>  	inode->i_op = &empty_iops;
>  	inode->i_fop = &no_open_fops;
>  	inode->i_ino = 0;
> @@ -545,7 +545,8 @@ static void init_once(void *foo)
>  void ihold(struct inode *inode)
>  {
>  	iobj_get(inode);
> -	WARN_ON(atomic_inc_return(&inode->i_count) < 2);
> +	refcount_inc(&inode->i_count);
> +	WARN_ON(icount_read(inode) < 2);
>  }
>  EXPORT_SYMBOL(ihold);
>  
> @@ -2011,7 +2012,7 @@ static void __iput(struct inode *inode, bool skip_lru)
>  		return;
>  	BUG_ON(inode->i_state & I_CLEAR);
>  
> -	if (atomic_add_unless(&inode->i_count, -1, 1)) {
> +	if (refcount_dec_not_one(&inode->i_count)) {
>  		iobj_put(inode);
>  		return;
>  	}
> @@ -2031,7 +2032,7 @@ static void __iput(struct inode *inode, bool skip_lru)
>  	 */
>  	drop = maybe_add_lru(inode, skip_lru);
>  
> -	if (atomic_dec_and_test(&inode->i_count)) {
> +	if (refcount_dec_and_test(&inode->i_count)) {
>  		/* iput_final() drops i_lock */
>  		iput_final(inode, drop);
>  	} else {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 999ffea2aac1..fc23e37ca250 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -875,7 +875,7 @@ struct inode {
>  	};
>  	atomic64_t		i_version;
>  	atomic64_t		i_sequence; /* see futex */
> -	atomic_t		i_count;
> +	refcount_t		i_count;
>  	atomic_t		i_dio_count;
>  	atomic_t		i_writecount;
>  #if defined(CONFIG_IMA) || defined(CONFIG_FILE_LOCKING)
> @@ -3399,12 +3399,12 @@ static inline unsigned int iobj_count_read(const struct inode *inode)
>  static inline void __iget(struct inode *inode)
>  {
>  	iobj_get(inode);
> -	atomic_inc(&inode->i_count);
> +	refcount_inc(&inode->i_count);
>  }
>  
>  static inline int icount_read(const struct inode *inode)
>  {
> -	return atomic_read(&inode->i_count);
> +	return refcount_read(&inode->i_count);
>  }
>  
>  extern void iget_failed(struct inode *);
> -- 
> 2.49.0
> 

  reply	other threads:[~2025-08-28 12:00 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-26 15:39 [PATCH v2 00/54] fs: rework inode reference counting Josef Bacik
2025-08-26 15:39 ` [PATCH v2 01/54] fs: make the i_state flags an enum Josef Bacik
2025-08-26 15:39 ` [PATCH v2 02/54] fs: add an icount_read helper Josef Bacik
2025-08-26 22:18   ` Mateusz Guzik
2025-08-27 11:25   ` (subset) " Christian Brauner
2025-08-26 15:39 ` [PATCH v2 03/54] fs: rework iput logic Josef Bacik
2025-08-27 12:58   ` Mateusz Guzik
2025-08-27 14:18     ` Mateusz Guzik
2025-08-27 14:54       ` Josef Bacik
2025-08-27 14:57       ` Christian Brauner
2025-08-27 16:24         ` [PATCH] fs: revamp iput() Mateusz Guzik
2025-08-30 15:54           ` Mateusz Guzik
2025-09-01  8:50             ` Jan Kara
2025-09-01 10:39               ` Christian Brauner
2025-09-01 10:41             ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 04/54] fs: add an i_obj_count refcount to the inode Josef Bacik
2025-08-26 15:39 ` [PATCH v2 05/54] fs: hold an i_obj_count reference in wait_sb_inodes Josef Bacik
2025-08-26 15:39 ` [PATCH v2 06/54] fs: hold an i_obj_count reference for the i_wb_list Josef Bacik
2025-08-26 15:39 ` [PATCH v2 07/54] fs: hold an i_obj_count reference for the i_io_list Josef Bacik
2025-08-26 15:39 ` [PATCH v2 08/54] fs: hold an i_obj_count reference in writeback_sb_inodes Josef Bacik
2025-08-26 15:39 ` [PATCH v2 09/54] fs: hold an i_obj_count reference while on the hashtable Josef Bacik
2025-08-26 15:39 ` [PATCH v2 10/54] fs: hold an i_obj_count reference while on the LRU list Josef Bacik
2025-08-26 15:39 ` [PATCH v2 11/54] fs: hold an i_obj_count reference while on the sb inode list Josef Bacik
2025-08-26 15:39 ` [PATCH v2 12/54] fs: stop accessing ->i_count directly in f2fs and gfs2 Josef Bacik
2025-08-26 15:39 ` [PATCH v2 13/54] fs: hold an i_obj_count when we have an i_count reference Josef Bacik
2025-08-26 15:39 ` [PATCH v2 14/54] fs: add an I_LRU flag to the inode Josef Bacik
2025-08-26 15:39 ` [PATCH v2 15/54] fs: maintain a list of pinned inodes Josef Bacik
2025-08-27 15:20   ` Christian Brauner
2025-08-27 16:07     ` Josef Bacik
2025-08-28  8:24       ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 16/54] fs: delete the inode from the LRU list on lookup Josef Bacik
2025-08-27 21:46   ` Dave Chinner
2025-08-28 11:42     ` Josef Bacik
2025-09-02  4:07       ` Dave Chinner
2025-08-26 15:39 ` [PATCH v2 17/54] fs: remove the inode from the LRU list on unlink/rmdir Josef Bacik
2025-08-27 12:32   ` Christian Brauner
2025-08-27 16:08     ` Josef Bacik
2025-08-27 22:01     ` Dave Chinner
2025-08-28 11:46       ` Josef Bacik
2025-09-02  1:48         ` Dave Chinner
2025-08-28  9:00   ` Christian Brauner
2025-08-28  9:06   ` Christian Brauner
2025-08-28 10:43     ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 18/54] fs: change evict_inodes to use iput instead of evict directly Josef Bacik
2025-08-28 10:18   ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 19/54] fs: hold a full ref while the inode is on a LRU Josef Bacik
2025-08-28 10:51   ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 20/54] fs: disallow 0 reference count inodes Josef Bacik
2025-08-28 11:02   ` Christian Brauner
2025-08-28 11:44     ` Josef Bacik
2025-08-26 15:39 ` [PATCH v2 21/54] fs: make evict_inodes add to the dispose list under the i_lock Josef Bacik
2025-08-26 15:39 ` [PATCH v2 22/54] fs: convert i_count to refcount_t Josef Bacik
2025-08-28 12:00   ` Christian Brauner [this message]
2025-08-26 15:39 ` [PATCH v2 23/54] fs: use refcount_inc_not_zero in igrab Josef Bacik
2025-08-28 22:08   ` Eric Biggers
2025-08-29 13:42     ` Josef Bacik
2025-08-26 15:39 ` [PATCH v2 24/54] fs: use inode_tryget in find_inode* Josef Bacik
2025-08-26 15:39 ` [PATCH v2 25/54] fs: update find_inode_*rcu to check the i_count count Josef Bacik
2025-08-26 15:39 ` [PATCH v2 26/54] fs: use igrab in insert_inode_locked Josef Bacik
2025-08-28 12:15   ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 27/54] fs: remove I_WILL_FREE|I_FREEING check from __inode_add_lru Josef Bacik
2025-08-26 15:39 ` [PATCH v2 28/54] fs: remove I_WILL_FREE|I_FREEING check in inode_pin_lru_isolating Josef Bacik
2025-08-26 15:39 ` [PATCH v2 29/54] fs: use inode_tryget in evict_inodes Josef Bacik
2025-08-26 15:39 ` [PATCH v2 30/54] fs: change evict_dentries_for_decrypted_inodes to use refcount Josef Bacik
2025-08-28 12:25   ` Christian Brauner
2025-08-28 22:26     ` Eric Biggers
2025-08-29  7:38       ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 31/54] block: use igrab in sync_bdevs Josef Bacik
2025-08-26 15:39 ` [PATCH v2 32/54] bcachefs: use the refcount instead of I_WILL_FREE|I_FREEING Josef Bacik
2025-08-26 15:39 ` [PATCH v2 33/54] btrfs: don't check I_WILL_FREE|I_FREEING Josef Bacik
2025-08-26 15:39 ` [PATCH v2 34/54] fs: use igrab in drop_pagecache_sb Josef Bacik
2025-08-26 15:39 ` [PATCH v2 35/54] fs: stop checking I_FREEING in d_find_alias_rcu Josef Bacik
2025-08-26 15:39 ` [PATCH v2 36/54] ext4: stop checking I_WILL_FREE|IFREEING in ext4_check_map_extents_env Josef Bacik
2025-08-26 15:39 ` [PATCH v2 37/54] fs: remove I_WILL_FREE|I_FREEING from fs-writeback.c Josef Bacik
2025-08-26 15:39 ` [PATCH v2 38/54] gfs2: remove I_WILL_FREE|I_FREEING usage Josef Bacik
2025-08-26 15:39 ` [PATCH v2 39/54] fs: remove I_WILL_FREE|I_FREEING check from dquot.c Josef Bacik
2025-08-28 12:35   ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 40/54] notify: remove I_WILL_FREE|I_FREEING checks in fsnotify_unmount_inodes Josef Bacik
2025-08-26 15:39 ` [PATCH v2 41/54] xfs: remove I_FREEING check Josef Bacik
2025-08-26 15:39 ` [PATCH v2 42/54] landlock: remove I_FREEING|I_WILL_FREE check Josef Bacik
2025-08-26 15:39 ` [PATCH v2 43/54] fs: change inode_is_dirtytime_only to use refcount Josef Bacik
2025-08-26 22:06   ` Mateusz Guzik
2025-08-28 12:38     ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 44/54] btrfs: remove references to I_FREEING Josef Bacik
2025-08-26 15:39 ` [PATCH v2 45/54] ext4: remove reference to I_FREEING in inode.c Josef Bacik
2025-08-26 15:39 ` [PATCH v2 46/54] ext4: remove reference to I_FREEING in orphan.c Josef Bacik
2025-08-26 15:39 ` [PATCH v2 47/54] pnfs: use i_count refcount to determine if the inode is going away Josef Bacik
2025-08-26 15:39 ` [PATCH v2 48/54] fs: remove some spurious I_FREEING references in inode.c Josef Bacik
2025-08-28 12:40   ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 49/54] xfs: remove reference to I_FREEING|I_WILL_FREE Josef Bacik
2025-08-26 15:39 ` [PATCH v2 50/54] ocfs2: do not set I_WILL_FREE Josef Bacik
2025-08-26 15:39 ` [PATCH v2 51/54] fs: remove I_FREEING|I_WILL_FREE Josef Bacik
2025-08-28 12:42   ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 52/54] fs: remove I_REFERENCED Josef Bacik
2025-08-28 12:47   ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 53/54] fs: remove I_LRU_ISOLATING flag Josef Bacik
2025-08-28  0:26   ` Dave Chinner
2025-08-28 10:35     ` Christian Brauner
2025-08-26 15:39 ` [PATCH v2 54/54] fs: add documentation explaining the reference count rules for inodes Josef Bacik
2025-08-27  8:03 ` [syzbot ci] Re: fs: rework inode reference counting syzbot ci
2025-08-27 11:14 ` (subset) [PATCH v2 00/54] " Christian Brauner
2025-08-28 12:51 ` Christian Brauner
2025-08-28 21:22   ` Josef Bacik
2025-09-02 10:06 ` Mateusz Guzik
2025-09-02 21:16   ` Josef Bacik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250828-bergkette-umtriebe-921d8f3bdf4e@brauner \
    --to=brauner@kernel.org \
    --cc=amir73il@gmail.com \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).