Messages in this thread |  | | | Date | Thu, 12 Mar 2009 08:13:21 -0700 | | From | Darren Hart <> | | Subject | Re: [PATCH 3/6] futex: add double_unlock_hb() |
| |
Thomas Gleixner wrote: > 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. >
Right, sorry, it's the double_lock that requires the test. Duh. I need to find a way to do some of this work during more regular hours I guess ;-) Thanks for the catch everyone.
Ingo shall I resubmit? Or did you already clean it up?
Thanks,
Darren
>> 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
-- Darren Hart IBM Linux Technology Center Real-Time Linux Team
|  |