* Re: [PATCH v3] tty: resolve loopback wait problem for aging serial port
[not found] <1609335929-13463-1-git-send-email-zhangqiumiao1@huawei.com>
@ 2020-12-30 13:57 ` Greg KH
2021-01-04 9:09 ` Johan Hovold
0 siblings, 1 reply; 2+ messages in thread
From: Greg KH @ 2020-12-30 13:57 UTC (permalink / raw)
To: zhangqiumiao1; +Cc: jirislaby, linux-kernel
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
^ permalink raw reply [flat|nested] 2+ messages in thread