On 09/22/2012 10:35 AM, Sasha Levin wrote: > On 09/19/2012 09:25 PM, Jiri Slaby wrote: >> On 07/10/2012 06:54 AM, Shachar Shemesh wrote: >>>> From: Shachar Shemesh <[email protected]> >>>> >>>> Commit acfa747b introduced the TTY_HUPPING flag to distinguish >>>> closed TTY from currently closing ones. The test in tty_set_ldisc >>>> still remained pointing at the old flag. This causes pppd to >>>> sometimes lapse into uninterruptible sleep when killed and >>>> restarted. >>>> >>>> Signed-off-by: Shachar Shemesh <[email protected]> >>>> --- >>>> Tested with 3.2.20 kernel. >>>> >>>> diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c >>>> index 24b95db..a662a24 100644 >>>> --- a/drivers/tty/tty_ldisc.c >>>> +++ b/drivers/tty/tty_ldisc.c >>>> @@ -658,7 +658,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) >>>> goto enable; >>>> } >>>> >>>> - if (test_bit(TTY_HUPPED, &tty->flags)) { >>>> + if (test_bit(TTY_HUPPING, &tty->flags)) { >>>> /* We were raced by the hangup method. It will have stomped >>>> the ldisc data and closed the ldisc down */ >>>> clear_bit(TTY_LDISC_CHANGING, &tty->flags); >> Yes, that makes the issue go away, but does not seem to be right too. >> There are two issues I see: >> * TTY_HUPPED has no use now. That is incorrect. Here should be a test >> for both flags, I think. >> * The change forces the set_ldisc path to always re-open the ldisc even >> if it the terminal is HUPPED. > > This patch also causes hangs on newer kernels. Can it be reverted please?
Just for the record, how reproducible is this? IOW can you 100% say that the hangs are gone if you revert the patch? Could you identify the process sitting on the tty you are trying to hang up? > [ 482.860279] INFO: task init:1 blocked for more than 120 seconds. > [ 482.864244] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this > message. > > [ 482.867175] init D ffff88000d618000 3424 1 0 > 0x00000002 > [ 482.869321] ffff88000d5b9c28 0000000000000002 ffff88000d5b9be8 > ffffffff8114ff65 > [ 482.870387] ffff88000d5b9fd8 ffff88000d5b9fd8 ffff88000d5b9fd8 > ffff88000d5b9fd8 > [ 482.871419] ffff88000d618000 ffff88000d5b0000 ffff88000d5b08f0 > 7fffffffffffffff > [ 482.872143] Call Trace: > [ 482.872336] [<ffffffff8114ff65>] ? sched_clock_local+0x25/0xa0 > [ 482.872796] [<ffffffff839edb35>] schedule+0x55/0x60 > [ 482.873433] [<ffffffff839ebab5>] schedule_timeout+0x45/0x360 > [ 482.874134] [<ffffffff839ef25d>] ? _raw_spin_unlock_irqrestore+0x5d/0xb0 > [ 482.874752] [<ffffffff8117a5dd>] ? trace_hardirqs_on+0xd/0x10 > [ 482.875835] [<ffffffff839ef284>] ? _raw_spin_unlock_irqrestore+0x84/0xb0 > [ 482.876744] [<ffffffff81136f97>] ? prepare_to_wait+0x77/0x90 > [ 482.877485] [<ffffffff81b98ef6>] tty_ldisc_wait_idle.isra.7+0x76/0xb0 > [ 482.878428] [<ffffffff81137170>] ? abort_exclusive_wait+0xb0/0xb0 > [ 482.879239] [<ffffffff81b99bab>] tty_ldisc_hangup+0x1cb/0x320 > [ 482.879988] [<ffffffff81b90d52>] ? __tty_hangup+0x122/0x430 > [ 482.880491] [<ffffffff81b90d5a>] __tty_hangup+0x12a/0x430 BTW that also means that my proposed patch will cause the same hangup and we should proceed to step 2 suggested in the same patch. Given nobody noticed in the past 3 years, which is another supporting argument. But let's first investigate what is going on. thanks, -- js suse labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
