Message ID | 20180904174808.GS335@mellanox.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | Regression from patch 'tty: hvc: hvc_poll() break hv read loop' | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
snowpatch_ozlabs/checkpatch | fail | Test checkpatch on branch next |
snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
On Wed, Sep 05, 2018 at 07:15:29AM +1000, Nicholas Piggin wrote: > On Tue, 4 Sep 2018 11:48:08 -0600 > Jason Gunthorpe <jgg@mellanox.com> wrote: > > > Hi Nicholas, > > > > I am testing 4.19-rc2 and I see bad behavior with my qemu hvc0 > > console.. > > > > Running interactive with qemu (qemu-2.11.2-1.fc28) on the console > > providing hvc0, using options like: > > > > -nographic > > -chardev stdio,id=stdio,mux=on,signal=off > > -mon chardev=stdio > > -device isa-serial,chardev=stdio > > -device virtio-serial-pci > > -device virtconsole,chardev=stdio > > > > I see the hvc0 console hang regularly, ie doing something like 'up > > arrow' in bash causes the hvc0 console to hang. Prior kernels worked > > OK. > > > > Any ideas? I'm not familiar with this code.. Thanks! > > Yes I have had another report, I'm working on a fix. Sorry it has taken > a while and thank you for the report. Okay, let me know when you have a fix and I will be able to test it for you! Thanks, Jason
On Tue, 4 Sep 2018 15:16:35 -0600 Jason Gunthorpe <jgg@mellanox.com> wrote: > On Wed, Sep 05, 2018 at 07:15:29AM +1000, Nicholas Piggin wrote: > > On Tue, 4 Sep 2018 11:48:08 -0600 > > Jason Gunthorpe <jgg@mellanox.com> wrote: > > > > > Hi Nicholas, > > > > > > I am testing 4.19-rc2 and I see bad behavior with my qemu hvc0 > > > console.. > > > > > > Running interactive with qemu (qemu-2.11.2-1.fc28) on the console > > > providing hvc0, using options like: > > > > > > -nographic > > > -chardev stdio,id=stdio,mux=on,signal=off > > > -mon chardev=stdio > > > -device isa-serial,chardev=stdio > > > -device virtio-serial-pci > > > -device virtconsole,chardev=stdio > > > > > > I see the hvc0 console hang regularly, ie doing something like 'up > > > arrow' in bash causes the hvc0 console to hang. Prior kernels worked > > > OK. > > > > > > Any ideas? I'm not familiar with this code.. Thanks! > > > > Yes I have had another report, I'm working on a fix. Sorry it has taken > > a while and thank you for the report. > > Okay, let me know when you have a fix and I will be able to test it > for you! Can you try this? diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c index 5414c4a87bea..f5fc3ba49130 100644 --- a/drivers/tty/hvc/hvc_console.c +++ b/drivers/tty/hvc/hvc_console.c @@ -49,6 +49,8 @@ #define N_OUTBUF 16 #define N_INBUF 16 +#define HVC_ATOMIC_READ_MAX 128 + #define __ALIGNED__ __attribute__((__aligned__(sizeof(long)))) static struct tty_driver *hvc_driver; @@ -522,6 +524,8 @@ static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count return -EIO; while (count > 0) { + int ret; + spin_lock_irqsave(&hp->lock, flags); rsize = hp->outbuf_size - hp->n_outbuf; @@ -537,10 +541,13 @@ static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count } if (hp->n_outbuf > 0) - hvc_push(hp); + ret = hvc_push(hp); spin_unlock_irqrestore(&hp->lock, flags); + if (!ret) + break; + if (count) { if (hp->n_outbuf > 0) hvc_flush(hp); @@ -669,8 +676,8 @@ static int __hvc_poll(struct hvc_struct *hp, bool may_sleep) if (!hp->irq_requested) poll_mask |= HVC_POLL_READ; + read_again: /* Read data if any */ - count = tty_buffer_request_room(&hp->port, N_INBUF); /* If flip is full, just reschedule a later read */ @@ -717,9 +724,23 @@ static int __hvc_poll(struct hvc_struct *hp, bool may_sleep) #endif /* CONFIG_MAGIC_SYSRQ */ tty_insert_flip_char(&hp->port, buf[i], 0); } - if (n == count) - poll_mask |= HVC_POLL_READ; - read_total = n; + read_total += n; + + if (may_sleep) { + /* Keep going until the flip is full */ + spin_unlock_irqrestore(&hp->lock, flags); + cond_resched(); + spin_lock_irqsave(&hp->lock, flags); + goto read_again; + } else if (read_total < HVC_ATOMIC_READ_MAX) { + /* Break and defer if it's a large read in atomic */ + goto read_again; + } + + /* + * Latency break, schedule another poll immediately. + */ + poll_mask |= HVC_POLL_READ; out: /* Wakeup write queue if necessary */
On Wed, Sep 05, 2018 at 01:51:56PM +1000, Nicholas Piggin wrote: > On Tue, 4 Sep 2018 15:16:35 -0600 > Jason Gunthorpe <jgg@mellanox.com> wrote: > > > On Wed, Sep 05, 2018 at 07:15:29AM +1000, Nicholas Piggin wrote: > > > On Tue, 4 Sep 2018 11:48:08 -0600 > > > Jason Gunthorpe <jgg@mellanox.com> wrote: > > > > > > > Hi Nicholas, > > > > > > > > I am testing 4.19-rc2 and I see bad behavior with my qemu hvc0 > > > > console.. > > > > > > > > Running interactive with qemu (qemu-2.11.2-1.fc28) on the console > > > > providing hvc0, using options like: > > > > > > > > -nographic > > > > -chardev stdio,id=stdio,mux=on,signal=off > > > > -mon chardev=stdio > > > > -device isa-serial,chardev=stdio > > > > -device virtio-serial-pci > > > > -device virtconsole,chardev=stdio > > > > > > > > I see the hvc0 console hang regularly, ie doing something like 'up > > > > arrow' in bash causes the hvc0 console to hang. Prior kernels worked > > > > OK. > > > > > > > > Any ideas? I'm not familiar with this code.. Thanks! > > > > > > Yes I have had another report, I'm working on a fix. Sorry it has taken > > > a while and thank you for the report. > > > > Okay, let me know when you have a fix and I will be able to test it > > for you! > > Can you try this? It worked for me, so it will work for Jason too (we have same setup). Tested-by: Leon Romanovsky <leonro@mellanox.com> Thanks
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c index fddb63322c6786..745ac220fce84c 100644 --- a/drivers/tty/hvc/hvc_console.c +++ b/drivers/tty/hvc/hvc_console.c @@ -592,7 +592,7 @@ static u32 timeout = MIN_TIMEOUT; int hvc_poll(struct hvc_struct *hp) { struct tty_struct *tty; - int i, n, poll_mask = 0; + int i, n, count, poll_mask = 0; char buf[N_INBUF] __ALIGNED__; unsigned long flags; int read_total = 0; @@ -618,7 +618,7 @@ int hvc_poll(struct hvc_struct *hp) /* Now check if we can get data (are we throttled ?) */ if (tty_throttled(tty)) - goto throttled; + goto out; /* If we aren't notifier driven and aren't throttled, we always * request a reschedule @@ -627,56 +627,58 @@ int hvc_poll(struct hvc_struct *hp) poll_mask |= HVC_POLL_READ; /* Read data if any */ - for (;;) { - int count = tty_buffer_request_room(&hp->port, N_INBUF); - /* If flip is full, just reschedule a later read */ - if (count == 0) { + count = tty_buffer_request_room(&hp->port, N_INBUF); + + /* If flip is full, just reschedule a later read */ + if (count == 0) { + poll_mask |= HVC_POLL_READ; + goto out; + } + + n = hp->ops->get_chars(hp->vtermno, buf, count); + if (n <= 0) { + /* Hangup the tty when disconnected from host */ + if (n == -EPIPE) { + spin_unlock_irqrestore(&hp->lock, flags); + tty_hangup(tty); + spin_lock_irqsave(&hp->lock, flags); + } else if ( n == -EAGAIN ) { + /* + * Some back-ends can only ensure a certain min + * num of bytes read, which may be > 'count'. + * Let the tty clear the flip buff to make room. + */ poll_mask |= HVC_POLL_READ; - break; } + goto out; + } - n = hp->ops->get_chars(hp->vtermno, buf, count); - if (n <= 0) { - /* Hangup the tty when disconnected from host */ - if (n == -EPIPE) { - spin_unlock_irqrestore(&hp->lock, flags); - tty_hangup(tty); - spin_lock_irqsave(&hp->lock, flags); - } else if ( n == -EAGAIN ) { - /* - * Some back-ends can only ensure a certain min - * num of bytes read, which may be > 'count'. - * Let the tty clear the flip buff to make room. - */ - poll_mask |= HVC_POLL_READ; - } - break; - } - for (i = 0; i < n; ++i) { + for (i = 0; i < n; ++i) { #ifdef CONFIG_MAGIC_SYSRQ - if (hp->index == hvc_console.index) { - /* Handle the SysRq Hack */ - /* XXX should support a sequence */ - if (buf[i] == '\x0f') { /* ^O */ - /* if ^O is pressed again, reset - * sysrq_pressed and flip ^O char */ - sysrq_pressed = !sysrq_pressed; - if (sysrq_pressed) - continue; - } else if (sysrq_pressed) { - handle_sysrq(buf[i]); - sysrq_pressed = 0; + if (hp->index == hvc_console.index) { + /* Handle the SysRq Hack */ + /* XXX should support a sequence */ + if (buf[i] == '\x0f') { /* ^O */ + /* if ^O is pressed again, reset + * sysrq_pressed and flip ^O char */ + sysrq_pressed = !sysrq_pressed; + if (sysrq_pressed) continue; - } + } else if (sysrq_pressed) { + handle_sysrq(buf[i]); + sysrq_pressed = 0; + continue; } -#endif /* CONFIG_MAGIC_SYSRQ */ - tty_insert_flip_char(&hp->port, buf[i], 0); } - - read_total += n; +#endif /* CONFIG_MAGIC_SYSRQ */ + tty_insert_flip_char(&hp->port, buf[i], 0); } - throttled: + if (n == count) + poll_mask |= HVC_POLL_READ; + read_total = n; + + out: /* Wakeup write queue if necessary */ if (hp->do_wakeup) { hp->do_wakeup = 0;