Messages in this thread | | | Date | Wed, 30 Dec 2020 14:57:14 +0100 | From | Greg KH <> | Subject | Re: [PATCH v3] tty: resolve loopback wait problem for aging serial port |
| |
On Wed, Dec 30, 2020 at 09:45:29PM +0800, zhangqiumiao1@huawei.com wrote: > From: Qiumiao Zhang <zhangqiumiao1@huawei.com> > > Because serial port is low-speed device, serial port writing will be suspended > when the buffer of driver layer is full due to serial port aging.
What do you mean here exactly? What is "serial port aging"?
And since when is a serial port a low-speed device? Compared to what?
> The concrete > representation is n_tty_write blocking in the process of wait_woken, the process > of writing serial port exists without feedback, and becomes a zombie process.
I do not understand this sentance, sorry. Can you rephrase it?
> So for serial port, set the timeout value of wait_woken function to 60s. Wake up > and flush the buffer after timeout.
Where did the 60 second number come from? Why that value and not any other specific value?
> > Signed-off-by: Qiumiao Zhang <zhangqiumiao1@huawei.com> > --- > v3: > add changes from v1 to v2 > > v2: > change to use "real name" > fix wrong expression in path description > remove config option > use driver type to judge tty device type > > drivers/tty/n_tty.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c > index 319d68c..8e7591a 100644 > --- a/drivers/tty/n_tty.c > +++ b/drivers/tty/n_tty.c > @@ -87,6 +87,8 @@ > # define n_tty_trace(f, args...) no_printk(f, ##args) > #endif > > +#define SIXTY_SEC_TIMEOUT (60 * HZ)
Is that define really needed?
> + > struct n_tty_data { > /* producer-published */ > size_t read_head; > @@ -2375,7 +2377,18 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file, > } > up_read(&tty->termios_rwsem); > > - wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT); > + /* Resolve the problem of loopback waiting when the serial port is aging seriously */ > + if (tty->driver->type == TTY_DRIVER_TYPE_SERIAL) { > + if (wait_woken(&wait, TASK_INTERRUPTIBLE, SIXTY_SEC_TIMEOUT) == 0) > + if (tty && tty->ops->flush_buffer) { > + printk(KERN_INFO "n_tty_write flush buffer\n");
Why have you left debugging code in here?
> + tty->ops->flush_buffer(tty); > + } else { > + printk(KERN_ERR "tty related structure not registered\n");
What can a user do with this? How can it be hit?
And never use printk() in a driver, please use dev_err() and friends instead.
thanks,
greg k-h
| |