Messages in this thread | | | From | Linus Torvalds <> | Date | Wed, 24 Aug 2011 11:24:40 -0700 | Subject | Re: [PATCH 00/18] x86: Ticket lock + cmpxchg cleanup |
| |
On Wed, Aug 24, 2011 at 11:03 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > The patches look good to me, and 50 lines less is always nice too! > > Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
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
| |