On Tue, Dec 05, 2006 at 08:36:22PM +0000, David Howells wrote:
> Nick Piggin <[email protected]> wrote:
>
> > Either approach works, and one is better than the current two approaches.
>
> >From one point of view that's true. But from other points of view, it isn't.
>
> > > have be implemented by spinlock on some archs, and so your approach is
> > > really not optimal in such cases. Not all archs have cmpxchg or any way
> > > of doing an
> >
> > We do lots of things that aren't optimal for everyone.
>
> In this case it can be particularly suboptimal. The approach you've chosen to
> generalise on is specifically for XADD-based rwsems. XADD is only available
> on i386, x86_64 and ia64 (FETCHADD), and of those only i386 and ia64 actually
> use the XADD-optimised rwsems. Andi wants x86_64 to use the spinlock version.
>
> Everywhere else, XADD is emulated.
>
> If you have to emulate XADD by using CMPXCHG or LL/SC equivalents, then
> there're actually better ways of doing things than either of these approaches
> that the kernel currently has. I just need to find the time to explore them.
>
> Look at how the counter works in the XADD-based version. That's the way it is
> *because* I'm using XADD. That's quite limiting.
Not really. ll/sc architectures "emulate" xadd the same as they would
emulate a spinlock. There is nothing suboptimal about it.
Look at alpha or powerpc, for example.
> > If it hurts, we can special case it. This really doesn't, considering the
> > rarity of rwsems.
>
> If you're willing to special case it, then what's the point in generalising?
Because I don't like the way it is currently done.
> And in case you're wondering, FS-Cache uses a _lot_ of rwsem operations, so I
> do have an interest in making it quick, and that includes undoing what Ingo
> did when it's not necessary.
And I bet you couldn't measure a difference, even on FRV.
> > I honestly couldn't see why you went on this tangent last time and I
> > can't see your point now. If FRV only has an SMP safe xchg, then it
> > will need to implement SMP atomics with spinlocks anyway, to get things
> > like add_return.
>
> That's _exactly_ my point.
>
> Think! If you _require_ the use of atomic ops in implementing rwsems, then
> you introduce _extra_, _general_ spinlocks into the XADD-based algorithm
> everywhere you call an atomic_xxx() op that has a side effect.
No, only where you also call down into the contended path. I never
claimed this is optimal for atomics-with-spinlocks architectures, and
I know how they are going to be impacted.
However, it is not 100% clear that they will be slower. For starters,
you are using 2 spinlocks, so if there is a lot of contention, you
can have concurrent down_read()s go through while the rwsem spinlock
is held for a long time while readers are being worken off the wait
list.
> And if you're going to have to be getting general spinlocks then the
> XADD-based rwsem really is worse by quite a bit than the current
> spinlock-based rwsem as the latter _doesn't_ need general spinlocks as it
> doesn't use atomic ops.
It is mostly worse in the contended case, in that it needs to take 2
spinlocks where 1 would do. But for *all* other SMP architectures (ie.
all - sparc32 - parisc) spinlock design is slower when there is
contention. I don't know how you could be advocating the spinlock
version on this basis.
FYI, x86-64 uses spinlocks because of the complexity and task limit
of the xadd algorithm, not because it was measured to be any faster.
> > And this is relevant how? Why do you imagine that atomic_cmpxchg
> > is so much harder than atomic_add_unless, atomic_add_return (or
> > even atomic_add) on an architecture like this?
>
> It isn't, but atomic_xxx() will _have_ to be implemented with spinlocks.
>
> My point here was that last time I believe you said that FRV managed to
> implement atomic ops without spinlocks, and so the case wasn't worth
> considering. My points are that (a) FRV would have to to do SMP, and (b) we
> have another arch lurking in the wings that does have to use spinlocks because
> I can't do the clever tricks there that I can on FRV.
So let's take another look when these are in the kernel.
> > Nor did I understand this objection last time. You must have just read the
> > description, rather than the code. I *do* use xadd on x86 for down read (ie.
> > atomic_add_return)
>
> I missed the atomic_add_return(), so I'll forgive you that bit.
You mean I'll forgive you ;) I've been trying to tell you this.
> > And the place where I use cmpxchg *already* uses cmpxchg on i386. Ditto for
> > alpha and powerpc (which are the ones implementing their own rwsem.h of your
> > above list).
> >
> > Actually all these architectures will generate basically identical code
> > (ignoring ool vs inline). So will all the UP architectures that supress
> > interrupts for atomic ops.
>
> Alpha and powerpc emulate XADD, so see above.
They *implement* atomic_add_return.
> > Which is why I'd prefer not to use it. The current rwsem stuff is much
> > worse though, which is why I'd consider it.
>
> No, it isn't. It's much more straightforward; or at least it was until Ingo
> decided to rearrange things.
Well I'm working with the mainline kernel.
Anyway, IMO it is cleaner to have down_read be a generic function which calls
into an arch helper to perform some simple procedure, rather than have
any architecture able implement a completely different algorithm.
> > 2 designs, 8 or so implementations (some subtly different) implemented in
> > arch specific asm.
>
> Yes, and?
Ah, that explains why you think this is pointless: you don't think that is
a problem. OK... if you think it is not then I won't be able to convince
you otherwise.
> > I moved to the implementation on the most widely used and tested arch,
> > as well as the ones which have the most parallelism and weakly ordered
> > memory (i386, powerpc, ia64). I figure this is likely to be adequately
> > scalable and have less chance of being buggy.
>
> The spinlock-based approach is easier to verify and has been tested on more
> archs than the other. I don't think either of them is more buggy than the
> other. The spinlock-based rwsems are also used on i386 under some
> circumstances.
On UP architectures, maybe. That's not too interesting.
> > > Actually, the code isn't anywhere near as difficult to read as the mutex
> > > code or the spinlock code. The fastcall "junk" is quite important wrt the
> > > i386 code and permitted a small speedup (though if we compile with
> > > regparms=3 nowadays, then the advantage is actually bypassed).
> >
> > No objections, then?
>
> WRONG!
>
> Remove the fastcall and the i386 rwsems will cease to function if
> CONFIG_REGPARM=n.
WRONG! Because they aren't called by assembly anymore. But if it means
that much to you, the fastcalls can stay.
> > Of course the code is slower when it is in L0 icache. I'm sure you know
> > that smaller code is the whole point...
>
> Not necessarily. Faster code may also be the whole point; it depends on what
Do you have to argue everything? I mean the whole point of OOLing spinlocks
and mutexes is to generate smaller code (with associated benefits like
reuse of branch predictor).
Anyway, this is not a big deal. If you want to continue arguing the point,
we can if we get past the other points.
-
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/
[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]
