All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Parri <parri.andrea@gmail.com>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: ajones@ventanamicro.com, heiko@sntech.de,
	linux-riscv@lists.infradead.org,
	Paul Walmsley <paul.walmsley@sifive.com>,
	linux-kernel@vger.kernel.org, christoph.muellner@vrull.eu,
	David.Laight@aculab.com, heiko.stuebner@vrull.eu
Subject: Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
Date: Tue, 30 May 2023 20:45:28 +0200	[thread overview]
Message-ID: <ZHZESDWJZvhuJ3Af@andrea> (raw)
In-Reply-To: <mhng-d92f84d8-03db-4fb1-93c3-0d5bfbe7a796@palmer-ri-x1c9a>

On Wed, May 24, 2023 at 04:00:43PM -0700, Palmer Dabbelt wrote:
> On Wed, 24 May 2023 10:05:52 PDT (-0700), ajones@ventanamicro.com wrote:

> > I guess this peeling off of the first iteration is because it's expected
> > that the load generated by READ_ONCE() is more efficient than lr.w/d? If
> > we're worried about unnecessary use of lr.w/d, then shouldn't we look
> > for a solution that doesn't issue those instructions when we don't have
> > the Zawrs extension?
> 
> It's actually just a consequence of how the Linux hooks are described:
> they're macros that take a C expression to test in the loop, and we can't
> handle C expressions in LR/SC loops as that'd require compiler support and
> nobody's figured out how to do that correctly yet (there were some patches,
> but they had issues).  So we need to do this awkward bit of checking without
> the reservation and then waiting with the reservation.

I believe Andrew was really just hinting to something like (from
arch/arm64/):

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

where the __cmpwait_relaxed() would issue NOPs without Zawrs, a
sequence "lr.* ; beq ; wrs.sto" otherwise.  (with the "dangling
reservation" when we branch, similarly to CMPXCHG)?

  Andrea

WARNING: multiple messages have this Message-ID (diff)
From: Andrea Parri <parri.andrea@gmail.com>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: ajones@ventanamicro.com, heiko@sntech.de,
	linux-riscv@lists.infradead.org,
	Paul Walmsley <paul.walmsley@sifive.com>,
	linux-kernel@vger.kernel.org, christoph.muellner@vrull.eu,
	David.Laight@aculab.com, heiko.stuebner@vrull.eu
Subject: Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
Date: Tue, 30 May 2023 20:45:28 +0200	[thread overview]
Message-ID: <ZHZESDWJZvhuJ3Af@andrea> (raw)
In-Reply-To: <mhng-d92f84d8-03db-4fb1-93c3-0d5bfbe7a796@palmer-ri-x1c9a>

On Wed, May 24, 2023 at 04:00:43PM -0700, Palmer Dabbelt wrote:
> On Wed, 24 May 2023 10:05:52 PDT (-0700), ajones@ventanamicro.com wrote:

> > I guess this peeling off of the first iteration is because it's expected
> > that the load generated by READ_ONCE() is more efficient than lr.w/d? If
> > we're worried about unnecessary use of lr.w/d, then shouldn't we look
> > for a solution that doesn't issue those instructions when we don't have
> > the Zawrs extension?
> 
> It's actually just a consequence of how the Linux hooks are described:
> they're macros that take a C expression to test in the loop, and we can't
> handle C expressions in LR/SC loops as that'd require compiler support and
> nobody's figured out how to do that correctly yet (there were some patches,
> but they had issues).  So we need to do this awkward bit of checking without
> the reservation and then waiting with the reservation.

I believe Andrew was really just hinting to something like (from
arch/arm64/):

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

where the __cmpwait_relaxed() would issue NOPs without Zawrs, a
sequence "lr.* ; beq ; wrs.sto" otherwise.  (with the "dangling
reservation" when we branch, similarly to CMPXCHG)?

  Andrea

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2023-05-30 18:45 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-21 11:47 [PATCH v3 0/2] Add Zawrs support and use it for spinlocks Heiko Stuebner
2023-05-21 11:47 ` Heiko Stuebner
2023-05-21 11:47 ` [PATCH v3 1/2] riscv: don't include kernel.h into alternative.h Heiko Stuebner
2023-05-21 11:47   ` Heiko Stuebner
2023-05-24 14:01   ` Andrew Jones
2023-05-24 14:01     ` Andrew Jones
2023-05-21 11:47 ` [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks Heiko Stuebner
2023-05-21 11:47   ` Heiko Stuebner
2023-05-22 17:43   ` Conor Dooley
2023-05-22 17:43     ` Conor Dooley
2023-05-24 17:05   ` Andrew Jones
2023-05-24 17:05     ` Andrew Jones
2023-05-24 23:00     ` Palmer Dabbelt
2023-05-24 23:00       ` Palmer Dabbelt
2023-05-30 18:45       ` Andrea Parri [this message]
2023-05-30 18:45         ` Andrea Parri
2023-10-19 14:21 ` [PATCH v3 0/2] Add Zawrs support and use it " Andrea Parri
2023-10-19 14:21   ` Andrea Parri
2023-10-19 16:22   ` Christoph Müllner
2023-10-19 16:22     ` Christoph Müllner
2023-10-20 10:19     ` Andrea Parri
2023-10-20 10:19       ` Andrea Parri
2024-01-08 11:35       ` Andrew Jones
2024-01-08 11:35         ` Andrew Jones
2024-01-08 13:38         ` Andrea Parri
2024-01-08 13:38           ` Andrea Parri
2024-01-08 14:00         ` Christoph Müllner
2024-01-08 14:00           ` Christoph Müllner
2024-01-08 14:10           ` Andrew Jones
2024-01-08 14:10             ` Andrew Jones
2024-03-05 23:31             ` Charlie Jenkins
2024-03-05 23:31               ` Charlie Jenkins

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=ZHZESDWJZvhuJ3Af@andrea \
    --to=parri.andrea@gmail.com \
    --cc=David.Laight@aculab.com \
    --cc=ajones@ventanamicro.com \
    --cc=christoph.muellner@vrull.eu \
    --cc=heiko.stuebner@vrull.eu \
    --cc=heiko@sntech.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.