On Wed May 10, 2023 at 1:31 PM AEST, Rohan McLure wrote:
> Annotate the release barrier and memory clobber (in effect, producing a
> compiler barrier) in the publish_tail_cpu call. These barriers have the
> effect of ensuring that qnode attributes are all written to prior to
> publish the node to the waitqueue.
>
> Even while the initial write to the 'locked' attribute is guaranteed to
> terminate prior to the node being visible, KCSAN still complains that
> the write is reorderable by the compiler. Issue a kcsan_release() to
> inform KCSAN of the release barrier contained in publish_tail_cpu().
>
> Signed-off-by: Rohan McLure <rmcl...@linux.ibm.com>
> ---
> v2: Remove extraneous compiler barrier, but annotate release-barrier
> contained in call publish_tail_cpu(), and include kcsan_release().
> ---
> arch/powerpc/lib/qspinlock.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/powerpc/lib/qspinlock.c b/arch/powerpc/lib/qspinlock.c
> index b76c1f6acce5..253620979d0c 100644
> --- a/arch/powerpc/lib/qspinlock.c
> +++ b/arch/powerpc/lib/qspinlock.c
> @@ -161,6 +161,8 @@ static __always_inline u32 publish_tail_cpu(struct
> qspinlock *lock, u32 tail)
> {
> u32 prev, tmp;
>
> + kcsan_release();
> +
> asm volatile(
> "\t" PPC_RELEASE_BARRIER " \n"
> "1: lwarx %0,0,%2 # publish_tail_cpu \n"
> @@ -570,6 +572,11 @@ static __always_inline void
> queued_spin_lock_mcs_queue(struct qspinlock *lock, b
>
> tail = encode_tail_cpu(node->cpu);
>
> + /*
> + * Assign all attributes of a node before it can be published.
> + * Issues an lwsync, serving as a release barrier, as well as a
> + * compiler barrier.
> + */
> old = publish_tail_cpu(lock, tail);
Possibly better to be with the publish function, hopefully the name
gives away it has a release barrier then store that makes it visible.
But that's nitpicking.
Thanks for the qspinlock fixes.
Reviewed-by: Nicholas Piggin <npig...@gmail.com>
