fs: allow lockless ->i_count bumps as long as it does not transition 0->1

With this change only 0->1 and 1->0 transitions need the lock.

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 (making the above change irrelevant to their correcness or
lack thereof).

I also confirmed nfs and btrfs like to call into these a lot and now
avoid the lock in the common case, shaving off some atomics.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
Link: https://patch.msgid.link/20260421182538.1215894-4-mjguzik@gmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/dcache.c b/fs/dcache.c
index 2c61aeea41f45df05bf335a28e4e54a16583f056..c8cc7c26b7257dba4920ed323c33f75e168cfa3f 100644 (file)
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2100,6 +2100,10 @@ void d_instantiate_new(struct dentry *entry, struct inode *inode)
        __d_instantiate(entry, inode);
        spin_unlock(&entry->d_lock);
        WARN_ON(!(inode_state_read(inode) & I_NEW));
+       /*
+        * Paired with igrab_from_hash()
+        */
+       smp_wmb();
        inode_state_clear(inode, I_NEW | I_CREATING);
        inode_wake_up_bit(inode, __I_NEW);
        spin_unlock(&inode->i_lock);

diff --git a/fs/inode.c b/fs/inode.c
index ab9573355f3b8858ac94bd9482f6bc2711fa4c12..8dc36f221256f835c8974cd1f2903b7e7835127f 100644 (file)
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1023,6 +1023,7 @@ long prune_icache_sb(struct super_block *sb, struct shrink_control *sc)
 }
 
 static void __wait_on_freeing_inode(struct inode *inode, bool hash_locked, bool rcu_locked);
+static bool igrab_from_hash(struct inode *inode);
 
 /*
  * Called with the inode lock held.
@@ -1047,6 +1048,11 @@ repeat:
                        continue;
                if (!test(inode, data))
                        continue;
+               if (igrab_from_hash(inode)) {
+                       rcu_read_unlock();
+                       *isnew = false;
+                       return inode;
+               }
                spin_lock(&inode->i_lock);
                if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE)) {
                        __wait_on_freeing_inode(inode, hash_locked, true);
@@ -1089,6 +1095,11 @@ repeat:
                        continue;
                if (inode->i_sb != sb)
                        continue;
+               if (igrab_from_hash(inode)) {
+                       rcu_read_unlock();
+                       *isnew = false;
+                       return inode;
+               }
                spin_lock(&inode->i_lock);
                if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE)) {
                        __wait_on_freeing_inode(inode, hash_locked, true);
@@ -1206,6 +1217,10 @@ void unlock_new_inode(struct inode *inode)
        lockdep_annotate_inode_mutex_key(inode);
        spin_lock(&inode->i_lock);
        WARN_ON(!(inode_state_read(inode) & I_NEW));
+       /*
+        * Paired with igrab_from_hash()
+        */
+       smp_wmb();
        inode_state_clear(inode, I_NEW | I_CREATING);
        inode_wake_up_bit(inode, __I_NEW);
        spin_unlock(&inode->i_lock);
@@ -1217,6 +1232,10 @@ void discard_new_inode(struct inode *inode)
        lockdep_annotate_inode_mutex_key(inode);
        spin_lock(&inode->i_lock);
        WARN_ON(!(inode_state_read(inode) & I_NEW));
+       /*
+        * Paired with igrab_from_hash()
+        */
+       smp_wmb();
        inode_state_clear(inode, I_NEW);
        inode_wake_up_bit(inode, __I_NEW);
        spin_unlock(&inode->i_lock);
@@ -1576,6 +1595,14 @@ EXPORT_SYMBOL(ihold);
 
 struct inode *igrab(struct inode *inode)
 {
+       /*
+        * Read commentary above igrab_from_hash() for an explanation why this works.
+        */
+       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);
@@ -1593,6 +1620,43 @@ struct inode *igrab(struct inode *inode)
 }
 EXPORT_SYMBOL(igrab);
 
+/*
+ * igrab_from_hash - special inode refcount acquire primitive for the inode hash
+ *
+ * It provides lockless refcount acquire in the common case of no problematic
+ * flags being set and the count being > 0.
+ *
+ * There are 4 state flags to worry about and the routine makes sure to not bump the
+ * ref if any of them is present.
+ *
+ * I_NEW and I_CREATING can only legally get set *before* the inode becomes visible
+ * during lookup. Thus if the flags are not spotted, they are guaranteed to not be
+ * a factor. However, we need an acquire fence before returning the inode just
+ * in case we raced against clearing the state to make sure our consumer picks up
+ * any other changes made prior. atomic_add_unless provides a full fence, which
+ * takes care of it.
+ *
+ * I_FREEING and I_WILL_FREE can only legally get set if ->i_count == 0 and it is
+ * illegal to bump the ref if either is present. Consequently if atomic_add_unless
+ * managed to replace a non-0 value with a bigger one, we have a guarantee neither
+ * of these flags is set. Note this means explicitly checking of these flags below
+ * is not necessary, it is only done because it does not cost anything on top of the
+ * load which already needs to be done to handle the other flags.
+ */
+static bool igrab_from_hash(struct inode *inode)
+{
+       if (inode_state_read_once(inode) & (I_NEW | I_CREATING | I_FREEING | I_WILL_FREE))
+               return false;
+       /*
+        * Paired with routines clearing I_NEW
+        */
+       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 true;
+       }
+       return false;
+}
+
 /**
  * ilookup5_nowait - search for an inode in the inode cache
  * @sb:                super block of file system to search

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 61ce251ad83ef79a89a3a738d7980304363cf117..c5c31a6f7ed58fa35c06a1d07a86c111b103540b 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2227,8 +2227,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)
 {