bcachefs: six locks: Simplify optimistic spinning

osq lock maintainers don't want it to be used outside of kernel/locking/
- but, we can do better.

Since we have lock handoff signalled via waitlist entries, there's no
reason for optimistic spinning to have to look at the lock at all -
aside from checking lock-owner; we can just spin looking at our waitlist
entry.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/Kconfig b/fs/bcachefs/Kconfig
index fddc7be580223a54357deb7647b9fa41748679e3..7d3ac9556c9867d479ae9eaa42fac6d8ac039467 100644 (file)
--- a/fs/bcachefs/Kconfig
+++ b/fs/bcachefs/Kconfig
@@ -85,6 +85,16 @@ config BCACHEFS_NO_LATENCY_ACCT
        help
        This disables device latency tracking and time stats, only for performance testing
 
+config BCACHEFS_SIX_OPTIMISTIC_SPIN
+       bool "Optimistic spinning for six locks"
+       depends on BCACHEFS_FS
+       depends on SMP
+       default y
+       help
+       Instead of immediately sleeping when attempting to take a six lock that
+       is held by another thread, spin for a short while, as long as the
+       thread owning the lock is running.
+
 config MEAN_AND_VARIANCE_UNIT_TEST
        tristate "mean_and_variance unit tests" if !KUNIT_ALL_TESTS
        depends on KUNIT

diff --git a/fs/bcachefs/six.c b/fs/bcachefs/six.c
index 97790445e67ad2923fc4a0413d2c824cf506455e..3a494c5d12478595c76bebc89fd15b517c5ed6d0 100644 (file)
--- a/fs/bcachefs/six.c
+++ b/fs/bcachefs/six.c
@@ -324,101 +324,57 @@ bool six_relock_ip(struct six_lock *lock, enum six_lock_type type,
 }
 EXPORT_SYMBOL_GPL(six_relock_ip);
 
-#ifdef CONFIG_SIX_LOCK_SPIN_ON_OWNER
+#ifdef CONFIG_BCACHEFS_SIX_OPTIMISTIC_SPIN
 
-static inline bool six_can_spin_on_owner(struct six_lock *lock)
+static inline bool six_owner_running(struct six_lock *lock)
 {
-       struct task_struct *owner;
-       bool ret;
-
-       if (need_resched())
-               return false;
-
+       /*
+        * When there's no owner, we might have preempted between the owner
+        * acquiring the lock and setting the owner field. If we're an RT task
+        * that will live-lock because we won't let the owner complete.
+        */
        rcu_read_lock();
-       owner = READ_ONCE(lock->owner);
-       ret = !owner || owner_on_cpu(owner);
+       struct task_struct *owner = READ_ONCE(lock->owner);
+       bool ret = owner ? owner_on_cpu(owner) : !rt_task(current);
        rcu_read_unlock();
 
        return ret;
 }
 
