Messages in this thread | | | Date | Thu, 12 Mar 2009 11:58:51 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH 3/6] futex: add double_unlock_hb() |
| |
On Thu, 12 Mar 2009, Ingo Molnar wrote: > * Peter Zijlstra <peterz@infradead.org> wrote: > > > On Thu, 2009-03-12 at 00:55 -0700, Darren Hart wrote: > > > The futex code uses double_lock_hb() which locks the hb->lock's in pointer > > > value order. There is no parallel unlock routine, and the code unlocks them > > > in name order, ignoring pointer value. This opens up a window for an ABBA > > > deadlock. This patch adds double_unlock_hb() to remove the window as well > > > as refactor the duplicated code segments. > > > > While I don't mind the patch per-se, I'm hard pressed to see > > any deadlock potential in the unordered unlock. > > > > All sites (at least those in the patch) always release both > > locks without taking another in between, therefore one would > > think there's no deadlock possible. > > yeah.
I can't see a deadlock either.
> The patch is still nice (as you mention), it factors out the > unlock sequence. I'll change the commit message accordingy.
We do not need the comparison magic. Can we just put the code into double_unlock_hb() which gets replaced ?
i.e:
spin_unlock(&hb1->lock); if (hb1 != hb2) spin_unlock(&hb2->lock);
This code is confusing enough.
Thanks,
tglx
| |