* [PATCH 1/5] fs: add icount_read_once()
2026-03-28 15:31 [PATCH 0/5] assorted ->i_count-related changes Mateusz Guzik
@ 2026-03-28 15:31 ` Mateusz Guzik
2026-03-28 15:31 ` [PATCH] fs: revert insert_inode_locked() eviction wait change and explain why Mateusz Guzik
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Mateusz Guzik @ 2026-03-28 15:31 UTC (permalink / raw)
To: brauner; +Cc: viro, jack, linux-kernel, linux-fsdevel, Mateusz Guzik
It will be used to denote the caller acknowledges how the count can
change from under them.
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
include/linux/fs.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8afbe2ef2686..0cf27085a579 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2225,6 +2225,11 @@ static inline void mark_inode_dirty_sync(struct inode *inode)
__mark_inode_dirty(inode, I_DIRTY_SYNC);
}
+static inline int icount_read_once(const struct inode *inode)
+{
+ return atomic_read(&inode->i_count);
+}
+
static inline int icount_read(const struct inode *inode)
{
return atomic_read(&inode->i_count);
--
2.48.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] fs: revert insert_inode_locked() eviction wait change and explain why
2026-03-28 15:31 [PATCH 0/5] assorted ->i_count-related changes Mateusz Guzik
2026-03-28 15:31 ` [PATCH 1/5] fs: add icount_read_once() Mateusz Guzik
@ 2026-03-28 15:31 ` Mateusz Guzik
2026-03-28 15:33 ` Mateusz Guzik
2026-03-28 15:31 ` [PATCH 2/5] Use icount_read() and icount_read_once() as appropriate Mateusz Guzik
` (3 subsequent siblings)
5 siblings, 1 reply; 8+ messages in thread
From: Mateusz Guzik @ 2026-03-28 15:31 UTC (permalink / raw)
To: brauner; +Cc: viro, jack, linux-kernel, linux-fsdevel, Mateusz Guzik, Lai, Yi
It causes a deadlock, reproducer can be found here:
https://lore.kernel.org/linux-fsdevel/abNvb2PcrKj1FBeC@ly-workstation/
The real bug is in ext4, but I'm not digging into it and a working order
needs to be restored.
Commentary is added as a warning sign for another sucker^Wdeveloper.
Fixes: 88ec797c468097a8 ("fs: make insert_inode_locked() wait for inode destruction")
Reported-by: "Lai, Yi" <yi1.lai@linux.intel.com>
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
fs/inode.c | 53 +++++++++++++++++++++++++++++------------------------
1 file changed, 29 insertions(+), 24 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index cc12b68e021b..5f7e76c9fb53 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1037,20 +1037,19 @@ long prune_icache_sb(struct super_block *sb, struct shrink_control *sc)
return freed;
}
-static void __wait_on_freeing_inode(struct inode *inode, bool hash_locked, bool rcu_locked);
-
+static void __wait_on_freeing_inode(struct inode *inode, bool is_inode_hash_locked);
/*
* Called with the inode lock held.
*/
static struct inode *find_inode(struct super_block *sb,
struct hlist_head *head,
int (*test)(struct inode *, void *),
- void *data, bool hash_locked,
+ void *data, bool is_inode_hash_locked,
bool *isnew)
{
struct inode *inode = NULL;
- if (hash_locked)
+ if (is_inode_hash_locked)
lockdep_assert_held(&inode_hash_lock);
else
lockdep_assert_not_held(&inode_hash_lock);
@@ -1064,7 +1063,7 @@ static struct inode *find_inode(struct super_block *sb,
continue;
spin_lock(&inode->i_lock);
if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE)) {
- __wait_on_freeing_inode(inode, hash_locked, true);
+ __wait_on_freeing_inode(inode, is_inode_hash_locked);
goto repeat;
}
if (unlikely(inode_state_read(inode) & I_CREATING)) {
@@ -1088,11 +1087,11 @@ static struct inode *find_inode(struct super_block *sb,
*/
static struct inode *find_inode_fast(struct super_block *sb,
struct hlist_head *head, unsigned long ino,
- bool hash_locked, bool *isnew)
+ bool is_inode_hash_locked, bool *isnew)
{
struct inode *inode = NULL;
- if (hash_locked)
+ if (is_inode_hash_locked)
lockdep_assert_held(&inode_hash_lock);
else
lockdep_assert_not_held(&inode_hash_lock);
@@ -1106,7 +1105,7 @@ static struct inode *find_inode_fast(struct super_block *sb,
continue;
spin_lock(&inode->i_lock);
if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE)) {
- __wait_on_freeing_inode(inode, hash_locked, true);
+ __wait_on_freeing_inode(inode, is_inode_hash_locked);
goto repeat;
}
if (unlikely(inode_state_read(inode) & I_CREATING)) {
@@ -1842,13 +1841,28 @@ int insert_inode_locked(struct inode *inode)
while (1) {
struct inode *old = NULL;
spin_lock(&inode_hash_lock);
-repeat:
hlist_for_each_entry(old, head, i_hash) {
if (old->i_ino != ino)
continue;
if (old->i_sb != sb)
continue;
spin_lock(&old->i_lock);
+ /*
+ * FIXME: inodes awaiting eviction don't get waited for
+ *
+ * This is a bug because the hash can temporarily end up with duplicate inodes.
+ * It happens to work becuase new inodes are inserted at the beginning of the
+ * chain, meaning they will be found first should anyone do a lookup.
+ *
+ * Fixing the above results in deadlocks in ext4 due to journal handling during
+ * inode creation and eviction -- the eviction side waits for creation side to
+ * finish. Adding __wait_on_freeing_inode results in both sides waiting on each
+ * other.
+ */
+ if (inode_state_read(old) & (I_FREEING | I_WILL_FREE)) {
+ spin_unlock(&old->i_lock);
+ continue;
+ }
break;
}
if (likely(!old)) {
@@ -1859,11 +1873,6 @@ int insert_inode_locked(struct inode *inode)
spin_unlock(&inode_hash_lock);
return 0;
}
- if (inode_state_read(old) & (I_FREEING | I_WILL_FREE)) {
- __wait_on_freeing_inode(old, true, false);
- old = NULL;
- goto repeat;
- }
if (unlikely(inode_state_read(old) & I_CREATING)) {
spin_unlock(&old->i_lock);
spin_unlock(&inode_hash_lock);
@@ -2534,18 +2543,16 @@ EXPORT_SYMBOL(inode_needs_sync);
* wake_up_bit(&inode->i_state, __I_NEW) after removing from the hash list
* will DTRT.
*/
-static void __wait_on_freeing_inode(struct inode *inode, bool hash_locked, bool rcu_locked)
+static void __wait_on_freeing_inode(struct inode *inode, bool is_inode_hash_locked)
{
struct wait_bit_queue_entry wqe;
struct wait_queue_head *wq_head;
- VFS_BUG_ON(!hash_locked && !rcu_locked);
-
/*
* Handle racing against evict(), see that routine for more details.
*/
if (unlikely(inode_unhashed(inode))) {
- WARN_ON(hash_locked);
+ WARN_ON(is_inode_hash_locked);
spin_unlock(&inode->i_lock);
return;
}
@@ -2553,16 +2560,14 @@ static void __wait_on_freeing_inode(struct inode *inode, bool hash_locked, bool
wq_head = inode_bit_waitqueue(&wqe, inode, __I_NEW);
prepare_to_wait_event(wq_head, &wqe.wq_entry, TASK_UNINTERRUPTIBLE);
spin_unlock(&inode->i_lock);
- if (rcu_locked)
- rcu_read_unlock();
- if (hash_locked)
+ rcu_read_unlock();
+ if (is_inode_hash_locked)
spin_unlock(&inode_hash_lock);
schedule();
finish_wait(wq_head, &wqe.wq_entry);
- if (hash_locked)
+ if (is_inode_hash_locked)
spin_lock(&inode_hash_lock);
- if (rcu_locked)
- rcu_read_lock();
+ rcu_read_lock();
}
static __initdata unsigned long ihash_entries;
--
2.48.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] fs: revert insert_inode_locked() eviction wait change and explain why
2026-03-28 15:31 ` [PATCH] fs: revert insert_inode_locked() eviction wait change and explain why Mateusz Guzik
@ 2026-03-28 15:33 ` Mateusz Guzik
0 siblings, 0 replies; 8+ messages in thread
From: Mateusz Guzik @ 2026-03-28 15:33 UTC (permalink / raw)
To: brauner; +Cc: viro, jack, linux-kernel, linux-fsdevel, Lai, Yi
oops, sorry guys. this one snuck in with bad globbing in the shell
On Sat, Mar 28, 2026 at 4:32 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> It causes a deadlock, reproducer can be found here:
> https://lore.kernel.org/linux-fsdevel/abNvb2PcrKj1FBeC@ly-workstation/
>
> The real bug is in ext4, but I'm not digging into it and a working order
> needs to be restored.
>
> Commentary is added as a warning sign for another sucker^Wdeveloper.
>
> Fixes: 88ec797c468097a8 ("fs: make insert_inode_locked() wait for inode destruction")
> Reported-by: "Lai, Yi" <yi1.lai@linux.intel.com>
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> ---
> fs/inode.c | 53 +++++++++++++++++++++++++++++------------------------
> 1 file changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index cc12b68e021b..5f7e76c9fb53 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1037,20 +1037,19 @@ long prune_icache_sb(struct super_block *sb, struct shrink_control *sc)
> return freed;
> }
>
> -static void __wait_on_freeing_inode(struct inode *inode, bool hash_locked, bool rcu_locked);
> -
> +static void __wait_on_freeing_inode(struct inode *inode, bool is_inode_hash_locked);
> /*
> * Called with the inode lock held.
> */
> static struct inode *find_inode(struct super_block *sb,
> struct hlist_head *head,
> int (*test)(struct inode *, void *),
> - void *data, bool hash_locked,
> + void *data, bool is_inode_hash_locked,
> bool *isnew)
> {
> struct inode *inode = NULL;
>
> - if (hash_locked)
> + if (is_inode_hash_locked)
> lockdep_assert_held(&inode_hash_lock);
> else
> lockdep_assert_not_held(&inode_hash_lock);
> @@ -1064,7 +1063,7 @@ static struct inode *find_inode(struct super_block *sb,
> continue;
> spin_lock(&inode->i_lock);
> if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE)) {
> - __wait_on_freeing_inode(inode, hash_locked, true);
> + __wait_on_freeing_inode(inode, is_inode_hash_locked);
> goto repeat;
> }
> if (unlikely(inode_state_read(inode) & I_CREATING)) {
> @@ -1088,11 +1087,11 @@ static struct inode *find_inode(struct super_block *sb,
> */
> static struct inode *find_inode_fast(struct super_block *sb,
> struct hlist_head *head, unsigned long ino,
> - bool hash_locked, bool *isnew)
> + bool is_inode_hash_locked, bool *isnew)
> {
> struct inode *inode = NULL;
>
> - if (hash_locked)
> + if (is_inode_hash_locked)
> lockdep_assert_held(&inode_hash_lock);
> else
> lockdep_assert_not_held(&inode_hash_lock);
> @@ -1106,7 +1105,7 @@ static struct inode *find_inode_fast(struct super_block *sb,
> continue;
> spin_lock(&inode->i_lock);
> if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE)) {
> - __wait_on_freeing_inode(inode, hash_locked, true);
> + __wait_on_freeing_inode(inode, is_inode_hash_locked);
> goto repeat;
> }
> if (unlikely(inode_state_read(inode) & I_CREATING)) {
> @@ -1842,13 +1841,28 @@ int insert_inode_locked(struct inode *inode)
> while (1) {
> struct inode *old = NULL;
> spin_lock(&inode_hash_lock);
> -repeat:
> hlist_for_each_entry(old, head, i_hash) {
> if (old->i_ino != ino)
> continue;
> if (old->i_sb != sb)
> continue;
> spin_lock(&old->i_lock);
> + /*
> + * FIXME: inodes awaiting eviction don't get waited for
> + *
> + * This is a bug because the hash can temporarily end up with duplicate inodes.
> + * It happens to work becuase new inodes are inserted at the beginning of the
> + * chain, meaning they will be found first should anyone do a lookup.
> + *
> + * Fixing the above results in deadlocks in ext4 due to journal handling during
> + * inode creation and eviction -- the eviction side waits for creation side to
> + * finish. Adding __wait_on_freeing_inode results in both sides waiting on each
> + * other.
> + */
> + if (inode_state_read(old) & (I_FREEING | I_WILL_FREE)) {
> + spin_unlock(&old->i_lock);
> + continue;
> + }
> break;
> }
> if (likely(!old)) {
> @@ -1859,11 +1873,6 @@ int insert_inode_locked(struct inode *inode)
> spin_unlock(&inode_hash_lock);
> return 0;
> }
> - if (inode_state_read(old) & (I_FREEING | I_WILL_FREE)) {
> - __wait_on_freeing_inode(old, true, false);
> - old = NULL;
> - goto repeat;
> - }
> if (unlikely(inode_state_read(old) & I_CREATING)) {
> spin_unlock(&old->i_lock);
> spin_unlock(&inode_hash_lock);
> @@ -2534,18 +2543,16 @@ EXPORT_SYMBOL(inode_needs_sync);
> * wake_up_bit(&inode->i_state, __I_NEW) after removing from the hash list
> * will DTRT.
> */
> -static void __wait_on_freeing_inode(struct inode *inode, bool hash_locked, bool rcu_locked)
> +static void __wait_on_freeing_inode(struct inode *inode, bool is_inode_hash_locked)
> {
> struct wait_bit_queue_entry wqe;
> struct wait_queue_head *wq_head;
>
> - VFS_BUG_ON(!hash_locked && !rcu_locked);
> -
> /*
> * Handle racing against evict(), see that routine for more details.
> */
> if (unlikely(inode_unhashed(inode))) {
> - WARN_ON(hash_locked);
> + WARN_ON(is_inode_hash_locked);
> spin_unlock(&inode->i_lock);
> return;
> }
> @@ -2553,16 +2560,14 @@ static void __wait_on_freeing_inode(struct inode *inode, bool hash_locked, bool
> wq_head = inode_bit_waitqueue(&wqe, inode, __I_NEW);
> prepare_to_wait_event(wq_head, &wqe.wq_entry, TASK_UNINTERRUPTIBLE);
> spin_unlock(&inode->i_lock);
> - if (rcu_locked)
> - rcu_read_unlock();
> - if (hash_locked)
> + rcu_read_unlock();
> + if (is_inode_hash_locked)
> spin_unlock(&inode_hash_lock);
> schedule();
> finish_wait(wq_head, &wqe.wq_entry);
> - if (hash_locked)
> + if (is_inode_hash_locked)
> spin_lock(&inode_hash_lock);
> - if (rcu_locked)
> - rcu_read_lock();
> + rcu_read_lock();
> }
>
> static __initdata unsigned long ihash_entries;
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/5] Use icount_read() and icount_read_once() as appropriate.
2026-03-28 15:31 [PATCH 0/5] assorted ->i_count-related changes Mateusz Guzik
2026-03-28 15:31 ` [PATCH 1/5] fs: add icount_read_once() Mateusz Guzik
2026-03-28 15:31 ` [PATCH] fs: revert insert_inode_locked() eviction wait change and explain why Mateusz Guzik
@ 2026-03-28 15:31 ` Mateusz Guzik
2026-03-28 15:31 ` [PATCH 3/5] fs: enforce locking in icount_read(), add some commentary Mateusz Guzik
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Mateusz Guzik @ 2026-03-28 15:31 UTC (permalink / raw)
To: brauner; +Cc: viro, jack, linux-kernel, linux-fsdevel, Mateusz Guzik
Don't open-code ->i_count access.
This is expected to be a nop.
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
arch/powerpc/platforms/cell/spufs/file.c | 2 +-
fs/btrfs/inode.c | 2 +-
fs/ceph/mds_client.c | 2 +-
fs/ext4/ialloc.c | 4 ++--
fs/hpfs/inode.c | 2 +-
fs/inode.c | 12 ++++++------
fs/nfs/inode.c | 4 ++--
fs/smb/client/inode.c | 2 +-
fs/ubifs/super.c | 2 +-
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_trace.h | 2 +-
include/trace/events/filelock.h | 2 +-
security/landlock/fs.c | 2 +-
13 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index 10fa9b844fcc..f6de8c1169d5 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -1430,7 +1430,7 @@ static int spufs_mfc_open(struct inode *inode, struct file *file)
if (ctx->owner != current->mm)
return -EINVAL;
- if (icount_read(inode) != 1)
+ if (icount_read_once(inode) != 1)
return -EBUSY;
mutex_lock(&ctx->mapping_lock);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 8d97a8ad3858..f36c49e83c04 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4741,7 +4741,7 @@ static void btrfs_prune_dentries(struct btrfs_root *root)
inode = btrfs_find_first_inode(root, min_ino);
while (inode) {
- if (icount_read(&inode->vfs_inode) > 1)
+ if (icount_read_once(&inode->vfs_inode) > 1)
d_prune_aliases(&inode->vfs_inode);
min_ino = btrfs_ino(inode) + 1;
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index b1746273f186..2cb3c919d40d 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2223,7 +2223,7 @@ static int trim_caps_cb(struct inode *inode, int mds, void *arg)
int count;
dput(dentry);
d_prune_aliases(inode);
- count = icount_read(inode);
+ count = icount_read_once(inode);
if (count == 1)
(*remaining)--;
doutc(cl, "%p %llx.%llx cap %p pruned, count now %d\n",
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 3fd8f0099852..8c80d5087516 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -252,10 +252,10 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
"nonexistent device\n", __func__, __LINE__);
return;
}
- if (icount_read(inode) > 1) {
+ if (icount_read_once(inode) > 1) {
ext4_msg(sb, KERN_ERR, "%s:%d: inode #%llu: count=%d",
__func__, __LINE__, inode->i_ino,
- icount_read(inode));
+ icount_read_once(inode));
return;
}
if (inode->i_nlink) {
diff --git a/fs/hpfs/inode.c b/fs/hpfs/inode.c
index 0e932cc8be1b..1b4fcf760aad 100644
--- a/fs/hpfs/inode.c
+++ b/fs/hpfs/inode.c
@@ -184,7 +184,7 @@ void hpfs_write_inode(struct inode *i)
struct hpfs_inode_info *hpfs_inode = hpfs_i(i);
struct inode *parent;
if (i->i_ino == hpfs_sb(i->i_sb)->sb_root) return;
- if (hpfs_inode->i_rddir_off && !icount_read(i)) {
+ if (hpfs_inode->i_rddir_off && !icount_read_once(i)) {
if (*hpfs_inode->i_rddir_off)
pr_err("write_inode: some position still there\n");
kfree(hpfs_inode->i_rddir_off);
diff --git a/fs/inode.c b/fs/inode.c
index 5ad169d51728..1f5a383ccf27 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -907,7 +907,7 @@ void evict_inodes(struct super_block *sb)
again:
spin_lock(&sb->s_inode_list_lock);
list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
- if (icount_read(inode))
+ if (icount_read_once(inode))
continue;
spin_lock(&inode->i_lock);
@@ -1926,7 +1926,7 @@ static void iput_final(struct inode *inode)
int drop;
WARN_ON(inode_state_read(inode) & I_NEW);
- VFS_BUG_ON_INODE(atomic_read(&inode->i_count) != 0, inode);
+ VFS_BUG_ON_INODE(icount_read(inode) != 0, inode);
if (op->drop_inode)
drop = op->drop_inode(inode);
@@ -1945,7 +1945,7 @@ static void iput_final(struct inode *inode)
* Re-check ->i_count in case the ->drop_inode() hooks played games.
* Note we only execute this if the verdict was to drop the inode.
*/
- VFS_BUG_ON_INODE(atomic_read(&inode->i_count) != 0, inode);
+ VFS_BUG_ON_INODE(icount_read(inode) != 0, inode);
if (drop) {
inode_state_set(inode, I_FREEING);
@@ -1989,7 +1989,7 @@ void iput(struct inode *inode)
* equal to one, then two CPUs racing to further drop it can both
* conclude it's fine.
*/
- VFS_BUG_ON_INODE(atomic_read(&inode->i_count) < 1, inode);
+ VFS_BUG_ON_INODE(icount_read_once(inode) < 1, inode);
if (atomic_add_unless(&inode->i_count, -1, 1))
return;
@@ -2023,7 +2023,7 @@ EXPORT_SYMBOL(iput);
void iput_not_last(struct inode *inode)
{
VFS_BUG_ON_INODE(inode_state_read_once(inode) & (I_FREEING | I_CLEAR), inode);
- VFS_BUG_ON_INODE(atomic_read(&inode->i_count) < 2, inode);
+ VFS_BUG_ON_INODE(icount_read_once(inode) < 2, inode);
WARN_ON(atomic_sub_return(1, &inode->i_count) == 0);
}
@@ -3046,7 +3046,7 @@ void dump_inode(struct inode *inode, const char *reason)
}
state = inode_state_read_once(inode);
- count = atomic_read(&inode->i_count);
+ count = icount_read_once(inode);
if (!sb ||
get_kernel_nofault(s_type, &sb->s_type) || !s_type ||
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 98a8f0de1199..22834eddd5b1 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -608,7 +608,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
inode->i_sb->s_id,
(unsigned long long)NFS_FILEID(inode),
nfs_display_fhandle_hash(fh),
- icount_read(inode));
+ icount_read_once(inode));
out:
return inode;
@@ -2261,7 +2261,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
dfprintk(VFS, "NFS: %s(%s/%llu fh_crc=0x%08x ct=%d info=0x%llx)\n",
__func__, inode->i_sb->s_id, inode->i_ino,
nfs_display_fhandle_hash(NFS_FH(inode)),
- icount_read(inode), fattr->valid);
+ icount_read_once(inode), fattr->valid);
if (!(fattr->valid & NFS_ATTR_FATTR_FILEID)) {
/* Only a mounted-on-fileid? Just exit */
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 888f9e35f14b..ab35e35b16d7 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -2842,7 +2842,7 @@ int cifs_revalidate_dentry_attr(struct dentry *dentry)
}
cifs_dbg(FYI, "Update attributes: %s inode 0x%p count %d dentry: 0x%p d_time %ld jiffies %ld\n",
- full_path, inode, icount_read(inode),
+ full_path, inode, icount_read_once(inode),
dentry, cifs_get_time(dentry), jiffies);
again:
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 9a77d8b64ffa..38972786817e 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -358,7 +358,7 @@ static void ubifs_evict_inode(struct inode *inode)
goto out;
dbg_gen("inode %llu, mode %#x", inode->i_ino, (int)inode->i_mode);
- ubifs_assert(c, !icount_read(inode));
+ ubifs_assert(c, !icount_read_once(inode));
truncate_inode_pages_final(&inode->i_data);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index beaa26ec62da..4f659eba6ae5 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1046,7 +1046,7 @@ xfs_itruncate_extents_flags(
int error = 0;
xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
- if (icount_read(VFS_I(ip)))
+ if (icount_read_once(VFS_I(ip)))
xfs_assert_ilocked(ip, XFS_IOLOCK_EXCL);
if (whichfork == XFS_DATA_FORK)
ASSERT(new_size <= XFS_ISIZE(ip));
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 5e8190fe2be9..cbdec40826b3 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1156,7 +1156,7 @@ DECLARE_EVENT_CLASS(xfs_iref_class,
TP_fast_assign(
__entry->dev = VFS_I(ip)->i_sb->s_dev;
__entry->ino = ip->i_ino;
- __entry->count = icount_read(VFS_I(ip));
+ __entry->count = icount_read_once(VFS_I(ip));
__entry->pincount = atomic_read(&ip->i_pincount);
__entry->iflags = ip->i_flags;
__entry->caller_ip = caller_ip;
diff --git a/include/trace/events/filelock.h b/include/trace/events/filelock.h
index 116774886244..c8c8847bb6f6 100644
--- a/include/trace/events/filelock.h
+++ b/include/trace/events/filelock.h
@@ -190,7 +190,7 @@ TRACE_EVENT(generic_add_lease,
__entry->i_ino = inode->i_ino;
__entry->wcount = atomic_read(&inode->i_writecount);
__entry->rcount = atomic_read(&inode->i_readcount);
- __entry->icount = icount_read(inode);
+ __entry->icount = icount_read_once(inode);
__entry->owner = fl->c.flc_owner;
__entry->flags = fl->c.flc_flags;
__entry->type = fl->c.flc_type;
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index c1ecfe239032..32d560f12dbd 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1278,7 +1278,7 @@ static void hook_sb_delete(struct super_block *const sb)
struct landlock_object *object;
/* Only handles referenced inodes. */
- if (!icount_read(inode))
+ if (!icount_read_once(inode))
continue;
/*
--
2.48.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 3/5] fs: enforce locking in icount_read(), add some commentary
2026-03-28 15:31 [PATCH 0/5] assorted ->i_count-related changes Mateusz Guzik
` (2 preceding siblings ...)
2026-03-28 15:31 ` [PATCH 2/5] Use icount_read() and icount_read_once() as appropriate Mateusz Guzik
@ 2026-03-28 15:31 ` Mateusz Guzik
2026-03-28 15:31 ` [PATCH 4/5] fs: handle hypothetical filesystems hich use I_DONTCACHE and drop the lock in ->drop_inode Mateusz Guzik
2026-03-28 15:31 ` [PATCH 5/5] fs: locklessly bump refs in igrab as long as it does not transition 0->1 Mateusz Guzik
5 siblings, 0 replies; 8+ messages in thread
From: Mateusz Guzik @ 2026-03-28 15:31 UTC (permalink / raw)
To: brauner; +Cc: viro, jack, linux-kernel, linux-fsdevel, Mateusz Guzik
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
include/linux/fs.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 0cf27085a579..07363fce4406 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2225,13 +2225,21 @@ static inline void mark_inode_dirty_sync(struct inode *inode)
__mark_inode_dirty(inode, I_DIRTY_SYNC);
}
+/*
+ * returns the refcount on the inode. it can change arbitrarily.
+ */
static inline int icount_read_once(const struct inode *inode)
{
return atomic_read(&inode->i_count);
}
+/*
+ * returns the refcount on the inode. The lock guarantees no new references
+ * are added, but references can be dropped as long as the result is > 0.
+ */
static inline int icount_read(const struct inode *inode)
{
+ lockdep_assert_held(&inode->i_lock);
return atomic_read(&inode->i_count);
}
--
2.48.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 4/5] fs: handle hypothetical filesystems hich use I_DONTCACHE and drop the lock in ->drop_inode
2026-03-28 15:31 [PATCH 0/5] assorted ->i_count-related changes Mateusz Guzik
` (3 preceding siblings ...)
2026-03-28 15:31 ` [PATCH 3/5] fs: enforce locking in icount_read(), add some commentary Mateusz Guzik
@ 2026-03-28 15:31 ` Mateusz Guzik
2026-03-28 15:31 ` [PATCH 5/5] fs: locklessly bump refs in igrab as long as it does not transition 0->1 Mateusz Guzik
5 siblings, 0 replies; 8+ messages in thread
From: Mateusz Guzik @ 2026-03-28 15:31 UTC (permalink / raw)
To: brauner; +Cc: viro, jack, linux-kernel, linux-fsdevel, Mateusz Guzik
f2fs and ntfs play games where they transitiong the refcount 0->1 and release
the inode spinlock, allowing other threads to grab a ref of their own.
They also return 0 in that case, making this problem harmless.
Should they start using the I_DONTCACHE machinery down the road while
retaining the above, iput_final() will get a race where it can proceed
to teardown an inode with references.
Future-proof it.
Developing better ->drop_inode and sanitizing all users is left as en
exercise for the reader.
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
fs/inode.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 1f5a383ccf27..fc6045e6d43f 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1933,20 +1933,29 @@ static void iput_final(struct inode *inode)
else
drop = inode_generic_drop(inode);
- if (!drop &&
- !(inode_state_read(inode) & I_DONTCACHE) &&
- (sb->s_flags & SB_ACTIVE)) {
+ /*
+ * XXXCRAP: there are ->drop_inode hooks playing nasty games releasing the
+ * spinlock and temporarily grabbing refs. This opens a possibility someone
+ * else will sneak in and grab a ref while it happens.
+ *
+ * If such a hook returns 0 (== don't drop) this happens to be harmless as long
+ * as the inode is not marked with I_DONTCACHE. Otherwise we are proceeding with
+ * teardown despite references being present.
+ *
+ * Damage-control the problem by including the count in the decision. However,
+ * assert no refs showed up if the hook decided to drop the inode.
+ */
+ if (drop)
+ VFS_BUG_ON_INODE(icount_read(inode) != 0, inode);
+
+ if (icount_read(inode) > 0 ||
+ (!drop && !(inode_state_read(inode) & I_DONTCACHE) &&
+ (sb->s_flags & SB_ACTIVE))) {
__inode_lru_list_add(inode, true);
spin_unlock(&inode->i_lock);
return;
}
- /*
- * Re-check ->i_count in case the ->drop_inode() hooks played games.
- * Note we only execute this if the verdict was to drop the inode.
- */
- VFS_BUG_ON_INODE(icount_read(inode) != 0, inode);
-
if (drop) {
inode_state_set(inode, I_FREEING);
} else {
--
2.48.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 5/5] fs: locklessly bump refs in igrab as long as it does not transition 0->1
2026-03-28 15:31 [PATCH 0/5] assorted ->i_count-related changes Mateusz Guzik
` (4 preceding siblings ...)
2026-03-28 15:31 ` [PATCH 4/5] fs: handle hypothetical filesystems hich use I_DONTCACHE and drop the lock in ->drop_inode Mateusz Guzik
@ 2026-03-28 15:31 ` Mateusz Guzik
5 siblings, 0 replies; 8+ messages in thread
From: Mateusz Guzik @ 2026-03-28 15:31 UTC (permalink / raw)
To: brauner; +Cc: viro, jack, linux-kernel, linux-fsdevel, Mateusz Guzik
The bump is gated by I_FREEING | I_WILL_FREE, but these flags can only
legally show up if the count is 0. Consequently if the value is at least
1 and it succesfully CAS'ed to something higher, the flags must not be
there.
I verified all places which look at the refcount either only care about
it staying 0 (and have the lock enforce it) or don't hold the inode lock
to begin with. Thus the patch retains the invariant for correct consumers
and does not make things worse for the rest.
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
fs/inode.c | 5 +++++
include/linux/fs.h | 4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index fc6045e6d43f..17b925887382 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1580,6 +1580,11 @@ EXPORT_SYMBOL(iunique);
struct inode *igrab(struct inode *inode)
{
+ if (atomic_add_unless(&inode->i_count, 1, 0)) {
+ VFS_BUG_ON_INODE(inode_state_read_once(inode) & (I_FREEING | I_WILL_FREE), inode);
+ return inode;
+ }
+
spin_lock(&inode->i_lock);
if (!(inode_state_read(inode) & (I_FREEING | I_WILL_FREE))) {
__iget(inode);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 07363fce4406..119e0a3d2f42 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2234,8 +2234,8 @@ static inline int icount_read_once(const struct inode *inode)
}
/*
- * returns the refcount on the inode. The lock guarantees no new references
- * are added, but references can be dropped as long as the result is > 0.
+ * returns the refcount on the inode. The lock guarantees no 0->1 or 1->0 transitions
+ * of the count are going to take place, otherwise it changes arbitrarily.
*/
static inline int icount_read(const struct inode *inode)
{
--
2.48.1
^ permalink raw reply [flat|nested] 8+ messages in thread