Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it

Russell King wrote:

On Thu, Dec 07, 2006 at 08:31:08PM +1100, Nick Piggin wrote:

Implementing ll/sc based accessor macros allows both ll/sc _and_ cmpxchg
architectures to produce optimal code.

Implementing an cmpxchg based accessor macro allows cmpxchg architectures
to produce optimal code and ll/sc non-optimal code.

See my point?

Wrong. Your ll/sc implementation with cmpxchg is buggy. The cmpxchg
load_locked is not locked at all,

Intentional - cmpxchg architectures don't generally have a load locked.

Exactly, so it is wrong -- you can't implement that behaviour with
load + cmpxchg.

and there can be interleaving writes
between the load and cmpxchg which do not cause the store_conditional
to fail.

In which case the cmpxchg fails and we do the atomic operation again,
in exactly the same way that we do the operation again if the 'sc'
fails in the ll/sc case.

Not if cmpxchg sees the same value, it won't fail, regardless of how
many writes have hit that memory address.

I do not see any problem.

This was not the big problem -- as I said, if this was the only problem
we could opt for a "watered down" version that doesn't actually load
locked [the ll/sc interface would be much cooler than cmpxchg, IMO :)]

The main problem is the restrictions between the ll and sc. This is why
I implemented atomic_cmpxchg rather than atomic_ll/sc. However I agree
that in critical code, a higher level API should be used instead (eg.
see atomic_add_unless, which can be implemented optimally on RISCs).

Nick

--
SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com -

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

---

• Follow-Ups:
  • Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
    • From: Russell King <[email protected]>

• References:
  • [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
    • From: David Howells <[email protected]>
  • Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
    • From: Christoph Lameter <[email protected]>
  • Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
    • From: Russell King <[email protected]>
  • Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
    • From: Christoph Lameter <[email protected]>
  • Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
    • From: Russell King <[email protected]>
  • Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
    • From: Nick Piggin <[email protected]>
  • Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
    • From: Russell King <[email protected]>

• Prev by Date: [PATCH 006 of 18] knfsd: nfsd4: handling more nfsd_cross_mnt errors in nfsd4 readdir
• Next by Date: [PATCH 007 of 18] knfsd: nfsd: don't drop silently on upcall deferral
• Previous by thread: Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
• Next by thread: Re: [PATCH] WorkStruct: Implement generic UP cmpxchg() where an arch doesn't support it
• Index(es):
  • Date
  • Thread

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]