On Wed, Dec 30, 2020 at 09:45:29PM +0800, zhangqiumi...@huawei.com wrote:
> From: Qiumiao Zhang <zhangqiumi...@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 <zhangqiumi...@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
