public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Boqun Feng <boqun@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Ingo Molnar <mingo@redhat.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Marc Zyngier <maz@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Huth <thuth@redhat.com>, Waiman Long <longman@redhat.com>,
	Will Deacon <will@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: osq_lock on arm64
Date: Sun, 15 Mar 2026 20:11:53 +0000	[thread overview]
Message-ID: <20260315201153.1d2ae1e3@pumpkin> (raw)

Commit f5bfdc8e3947 change the osq_lock (optimistic spin queue) code to
use smp_cond_load_relaxed() to wait for another cpu to relase the lock.

It waits for 'VAL || need_resched()' with a comment that the IPI that
sets need_resched() will break any monitor/wait code.
However arm64 implements it as (03e3c2b7edbe and 1cfc63b5ae6):

#define smp_cond_load_acquire(ptr, cond_expr)                          \
({                                                                     \
       typeof(ptr) __PTR = (ptr);                                      \
       typeof(*ptr) VAL;                                               \
       for (;;) {                                                      \
               VAL = smp_load_acquire(__PTR);                          \
               if (cond_expr)                                          \
                       break;                                          \
               __cmpwait_relaxed(__PTR, VAL);                          \
       }                                                               \
       VAL;                                                            \
})

#define __CMPWAIT_CASE(w, sz, name)                                    \
static inline void __cmpwait_case_##name(volatile void *ptr,           \
                                        unsigned long val)             \
{                                                                      \
       unsigned long tmp;                                              \
                                                                       \
       asm volatile(                                                   \
       "       sevl\n"                                                 \
       "       wfe\n"                                                  \
       "       ldxr" #sz "\t%" #w "[tmp], %[v]\n"                      \
       "       eor     %" #w "[tmp], %" #w "[tmp], %" #w "[val]\n"     \
       "       cbnz    %" #w "[tmp], 1f\n"                             \
       "       wfe\n"                                                  \
       "1:"                                                            \
       : [tmp] "=&r" (tmp), [v] "+Q" (*(unsigned long *)ptr)           \
       : [val] "r" (val));                                             \
}

The "eor" and "cbnz" seem to be there to skip the "wfe" if the value has
already changed, but this only works if the condition only depends on VAL.
This means it can miss the IPI that makes need_resched() return non-zero.

To work I think it would need the cmp_load_acquire() to be "ldxr" with
a "wfe" instead of __cmpwait_relaxed().
But that wouldn't work if 'cond_expr' issued another "ldx".

The documentation I found for the arm64 instruction set doesn't mention that
losing the exclusive access requested by ldx will wake wfe, I assume that
is an omission from the docs?

	David 


             reply	other threads:[~2026-03-15 20:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-15 20:11 David Laight [this message]
2026-03-15 22:02 ` osq_lock on arm64 Marc Zyngier

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=20260315201153.1d2ae1e3@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=boqun@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=lpieralisi@kernel.org \
    --cc=maz@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=thuth@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    /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