Re: [PATCH 00/18] x86: Ticket lock + cmpxchg cleanup

[Linus Torvalds]

On Wed, Aug 24, 2011 at 11:03 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> The patches look good to me, and 50 lines less is always nice too!
>
> Acked-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>

So I mostly agree, except for the abortion that is the
__ticket_spin_unlock changes. Please just kill that part.

I also personally would have preferred to re-order things a bit. For
example,  I despise

  [PATCH 05/18] x86/ticketlock: make __ticket_spin_lock common

because it is totally pointless. If patch "[PATCH 07/18] x86: add xadd
helper macro" had been before that patch, then the whole "if
(sizeof..)" crap would never have existed. So even though the end
result there looks good (unlike the __ticket_spin_unlock case, which
just *stays* uglier after the whole series), the individual patches
are much uglier than needed, just because they are done in the wrong
order.

So I'd much have preferred that the xadd helper go in first (07/18),
then the patch to make __ticket_spin_lock the same for 32/64 would go
in next (04/18) using that helper, and then they'd just have been
identical automatically.

Instead, there are a few patches that actually make the code uglier
temporarily, only to then be undone later. That seems bogus, when it
looks so simple to fix it.

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