Messages in this thread |  | | | Date | Sat, 6 Feb 2010 22:59:56 -0800 (PST) | | From | Linus Torvalds <> | | Subject | Re: [2.6.33-rc5] tty: possible irq lock inversion dependency in tty_fasync |
| |
On Sun, 7 Feb 2010, Américo Wang wrote: > > We already fixed this, a better fix:
No we didn't.
> http://lkml.org/lkml/2010/1/26/338 > > I sent a same fix with Greg's.
We already did that. You didn't read Tetsuo's email carefully.
Let's quote the important parts:
"is not yet fixed as of 2.6.33-rc7."
and also the _second_ lockdep complaint he quotes, which starts out with
[ 81.651199] ========================================================= [ 81.651199] [ INFO: possible irq lock inversion dependency detected ] [ 81.651199] 2.6.33-rc7 #11 [ 81.651199] --------------------------------------------------------- (note the -rc7 there).
The problem? Look at f_getown: it does
read_lock(&filp->f_owner.lock);
ie it holds f_owner without interrupts disabled. Now an interrupt comes in, and takes 'siglock' because it ends up sending a signal (timer, SIGIO, whatever). So you have a f_owner -> siglock ordering.
But we _also_ have a siglock -> ctrl_lock -> f_owner ordering, in that problematic tty_fasync() thing. So we have a ABBA deadlock situation.
Yes, it's hard (practically impossible) to trigger, because you have to get an interrupt just at the right point with all the right processes, but lockdep seems to be entirely correct.
So it is simply _wrong_ to take f_owner while we hold ctrl_lock.
Which is why I suggest just reverting both the original problematic commit _and_ the commit you point to, and just fix the race with that pid_get/put pair instead. As per my patch.
Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |