All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Will Deacon <will.deacon@arm.com>
Cc: "waiman.long@hp.com" <waiman.long@hp.com>,
	"ksummit-discuss@lists.linuxfoundation.org"
	<ksummit-discuss@lists.linuxfoundation.org>
Subject: Re: [Ksummit-discuss] [TECH TOPIC] asm-generic implementations of low-level synchronisation constructs
Date: Thu, 8 May 2014 11:13:12 +0200	[thread overview]
Message-ID: <20140508091312.GH2844@laptop.programming.kicks-ass.net> (raw)
In-Reply-To: <20140507212001.GA5311@arm.com>

On Wed, May 07, 2014 at 10:20:01PM +0100, Will Deacon wrote:
> > In any case, something that's been brewing in the back of my mind is an
> > ATOMIC_OP() and ATOMIC_RET_OP() macro construct that takes a lambda
> > function (expr-stmt is I think the closes we get in C) and either
> > generates the appropriate ll/sc loop or a cmpxchg loop, depending on
> > arch.
> 
> I've been thinking along the same lines but decided it was a bit too
> abstract to propose here. I'd certainly be interested in talking about it
> though. Another cool thing would be to allow for arbitrary compositions of
> different atomic operations, then apply barrier semantics to the whole lot.
> Not sure how much mileage there is in that though, especially given the
> typical architectural restrictions on what you can in a LL/SC loop (and
> if they get too big, you shoot yourself in the foot).

OK, so I was bored in a waiting room..

So I've not yet had a look at all the arch ll/sc loop restrictions, for
some I'm sure the below will not work, but I'm hoping that for some
others it at least has a chance.

(also, waiting rooms suck..)

More or less Pseudo C, the ATOMIC things should be proper macros but I
was too lazy to do all the \ muck.

---

#ifndef load_exclusive
#define load_exclusive(ptr) ACCESS_ONCE(*ptr)
#endif

#ifndef	cmpxchg_relaxed
#define cmpxchg_relaxed	cmpxchg
#endif

/*
 * The 'stmt' statements below must include a statement of the form:
 *   __new == f(__val);
 * which computes the new value from the current/old value.
 *
 * The __ret argument should be either __new or __val, to return the new or old
 * value resp.
 */

#ifdef HAS_LL_SC

ATOMIC(ptr, stmt)
do {
	typeof(*ptr) __new, __val;

	do {
		__val = load_locked(ptr);
		stmt;
	} while (!store_conditional(ptr, __new));
} while (0)


ATOMIC_RET(ptr, __ret, stmt)
({
	typeof(*ptr) __new, __val;

	smp_mb__before_llsc();

	do {
		__val = load_locked(ptr);
		stmt;
	} while (!store_conditional(ptr, __new));

	smp_mb__after_llsc();

	__ret;
})

#else

ATOMIC(ptr, stmt)
do {
	typeof(*ptr) __old, __new, __val;

	__val = load_exclusive(ptr);
	for (;;) {
		stmt;
		__old = cmpxchg_relaxed(ptr, __val, __new);
		if (__old == __val)
			break;
		__val = __old;
	}
} while (0)

ATOMIC(ptr, __ret, stmt)
({
	typeof(*ptr) __old, __new, __val;

	__val = load_exclusive(ptr);
	for (;;) {
		stmt;
		__old = cmpxchg(ptr, __val, __new);
		if (__old == __val)
			break;
		__val = __old;
	}

	__ret;
})

#endif


static inline int atomic_add_unless(atomic_t *v, int a, int u)
{
	return ATOMIC_RET(&v->counter, __old,
		if (unlikely(__val == u))
			break;
		__new = __val + a;
	);
}


And this also raises your other point, what barrier does/should
add_unless() imply in the failure case. The cmpxchg() variant won't in
fact guarantee any barrier, while the ll/sc one depends on the arch.

Also, maybe, we should take this discussion elsewhere.. :-)

  reply	other threads:[~2014-05-08  9:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-07 18:29 [Ksummit-discuss] [TECH TOPIC] asm-generic implementations of low-level synchronisation constructs Will Deacon
2014-05-07 19:12 ` Peter Zijlstra
2014-05-07 21:20   ` Will Deacon
2014-05-08  9:13     ` Peter Zijlstra [this message]
2014-05-08 14:27       ` Peter Zijlstra
2014-05-08 14:43         ` David Woodhouse
2014-05-08 15:13         ` Will Deacon
2014-05-08 16:39           ` Paul E. McKenney
2014-05-07 21:17 ` Waiman Long
2014-05-07 21:26   ` Peter Zijlstra
2014-05-07 22:29     ` Waiman Long
2014-05-08 14:16   ` Will Deacon

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=20140508091312.GH2844@laptop.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=ksummit-discuss@lists.linuxfoundation.org \
    --cc=waiman.long@hp.com \
    --cc=will.deacon@arm.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.