-static inline bool six_spin_on_owner(struct six_lock *lock,
-                                    struct task_struct *owner,
-                                    u64 end_time)
+static inline bool six_optimistic_spin(struct six_lock *lock,
+                                      struct six_lock_waiter *wait,
+                                      enum six_lock_type type)
 {
-       bool ret = true;
        unsigned loop = 0;
-
-       rcu_read_lock();
-       while (lock->owner == owner) {
-               /*
-                * Ensure we emit the owner->on_cpu, dereference _after_
-                * checking lock->owner still matches owner. If that fails,
-                * owner might point to freed memory. If it still matches,
-                * the rcu_read_lock() ensures the memory stays valid.
-                */
-               barrier();
-
-               if (!owner_on_cpu(owner) || need_resched()) {
-                       ret = false;
-                       break;
-               }
-
-               if (!(++loop & 0xf) && (time_after64(sched_clock(), end_time))) {
-                       six_set_bitmask(lock, SIX_LOCK_NOSPIN);
-                       ret = false;
-                       break;
-               }
-
-               cpu_relax();
-       }
-       rcu_read_unlock();
-
-       return ret;
-}
-
-static inline bool six_optimistic_spin(struct six_lock *lock, enum six_lock_type type)
-{
-       struct task_struct *task = current;
        u64 end_time;
 
        if (type == SIX_LOCK_write)
                return false;
 
-       preempt_disable();
-       if (!six_can_spin_on_owner(lock))
-               goto fail;
+       if (lock->wait_list.next != &wait->list)
+               return false;
 
-       if (!osq_lock(&lock->osq))
-               goto fail;
+       if (atomic_read(&lock->state) & SIX_LOCK_NOSPIN)
+               return false;
 
+       preempt_disable();
        end_time = sched_clock() + 10 * NSEC_PER_USEC;
 
-       while (1) {
-               struct task_struct *owner;
-
+       while (!need_resched() && six_owner_running(lock)) {
                /*
-                * If there's an owner, wait for it to either
-                * release the lock or go to sleep.
+                * Ensures that writes to the waitlist entry happen after we see
+                * wait->lock_acquired: pairs with the smp_store_release in
+                * __six_lock_wakeup
                 */
-               owner = READ_ONCE(lock->owner);
-               if (owner && !six_spin_on_owner(lock, owner, end_time))
-                       break;
-
-               if (do_six_trylock(lock, type, false)) {
-                       osq_unlock(&lock->osq);
+               if (smp_load_acquire(&wait->lock_acquired)) {
                        preempt_enable();
                        return true;
                }
 
-               /*
-                * When there's no owner, we might have preempted between the
-                * owner acquiring the lock and setting the owner field. If
-                * we're an RT task that will live-lock because we won't let
-                * the owner complete.
-                */
-               if (!owner && (need_resched() || rt_task(task)))
+               if (!(++loop & 0xf) && (time_after64(sched_clock(), end_time))) {
+                       six_set_bitmask(lock, SIX_LOCK_NOSPIN);
                        break;
+               }
 
                /*
                 * The cpu_relax() call is a compiler barrier which forces
@@ -429,24 +385,15 @@ static inline bool six_optimistic_spin(struct six_lock *lock, enum six_lock_type
                cpu_relax();
        }
 
-       osq_unlock(&lock->osq);
-fail:
        preempt_enable();
-
-       /*
-        * If we fell out of the spin path because of need_resched(),
-        * reschedule now, before we try-lock again. This avoids getting
-        * scheduled out right after we obtained the lock.
-        */
-       if (need_resched())
-               schedule();
-
        return false;
 }
 
-#else /* CONFIG_SIX_LOCK_SPIN_ON_OWNER */
+#else /* CONFIG_LOCK_SPIN_ON_OWNER */
 
-static inline bool six_optimistic_spin(struct six_lock *lock, enum six_lock_type type)
+static inline bool six_optimistic_spin(struct six_lock *lock,
+                                      struct six_lock_waiter *wait,
+                                      enum six_lock_type type)
 {
        return false;
 }
@@ -470,9 +417,6 @@ static int six_lock_slowpath(struct six_lock *lock, enum six_lock_type type,
        trace_contention_begin(lock, 0);
        lock_contended(&lock->dep_map, ip);
 
-       if (six_optimistic_spin(lock, type))
-               goto out;
-
        wait->task              = current;
        wait->lock_want         = type;
        wait->lock_acquired     = false;
@@ -510,6 +454,9 @@ static int six_lock_slowpath(struct six_lock *lock, enum six_lock_type type,
                ret = 0;
        }
 
+       if (six_optimistic_spin(lock, wait, type))
+               goto out;
+
        while (1) {
                set_current_state(TASK_UNINTERRUPTIBLE);
 

diff --git a/fs/bcachefs/six.h b/fs/bcachefs/six.h
index 4c268b0b83162cc314c9ff810261a6c6362e6b97..a7104ac1d35c2ad0c7e61ce15a398f875e450407 100644 (file)
--- a/fs/bcachefs/six.h
+++ b/fs/bcachefs/six.h
@@ -127,10 +127,6 @@
 #include <linux/sched.h>
 #include <linux/types.h>
 
-#ifdef CONFIG_SIX_LOCK_SPIN_ON_OWNER
-#include <linux/osq_lock.h>
-#endif
-
 enum six_lock_type {
        SIX_LOCK_read,
        SIX_LOCK_intent,
@@ -143,9 +139,6 @@ struct six_lock {
        unsigned                intent_lock_recurse;
        struct task_struct      *owner;
        unsigned __percpu       *readers;
-#ifdef CONFIG_SIX_LOCK_SPIN_ON_OWNER
-       struct optimistic_spin_queue osq;
-#endif
        raw_spinlock_t          wait_lock;
        struct list_head        wait_list;
 #ifdef CONFIG_DEBUG_LOCK_ALLOC