linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: juanfengpy@gmail.com
Cc: jirislaby@kernel.org, ilpo.jarvinen@linux.intel.com,
	benbjiang@tencent.com, robinlai@tencent.com,
	linux-serial@vger.kernel.org, caelli <caelli@tencent.com>
Subject: Re: [PATCH v3] tty: fix hang on tty device with no_room set
Date: Mon, 13 Jun 2022 19:20:17 +0200	[thread overview]
Message-ID: <Yqdx0W8HhvT5qZlP@kroah.com> (raw)
In-Reply-To: <1655123429-15376-1-git-send-email-caelli@tencent.com>

On Mon, Jun 13, 2022 at 08:30:29PM +0800, juanfengpy@gmail.com wrote:
> From: caelli <caelli@tencent.com>

This name/address does not match what you are sending it from, and I do
not think this is how you sign legal documents right?

For that reason alone, I can't take this :(

> 
> We have met a hang on pty device, the reader was blocking
> at epoll on master side, the writer was sleeping at wait_woken
> inside n_tty_write on slave side, and the write buffer on
> tty_port was full, we found that the reader and writer would
> never be woken again and blocked forever.
> 
> The problem was caused by a race between reader and kworker:
> n_tty_read(reader):  n_tty_receive_buf_common(kworker):
>                     |room = N_TTY_BUF_SIZE - (ldata->read_head - tail)
>                     |room <= 0
> copy_from_read_buf()|
> n_tty_kick_worker() |
>                     |ldata->no_room = true
> 
> After writing to slave device, writer wakes up kworker to flush
> data on tty_port to reader, and the kworker finds that reader
> has no room to store data so room <= 0 is met. At this moment,
> reader consumes all the data on reader buffer and call
> n_tty_kick_worker to check ldata->no_room which is false and
> reader quits reading. Then kworker sets ldata->no_room=true
> and quits too.
> 
> If write buffer is not full, writer will wake kworker to flush data
> again after following writes, but if write buffer is full and writer
> goes to sleep, kworker will never be woken again and tty device is
> blocked.
> 
> This problem can be solved with a check for read buffer size inside
> n_tty_receive_buf_common, if read buffer is empty and ldata->no_room
> is true, a call to n_tty_kick_worker is necessary to keep flushing
> data to reader.
> 
> Signed-off-by: caelli <caelli@tencent.com>
> ---
> Previous threads:
> https://lore.kernel.org/all/CAPmgiULo4h8bOrzL+XJ5Pndw0kz80fBPfH_KNLx3c5j-Yj04SA@mail.gmail.com/t/
> 
> I corrected some format problems as recommended and switched client to git send-email,
> which may be ok. And subject is changed from 'tty: fix a possible hang on tty device' to
> 'tty: fix hang on tty device with no_room set' to make subject more obvious.

Please properly version your patches like the documentation explains how
to, so we know what has changed from previous versions.  Otherwise they
all look identical to us.

thanks,

greg k-h

  reply	other threads:[~2022-06-13 19:24 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-24  2:21 tty: fix a possible hang on tty device cael
2022-05-24  9:11 ` Ilpo Järvinen
2022-05-24 11:09   ` cael
2022-05-24 11:40     ` Ilpo Järvinen
2022-05-24 12:47       ` cael
2022-05-24 13:25         ` Ilpo Järvinen
2022-05-25 10:36           ` cael
2022-05-25 11:21             ` Ilpo Järvinen
2022-05-30 13:13               ` cael
2022-05-31 12:37                 ` Ilpo Järvinen
2022-06-01  9:38 ` Greg KH
2022-06-01 13:39   ` cael
2022-06-01 14:47     ` Greg KH
2022-06-01 15:28     ` Ilpo Järvinen
2022-06-06 13:40       ` cael
2022-06-06 14:43         ` Greg KH
2022-06-11  6:50           ` cael
2022-06-11  7:32             ` Greg KH
2022-06-13 12:30               ` [PATCH v3] tty: fix hang on tty device with no_room set juanfengpy
2022-06-13 17:20                 ` Greg KH [this message]
2022-06-15  3:45                   ` [PATCH v4] " cael
2022-06-15  5:00                     ` Greg KH
2022-06-15  7:57                       ` Ilpo Järvinen
2022-06-15  9:29                         ` Greg KH
2022-06-15 11:17                           ` [PATCH v5] " cael
2022-06-15 11:29                             ` Ilpo Järvinen
2022-06-15 13:33                               ` caelli
2022-06-27 12:05                             ` Greg KH
2022-06-27 13:53                               ` [PATCH v6] " juanfengpy
2023-03-17  2:41                               ` [PATCH v7] " juanfengpy
2023-03-17  6:32                                 ` Jiri Slaby
2023-03-17  7:25                                   ` [PATCH v8] " juanfengpy
2023-04-06  2:44                                     ` [PATCH v9] " juanfengpy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yqdx0W8HhvT5qZlP@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=benbjiang@tencent.com \
    --cc=caelli@tencent.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jirislaby@kernel.org \
    --cc=juanfengpy@gmail.com \
    --cc=linux-serial@vger.kernel.org \
    --cc=robinlai@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).