On Mon, Aug 19, 2024 at 11:30:15AM -0700, Christoph Lameter via B4 Relay wrote:
> diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
> index d90d8ee29d81..353fcf32b800 100644
> --- a/include/linux/seqlock.h
> +++ b/include/linux/seqlock.h
> @@ -176,6 +176,28 @@ __seqprop_##lockname##_sequence(const seqcount_##lockname##_t *s) \
> return seq; \
> } \
> \
> +static __always_inline unsigned \
> +__seqprop_##lockname##_sequence_acquire(const seqcount_##lockname##_t *s) \
> +{ \
> + unsigned seq = smp_load_acquire(&s->seqcount.sequence); \
> + \
> + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) \
> + return seq; \
> + \
> + if (preemptible && unlikely(seq & 1)) { \
> + __SEQ_LOCK(lockbase##_lock(s->lock)); \
> + __SEQ_LOCK(lockbase##_unlock(s->lock)); \
> + \
> + /* \
> + * Re-read the sequence counter since the (possibly \
> + * preempted) writer made progress. \
> + */ \
> + seq = smp_load_acquire(&s->seqcount.sequence); \
We could probably do even better with LDAPR here, as that should be
sufficient for this. It's a can of worms though, as it's not implemented
on all CPUs and relaxing smp_load_acquire() might introduce subtle
breakage in places where it's used to build other types of lock. Maybe
you can hack something to see if there's any performance left behind
without it?
> + } \
> + \
> + return seq; \
> +} \
> + \
> static __always_inline bool \
> __seqprop_##lockname##_preemptible(const seqcount_##lockname##_t *s) \
> { \
> @@ -211,6 +233,11 @@ static inline unsigned __seqprop_sequence(const seqcount_t *s)
> return READ_ONCE(s->sequence);
> }
>
> +static inline unsigned __seqprop_sequence_acquire(const seqcount_t *s)
> +{
> + return smp_load_acquire(&s->sequence);
> +}
> +
> static inline bool __seqprop_preemptible(const seqcount_t *s)
> {
> return false;
> @@ -259,6 +286,7 @@ SEQCOUNT_LOCKNAME(mutex, struct mutex, true, mutex)
> #define seqprop_ptr(s) __seqprop(s, ptr)(s)
> #define seqprop_const_ptr(s) __seqprop(s, const_ptr)(s)
> #define seqprop_sequence(s) __seqprop(s, sequence)(s)
> +#define seqprop_sequence_acquire(s) __seqprop(s, sequence_acquire)(s)
> #define seqprop_preemptible(s) __seqprop(s, preemptible)(s)
> #define seqprop_assert(s) __seqprop(s, assert)(s)
>
> @@ -293,6 +321,18 @@ SEQCOUNT_LOCKNAME(mutex, struct mutex, true, mutex)
> *
> * Return: count to be passed to read_seqcount_retry()
> */
> +#ifdef CONFIG_ARCH_HAS_ACQUIRE_RELEASE
> +#define raw_read_seqcount_begin(s) \
> +({ \
> + unsigned _seq; \
> + \
> + while ((_seq = seqprop_sequence_acquire(s)) & 1) \
> + cpu_relax(); \
It would also be interesting to see whether smp_cond_load_acquire()
performs any better that this loop in the !RT case.
Both things to look at separately though, so:
Acked-by: Will Deacon <will@xxxxxxxxxx>
I assume this will go via -tip.
Will
