We restored tty_ldisc_wait_idle in 100eeae2c5c (TTY: restore
tty_ldisc_wait_idle). We used it in the ldisc changing path to fix the
case where there are tasks in n_tty_read waiting for data and somebody
tries to change ldisc.

Similar to the case above, there may be also tasks waiting in
n_tty_read. As 65b770468e98 (tty-ldisc: turn ldisc user count into a
proper refcount) removed the wait-until-idle from all paths, hangup
path won't wait for them to disappear either. So add it back even to
the hangup path.

There is a difference, we need uninterruptible sleep as there is
obviously HUP signal pending. So tty_ldisc_wait_idle now takes
interruptible parameter to decide. I'm not sure whether it's worth the
complexity. Instead uninterruptible waiting may fit even for the ldisc
change code. It wouldn't be possible to interrupted the change, but
there is 5s timeout anyway.

Before 65b770468e98 tty_ldisc_release was called also from
tty_ldisc_release. It is called from tty_release, so I don't think we
need to restore that one.

This is nicely reproducible after constify the timing as follows:
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1548,6 +1549,7 @@ static int n_tty_open(struct tty_struct *tty)

        /* These are ugly. Currently a malloc failure here can panic */
        if (!tty->read_buf) {
+               msleep(100);
                tty->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
                if (!tty->read_buf)
                        return -ENOMEM;
@@ -1741,6 +1743,7 @@ do_it_again:

        add_wait_queue(&tty->read_wait, &wait);
        while (nr) {
+               WARN_ON(!tty->read_buf);
                /* First test for status change. */
                if (packet && tty->link->ctrl_status) {
                        unsigned char cs;
@@ -1785,6 +1788,7 @@ do_it_again:
                                break;
                        }
                        timeout = schedule_timeout(timeout);
+                       msleep(20);
                        continue;
                }
                __set_current_state(TASK_RUNNING);

==================================

With a process:
    while (1) {
        int fd = open(argv[1], O_RDWR);
        read(fd, buf, sizeof(buf));
        close(fd);
    }
===== and its child: =====
        setsid();
        while (1) {
                int fd = open(tty, O_RDWR|O_NOCTTY);
                ioctl(fd, TIOCSCTTY, 1);
                vhangup();
                close(fd);
                usleep(100 * (10 + random() % 1000));
        }

References: https://bugzilla.novell.com/show_bug.cgi?id=693374
References: https://bugzilla.novell.com/show_bug.cgi?id=694509
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Alan Cox <[email protected]>
Cc: [email protected] [32, 33, 34, 39]
Cc: Linus Torvalds <[email protected]>
---
 drivers/tty/tty_ldisc.c |   34 +++++++++++++++++++++++++++-------
 1 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 5d01d32..3c01bf0 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -552,14 +552,32 @@ static void tty_ldisc_flush_works(struct tty_struct *tty)
  *     Wait for the line discipline to become idle. The discipline must
  *     have been halted for this to guarantee it remains idle.
  */
-static int tty_ldisc_wait_idle(struct tty_struct *tty)
+static int tty_ldisc_wait_idle(struct tty_struct *tty, bool interruptible)
 {
+       DEFINE_WAIT(wait);
+       long timeout = 5 * HZ;
+       int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE;
        int ret;
-       ret = wait_event_interruptible_timeout(tty_ldisc_idle,
-                       atomic_read(&tty->ldisc->users) == 1, 5 * HZ);
-       if (ret < 0)
-               return ret;
-       return ret > 0 ? 0 : -EBUSY;
+
+       for (;;) {
+               prepare_to_wait(&tty_ldisc_idle, &wait, state);
+               if (atomic_read(&tty->ldisc->users) == 1) {
+                       ret = 0;
+                       break;
+               }
+               if (interruptible && signal_pending(current)) {
+                       ret = -ERESTARTSYS;
+                       break;
+               }
+
+               timeout = schedule_timeout(timeout);
+               if (!timeout) {
+                       ret = -EBUSY;
+                       break;
+               }
+       }
+       finish_wait(&tty_ldisc_idle, &wait);
+       return ret;
 }
 
 /**
@@ -666,7 +684,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 
        tty_ldisc_flush_works(tty);
 
-       retval = tty_ldisc_wait_idle(tty);
+       retval = tty_ldisc_wait_idle(tty, 1);
 
        tty_lock();
        mutex_lock(&tty->ldisc_mutex);
@@ -763,6 +781,8 @@ static int tty_ldisc_reinit(struct tty_struct *tty, int 
ldisc)
        if (IS_ERR(ld))
                return -1;
 
+       WARN_ON_ONCE(tty_ldisc_wait_idle(tty, 0));
+
        tty_ldisc_close(tty, tty->ldisc);
        tty_ldisc_put(tty->ldisc);
        tty->ldisc = NULL;
-- 
1.7.5.3


_______________________________________________
stable mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/stable

Reply via email to