lkml.org 
[lkml]   [2009]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] kdesu broken


On Tue, 28 Jul 2009, Linus Torvalds wrote:
>
> And when it _is_ called, it also clears TTY_LDISC, so now tty_ldisc_ref()
> will return NULL, so then flush_to_ldisc() will be a no-op.
>
> So as far as I can tell, we already protect against this whole case: if we
> call flush_to_ldisc() after the tty has been HUP'ed, it just won't _do_
> anything (unless the work hasn't been canceled, but in that case the timer
> would have done the same thing, so nothing new is introduced).

Btw, if you worry about the fact that this all could happen concurrently
(ie the hangup is done on one cpu, just as the other cpu is doing that
flush_to_ldisc() thing), then again, Ogawa's patch doesn't actually change
anything. The synchronous flush_to_ldisc() (done by Ogawa's patch) could
equally have been an asynchronous (done by a timer), and the timer may
already have triggered - so 'tty_ldisc_halt()' doing a cancel on the
delayed work is too late.

So I don't think there are any new races wrt concurrency there either,
even though we do not take any locks in the tty_flush_to_ldisc() case.
Because the timer wouldn't have taken any locks either..

Of course, if "tty_ldisc_halt()" (to remove any pending timer) and
"tty_ldisc_wait_idle()" (to make sure nothing else is executing right
then) is not sufficient, then there's a possible problem there if you hit
the timing just right, but again, that would be true _regardless_ of
Ogawa's changes as far as I can tell.

IOW, the whole argument really hinges on the fact that calling
flush_to_ldisc() manually (without any locking) is really not
fundamentally any different from the delayed work doing it from a timer.

And when we _do_ disable the timer, we also make that flush_to_ldisc()
function be a no-op, so the "tty_ldisc_halt()" effectively stops both the
timer and (conceptually) the manual call case too.

So now we have one remaining case, namely the case of the ldisc then being
re-initialized and TTY_LDISC is set again. But at that point, calling
flush_to_ldisc() had better be ok again, wouldn't you agree?

Linus


\
 
 \ /
  Last update: 2009-10-18 23:28    [W:0.387 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site