* [GIT PATCH] TTY/Serial fixes for 3.12-rc4 @ 2013-10-05 17:34 Greg KH 2013-10-05 18:53 ` Linus Torvalds 0 siblings, 1 reply; 11+ messages in thread From: Greg KH @ 2013-10-05 17:34 UTC (permalink / raw) To: Linus Torvalds, Jiri Slaby; +Cc: Andrew Morton, linux-kernel, linux-serial The following changes since commit 15c03dd4859ab16f9212238f29dd315654aa94f6: Linux 3.12-rc3 (2013-09-29 15:02:38 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-3.12-rc4 for you to fetch changes up to a9fbf4d591da6cd1d3eaab826c7c15f77fc8f6a3: xen/hvc: allow xenboot console to be used again (2013-10-03 16:12:58 -0700) ---------------------------------------------------------------- TTY fixes for 3.12-rc4 Here are 2 tty driver fixes for 3.12-rc4. One fixes the reported regression in the n_tty code that a number of people found recently, and the other one fixes an issue with xen consoles that broke in 3.10. Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ---------------------------------------------------------------- David Vrabel (1): xen/hvc: allow xenboot console to be used again Peter Hurley (1): tty: Fix pty master read() after slave closes drivers/tty/hvc/hvc_xen.c | 1 + drivers/tty/n_tty.c | 46 ++++++++++++++++++++++++++-------------------- 2 files changed, 27 insertions(+), 20 deletions(-) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PATCH] TTY/Serial fixes for 3.12-rc4 2013-10-05 17:34 [GIT PATCH] TTY/Serial fixes for 3.12-rc4 Greg KH @ 2013-10-05 18:53 ` Linus Torvalds 2013-10-05 23:57 ` Peter Hurley 0 siblings, 1 reply; 11+ messages in thread From: Linus Torvalds @ 2013-10-05 18:53 UTC (permalink / raw) To: Greg KH, Mikael Pettersson, Peter Hurley Cc: Jiri Slaby, Andrew Morton, Linux Kernel Mailing List, linux-serial On Sat, Oct 5, 2013 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote: > > One fixes the reported regression in the n_tty code that a number of > people found recently That one looks broken. Well, it looks like it might "work", but do so by hiding the issue for one case, while leaving it in the more general case. Why does it do that up_read(&tty->termios_rwsem); tty_flush_to_ldisc(tty); down_read(&tty->termios_rwsem); only if TTY_OTHER_CLOSED is set? If flushing the ldisc can generate more data, then we should do it *unconditionally* when we see that we currently have no data to read. As it is, it looks like the patch fixes the TTY_OTHER_CLOSED case ("read all pending data before returning -EIO"), but it leaves the same broken case for the O_NONBLOCK case and for a hung up tty. The O_NONBLOCK case is presumably just a performance problem (the data will come at _some_ point later), but it just looks bad in general. And the tty_hung_up_p() looks actiely buggy, with the same bug as the TTY_OTHER_CLOSED case that the patch tried to fix. Hmm? Am I missing something? The code is a bit confusing in *other* ways too: if you look later, it does this: n_tty_set_room(tty); which is documented to have to happen inside the termios_rwsem. HOWEVER, what does that do? It actually does an _asynchronous_ queue_work() of &tty->port->buf.work, in case there is now more room, and the previous one was blocked. And guess what that workqueue is all about? Right: it's flush_to_ldisc() - which is the work that tty_flush_to_ldisc() is trying to flush. So we're actually basically making sure we've flush the previous pending work. So even the tty_flush_to_ldisc(tty) that gets done in that patch is not necessarily sufficient, because the work might not have been scheduled because the flip buffer used to be full. Then flushing the work won't do anything, even though there is actually more data. Now, that is a very unlikely situation (I think it requires two concurrent readers), but it looks like it might be real. So I suspect we should *unconditionally* do n_tty_set_room(tty); up_read(&tty->termios_rwsem); tty_flush_to_ldisc(tty); down_read(&tty->termios_rwsem); if we don't have any pending input. And then test input_available_p() again. And only if we don't have any input after that flushing do we start doing the whole TTY_OTHER_CLOSED and tty_hung_up_p() tests. Hmm? Linus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PATCH] TTY/Serial fixes for 3.12-rc4 2013-10-05 18:53 ` Linus Torvalds @ 2013-10-05 23:57 ` Peter Hurley 2013-10-06 2:11 ` Peter Hurley 0 siblings, 1 reply; 11+ messages in thread From: Peter Hurley @ 2013-10-05 23:57 UTC (permalink / raw) To: Linus Torvalds Cc: Greg KH, Mikael Pettersson, Jiri Slaby, Andrew Morton, Linux Kernel Mailing List, linux-serial On 10/05/2013 02:53 PM, Linus Torvalds wrote: > On Sat, Oct 5, 2013 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote: >> >> One fixes the reported regression in the n_tty code that a number of >> people found recently > > That one looks broken. > > Well, it looks like it might "work", but do so by hiding the issue for > one case, while leaving it in the more general case. > > Why does it do that > > up_read(&tty->termios_rwsem); > tty_flush_to_ldisc(tty); > down_read(&tty->termios_rwsem); > > only if TTY_OTHER_CLOSED is set? If flushing the ldisc can generate > more data, then we should do it *unconditionally* when we see that we > currently have no data to read. > > As it is, it looks like the patch fixes the TTY_OTHER_CLOSED case > ("read all pending data before returning -EIO"), but it leaves the > same broken case for the O_NONBLOCK case and for a hung up tty. > > The O_NONBLOCK case is presumably just a performance problem (the data > will come at _some_ point later), but it just looks bad in general. > And the tty_hung_up_p() looks actiely buggy, with the same bug as the > TTY_OTHER_CLOSED case that the patch tried to fix. > > Hmm? Am I missing something? When a slave pty is closed, it's not hung up specifically so the master pty can be read. The same is not true for a regular tty; when a regular tty is hung up, all the pending data is vaporized (ie., what the tty layer refers to as 'flushed'). So checking for more data when tty_hung_up_p() is true is pointless. The distinction is clearer when you consider that even after the slave pty is closed, the master pty can still be read() even if it wasn't waiting in n_tty_read() at the time; this is not true of a regular tty, which cannot be read() after a hangup [tty_hung_up_p() tests if the file_operations pointer is set to non-operational read/write/ioctl functions]. The patch fixes a race condition which is peculiar to ptys only. > The code is a bit confusing in *other* ways too: if you look later, it > does this: > > n_tty_set_room(tty); > > which is documented to have to happen inside the termios_rwsem. > HOWEVER, what does that do? It actually does an _asynchronous_ > queue_work() of &tty->port->buf.work, in case there is now more room, > and the previous one was blocked. And guess what that workqueue is all > about? Right: it's flush_to_ldisc() - which is the work that > tty_flush_to_ldisc() is trying to flush. So we're actually basically > making sure we've flush the previous pending work. The flush_to_ldisc() worker no longer re-schedules itself. The flush_to_ldisc() worker is scheduled when one of two events happen; 1) the driver has just written received data to the tty buffers, or 2) space has just become available in the N_TTY line discipline's read buffer when it was previously full. The flush_to_ldisc() worker continues to run as long as space is available in the line discipline's read buffer or until the tty buffers are empty. Concurrently with the flush_to_ldisc() worker, a reader may have an empty read buffer; the flush_to_ldisc() worker may or may not generate more input to be read. Generally when a reader has an empty read buffer, it will sleep unless one of the other conditions is met (TTY_OTHER_CLOSED, tty_hung_up_p, non-blocking, etc). Before sleeping, the reader will (re-)schedule the flush_to_ldisc() worker in case it read some input on the previous loop iteration (thus creating space in the read buffer when there was none previously). (That doesn't preclude that flush_to_ldisc() may already be running but isn't processing as fast as the reader is reading.) > So even the tty_flush_to_ldisc(tty) that gets done in that patch is > not necessarily sufficient, because the work might not have been > scheduled because the flip buffer used to be full. Then flushing the > work won't do anything, even though there is actually more data. Now, > that is a very unlikely situation (I think it requires two concurrent > readers), but it looks like it might be real. I don't think this condition is possible for a single reader (because the read condition will be satisfied and the reader will return). Multiple concurrent readers are excluded by the atomic_read_lock mutex at the the top of n_tty_read(). > So I suspect we should *unconditionally* do > > n_tty_set_room(tty); > up_read(&tty->termios_rwsem); > tty_flush_to_ldisc(tty); > down_read(&tty->termios_rwsem); > > if we don't have any pending input. And then test input_available_p() > again. And only if we don't have any input after that flushing do we > start doing the whole TTY_OTHER_CLOSED and tty_hung_up_p() tests. > > Hmm? flush_to_ldisc() should not be rescheduled via n_tty_set_room() after the tty has been hung up. This will trigger the diagnostic warning that was introduced back in 3.8 which proved that the line discipline was still running after the tty had been released. Maybe that's less of an issue (or a non-issue) now that the port owns the tty buffers and drivers can push i/o without a tty. I'll give some thought to that. Looking at this problem at a higher level, in some situations it would be desirable for a tty to continue to be readable after hang up (like after carrier loss) but not in others (like exit()). This is something I'm (kind of) working around right now in the tty-over-firewire staging driver by delaying hangup on carrier loss. Regards, Peter Hurley ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PATCH] TTY/Serial fixes for 3.12-rc4 2013-10-05 23:57 ` Peter Hurley @ 2013-10-06 2:11 ` Peter Hurley 2013-10-17 21:12 ` Greg KH 0 siblings, 1 reply; 11+ messages in thread From: Peter Hurley @ 2013-10-06 2:11 UTC (permalink / raw) To: Linus Torvalds Cc: Greg KH, Mikael Pettersson, Jiri Slaby, Andrew Morton, Linux Kernel Mailing List, linux-serial On 10/05/2013 07:57 PM, Peter Hurley wrote: > On 10/05/2013 02:53 PM, Linus Torvalds wrote: >> On Sat, Oct 5, 2013 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote: >>> >>> One fixes the reported regression in the n_tty code that a number of >>> people found recently >> >> That one looks broken. >> >> Well, it looks like it might "work", but do so by hiding the issue for >> one case, while leaving it in the more general case. >> >> Why does it do that >> >> up_read(&tty->termios_rwsem); >> tty_flush_to_ldisc(tty); >> down_read(&tty->termios_rwsem); >> >> only if TTY_OTHER_CLOSED is set? If flushing the ldisc can generate >> more data, then we should do it *unconditionally* when we see that we >> currently have no data to read. >> >> As it is, it looks like the patch fixes the TTY_OTHER_CLOSED case >> ("read all pending data before returning -EIO"), but it leaves the >> same broken case for the O_NONBLOCK case and for a hung up tty. >> >> The O_NONBLOCK case is presumably just a performance problem (the data >> will come at _some_ point later), but it just looks bad in general. >> And the tty_hung_up_p() looks actiely buggy, with the same bug as the >> TTY_OTHER_CLOSED case that the patch tried to fix. >> >> Hmm? Am I missing something? Apologies, I realized I didn't address the O_NONBLOCK case. My reasoning for excluding O_NONBLOCK is that tty_flush_to_ldisc() _waits_ for flush_to_ldisc() to complete. In the worst (admittedly contrived) case, this could be unbounded running time: a sufficiently fast source could keep flush_to_ldisc() running forever by writing 4K chars and then 4k backspaces, ad infinitum. > When a slave pty is closed, it's not hung up specifically so the > master pty can be read. > > The same is not true for a regular tty; when a regular tty is hung up, > all the pending data is vaporized (ie., what the tty layer refers > to as 'flushed'). So checking for more data when tty_hung_up_p() is > true is pointless. > > The distinction is clearer when you consider that even after the slave > pty is closed, the master pty can still be read() even if it wasn't > waiting in n_tty_read() at the time; this is not true of a regular tty, which > cannot be read() after a hangup [tty_hung_up_p() tests if the > file_operations pointer is set to non-operational read/write/ioctl functions]. > > The patch fixes a race condition which is peculiar to ptys only. > >> The code is a bit confusing in *other* ways too: if you look later, it >> does this: >> >> n_tty_set_room(tty); >> >> which is documented to have to happen inside the termios_rwsem. >> HOWEVER, what does that do? It actually does an _asynchronous_ >> queue_work() of &tty->port->buf.work, in case there is now more room, >> and the previous one was blocked. And guess what that workqueue is all >> about? Right: it's flush_to_ldisc() - which is the work that >> tty_flush_to_ldisc() is trying to flush. So we're actually basically >> making sure we've flush the previous pending work. > > The flush_to_ldisc() worker no longer re-schedules itself. > > The flush_to_ldisc() worker is scheduled when one of two events > happen; 1) the driver has just written received data to the tty > buffers, or 2) space has just become available in the N_TTY line > discipline's read buffer when it was previously full. > > The flush_to_ldisc() worker continues to run as long as space > is available in the line discipline's read buffer or until the > tty buffers are empty. > > Concurrently with the flush_to_ldisc() worker, a reader may have > an empty read buffer; the flush_to_ldisc() worker may or may not > generate more input to be read. > > Generally when a reader has an empty read buffer, it will sleep unless > one of the other conditions is met (TTY_OTHER_CLOSED, tty_hung_up_p, > non-blocking, etc). > > Before sleeping, the reader will (re-)schedule the flush_to_ldisc() > worker in case it read some input on the previous loop iteration > (thus creating space in the read buffer when there was none previously). > > (That doesn't preclude that flush_to_ldisc() may already be running > but isn't processing as fast as the reader is reading.) > > >> So even the tty_flush_to_ldisc(tty) that gets done in that patch is >> not necessarily sufficient, because the work might not have been >> scheduled because the flip buffer used to be full. Then flushing the >> work won't do anything, even though there is actually more data. Now, >> that is a very unlikely situation (I think it requires two concurrent >> readers), but it looks like it might be real. > > I don't think this condition is possible for a single reader (because > the read condition will be satisfied and the reader will return). > Multiple concurrent readers are excluded by the atomic_read_lock mutex > at the the top of n_tty_read(). However, the atomic_read_lock exclusion needs to cover the unconditional n_tty_set_room() when leaving n_tty_read() and it doesn't. Thus, the departing reader fails to ensure the subsequent reader has a running flush_to_ldisc() worker. Patch forthcoming. >> So I suspect we should *unconditionally* do >> >> n_tty_set_room(tty); >> up_read(&tty->termios_rwsem); >> tty_flush_to_ldisc(tty); >> down_read(&tty->termios_rwsem); >> >> if we don't have any pending input. And then test input_available_p() >> again. And only if we don't have any input after that flushing do we >> start doing the whole TTY_OTHER_CLOSED and tty_hung_up_p() tests. >> >> Hmm? > > flush_to_ldisc() should not be rescheduled via n_tty_set_room() after > the tty has been hung up. This will trigger the diagnostic warning that > was introduced back in 3.8 which proved that the line discipline > was still running after the tty had been released. Apologies again. This is not true for any existing reader. Regards, Peter Hurley ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PATCH] TTY/Serial fixes for 3.12-rc4 2013-10-06 2:11 ` Peter Hurley @ 2013-10-17 21:12 ` Greg KH 2013-10-23 13:16 ` Peter Hurley 2013-11-07 18:59 ` [PATCH] n_tty: Ensure reader restarts worker for next reader Peter Hurley 0 siblings, 2 replies; 11+ messages in thread From: Greg KH @ 2013-10-17 21:12 UTC (permalink / raw) To: Peter Hurley Cc: Linus Torvalds, Mikael Pettersson, Jiri Slaby, Andrew Morton, Linux Kernel Mailing List, linux-serial On Sat, Oct 05, 2013 at 10:11:25PM -0400, Peter Hurley wrote: > On 10/05/2013 07:57 PM, Peter Hurley wrote: > >On 10/05/2013 02:53 PM, Linus Torvalds wrote: > >>On Sat, Oct 5, 2013 at 10:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote: > >>> > >>>One fixes the reported regression in the n_tty code that a number of > >>>people found recently > >> > >>That one looks broken. > >> > >>Well, it looks like it might "work", but do so by hiding the issue for > >>one case, while leaving it in the more general case. > >> > >>Why does it do that > >> > >> up_read(&tty->termios_rwsem); > >> tty_flush_to_ldisc(tty); > >> down_read(&tty->termios_rwsem); > >> > >>only if TTY_OTHER_CLOSED is set? If flushing the ldisc can generate > >>more data, then we should do it *unconditionally* when we see that we > >>currently have no data to read. > >> > >>As it is, it looks like the patch fixes the TTY_OTHER_CLOSED case > >>("read all pending data before returning -EIO"), but it leaves the > >>same broken case for the O_NONBLOCK case and for a hung up tty. > >> > >>The O_NONBLOCK case is presumably just a performance problem (the data > >>will come at _some_ point later), but it just looks bad in general. > >>And the tty_hung_up_p() looks actiely buggy, with the same bug as the > >>TTY_OTHER_CLOSED case that the patch tried to fix. > >> > >>Hmm? Am I missing something? > > Apologies, I realized I didn't address the O_NONBLOCK case. > > My reasoning for excluding O_NONBLOCK is that tty_flush_to_ldisc() > _waits_ for flush_to_ldisc() to complete. In the worst (admittedly > contrived) case, this could be unbounded running time: a sufficiently > fast source could keep flush_to_ldisc() running forever by writing > 4K chars and then 4k backspaces, ad infinitum. > > > >When a slave pty is closed, it's not hung up specifically so the > >master pty can be read. > > > >The same is not true for a regular tty; when a regular tty is hung up, > >all the pending data is vaporized (ie., what the tty layer refers > >to as 'flushed'). So checking for more data when tty_hung_up_p() is > >true is pointless. > > > >The distinction is clearer when you consider that even after the slave > >pty is closed, the master pty can still be read() even if it wasn't > >waiting in n_tty_read() at the time; this is not true of a regular tty, which > >cannot be read() after a hangup [tty_hung_up_p() tests if the > >file_operations pointer is set to non-operational read/write/ioctl functions]. > > > >The patch fixes a race condition which is peculiar to ptys only. > > > >>The code is a bit confusing in *other* ways too: if you look later, it > >>does this: > >> > >> n_tty_set_room(tty); > >> > >>which is documented to have to happen inside the termios_rwsem. > >>HOWEVER, what does that do? It actually does an _asynchronous_ > >>queue_work() of &tty->port->buf.work, in case there is now more room, > >>and the previous one was blocked. And guess what that workqueue is all > >>about? Right: it's flush_to_ldisc() - which is the work that > >>tty_flush_to_ldisc() is trying to flush. So we're actually basically > >>making sure we've flush the previous pending work. > > > >The flush_to_ldisc() worker no longer re-schedules itself. > > > >The flush_to_ldisc() worker is scheduled when one of two events > >happen; 1) the driver has just written received data to the tty > >buffers, or 2) space has just become available in the N_TTY line > >discipline's read buffer when it was previously full. > > > >The flush_to_ldisc() worker continues to run as long as space > >is available in the line discipline's read buffer or until the > >tty buffers are empty. > > > >Concurrently with the flush_to_ldisc() worker, a reader may have > >an empty read buffer; the flush_to_ldisc() worker may or may not > >generate more input to be read. > > > >Generally when a reader has an empty read buffer, it will sleep unless > >one of the other conditions is met (TTY_OTHER_CLOSED, tty_hung_up_p, > >non-blocking, etc). > > > >Before sleeping, the reader will (re-)schedule the flush_to_ldisc() > >worker in case it read some input on the previous loop iteration > >(thus creating space in the read buffer when there was none previously). > > > >(That doesn't preclude that flush_to_ldisc() may already be running > >but isn't processing as fast as the reader is reading.) > > > > > >>So even the tty_flush_to_ldisc(tty) that gets done in that patch is > >>not necessarily sufficient, because the work might not have been > >>scheduled because the flip buffer used to be full. Then flushing the > >>work won't do anything, even though there is actually more data. Now, > >>that is a very unlikely situation (I think it requires two concurrent > >>readers), but it looks like it might be real. > > > >I don't think this condition is possible for a single reader (because > >the read condition will be satisfied and the reader will return). > >Multiple concurrent readers are excluded by the atomic_read_lock mutex > >at the the top of n_tty_read(). > > However, the atomic_read_lock exclusion needs to cover the > unconditional n_tty_set_room() when leaving n_tty_read() and > it doesn't. Thus, the departing reader fails to ensure the subsequent > reader has a running flush_to_ldisc() worker. > > Patch forthcoming. Did I miss this patch, or did it not come forth? thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PATCH] TTY/Serial fixes for 3.12-rc4 2013-10-17 21:12 ` Greg KH @ 2013-10-23 13:16 ` Peter Hurley 2013-11-07 18:59 ` [PATCH] n_tty: Ensure reader restarts worker for next reader Peter Hurley 1 sibling, 0 replies; 11+ messages in thread From: Peter Hurley @ 2013-10-23 13:16 UTC (permalink / raw) To: Greg KH Cc: Linus Torvalds, Mikael Pettersson, Jiri Slaby, Andrew Morton, Linux Kernel Mailing List, linux-serial On 10/17/2013 05:12 PM, Greg KH wrote: > On Sat, Oct 05, 2013 at 10:11:25PM -0400, Peter Hurley wrote: >> On 10/05/2013 07:57 PM, Peter Hurley wrote: >> >> However, the atomic_read_lock exclusion needs to cover the >> unconditional n_tty_set_room() when leaving n_tty_read() and >> it doesn't. Thus, the departing reader fails to ensure the subsequent >> reader has a running flush_to_ldisc() worker. >> >> Patch forthcoming. > > Did I miss this patch, or did it not come forth? Sorry to go off-grid like that; interrupt(life) happened. I'm testing that patch now. Regards, Peter Hurley ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] n_tty: Ensure reader restarts worker for next reader 2013-10-17 21:12 ` Greg KH 2013-10-23 13:16 ` Peter Hurley @ 2013-11-07 18:59 ` Peter Hurley 2013-11-07 20:01 ` Peter Hurley 2013-11-20 0:21 ` Linus Torvalds 1 sibling, 2 replies; 11+ messages in thread From: Peter Hurley @ 2013-11-07 18:59 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, Linus Torvalds, Mikael Pettersson, Andrew Morton, linux-serial, Peter Hurley, stable A departing reader must restart a flush_to_ldisc() worker _before_ the next reader enters the read loop; this is to avoid the new reader concluding no more i/o is available and prematurely exiting, when the old reader simply hasn't re-started the worker yet. Cc: stable@kernel.org # 3.12 Signed-off-by: Peter Hurley <peter@hurleysoftware.com> --- drivers/tty/n_tty.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 7a744b6..ce11cd5 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -2250,6 +2250,9 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, if (time) timeout = time; } + n_tty_set_room(tty); + up_read(&tty->termios_rwsem); + mutex_unlock(&ldata->atomic_read_lock); remove_wait_queue(&tty->read_wait, &wait); @@ -2260,8 +2263,6 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, if (b - buf) retval = b - buf; - n_tty_set_room(tty); - up_read(&tty->termios_rwsem); return retval; } -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] n_tty: Ensure reader restarts worker for next reader 2013-11-07 18:59 ` [PATCH] n_tty: Ensure reader restarts worker for next reader Peter Hurley @ 2013-11-07 20:01 ` Peter Hurley 2013-11-20 0:21 ` Linus Torvalds 1 sibling, 0 replies; 11+ messages in thread From: Peter Hurley @ 2013-11-07 20:01 UTC (permalink / raw) To: Peter Hurley, Greg Kroah-Hartman Cc: linux-kernel, Linus Torvalds, Mikael Pettersson, Andrew Morton, linux-serial, stable On 11/07/2013 01:59 PM, Peter Hurley wrote: > A departing reader must restart a flush_to_ldisc() worker _before_ > the next reader enters the read loop; this is to avoid the new reader > concluding no more i/o is available and prematurely exiting, when the > old reader simply hasn't re-started the worker yet. > > Cc: stable@kernel.org # 3.12 > Signed-off-by: Peter Hurley <peter@hurleysoftware.com> Greg, I fluffed the stable cc: Please pick this up for 3.12.1. Regards, Peter Hurley ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] n_tty: Ensure reader restarts worker for next reader 2013-11-07 18:59 ` [PATCH] n_tty: Ensure reader restarts worker for next reader Peter Hurley 2013-11-07 20:01 ` Peter Hurley @ 2013-11-20 0:21 ` Linus Torvalds 2013-11-20 0:30 ` Greg Kroah-Hartman 1 sibling, 1 reply; 11+ messages in thread From: Linus Torvalds @ 2013-11-20 0:21 UTC (permalink / raw) To: Peter Hurley Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Mikael Pettersson, Andrew Morton, linux-serial, # .39.x Greg, what happened to this patch? Is it still waiting in some random tree of yours, or did it get lost? Linus On Thu, Nov 7, 2013 at 10:59 AM, Peter Hurley <peter@hurleysoftware.com> wrote: > A departing reader must restart a flush_to_ldisc() worker _before_ > the next reader enters the read loop; this is to avoid the new reader > concluding no more i/o is available and prematurely exiting, when the > old reader simply hasn't re-started the worker yet. > > Cc: stable@kernel.org # 3.12 > Signed-off-by: Peter Hurley <peter@hurleysoftware.com> > --- > drivers/tty/n_tty.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c > index 7a744b6..ce11cd5 100644 > --- a/drivers/tty/n_tty.c > +++ b/drivers/tty/n_tty.c > @@ -2250,6 +2250,9 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, > if (time) > timeout = time; > } > + n_tty_set_room(tty); > + up_read(&tty->termios_rwsem); > + > mutex_unlock(&ldata->atomic_read_lock); > remove_wait_queue(&tty->read_wait, &wait); > > @@ -2260,8 +2263,6 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, > if (b - buf) > retval = b - buf; > > - n_tty_set_room(tty); > - up_read(&tty->termios_rwsem); > return retval; > } > > -- > 1.8.1.2 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] n_tty: Ensure reader restarts worker for next reader 2013-11-20 0:21 ` Linus Torvalds @ 2013-11-20 0:30 ` Greg Kroah-Hartman 2013-11-20 0:43 ` Linus Torvalds 0 siblings, 1 reply; 11+ messages in thread From: Greg Kroah-Hartman @ 2013-11-20 0:30 UTC (permalink / raw) To: Linus Torvalds Cc: Peter Hurley, Linux Kernel Mailing List, Mikael Pettersson, Andrew Morton, linux-serial, # .39.x On Tue, Nov 19, 2013 at 04:21:54PM -0800, Linus Torvalds wrote: > Greg, > what happened to this patch? Is it still waiting in some random tree > of yours, or did it get lost? I was waiting for 3.13-rc1 to be out to populate my trees and then send it on to you. But if you want to take it now, great, I have no objection, otherwise I was going to wait till next week. greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] n_tty: Ensure reader restarts worker for next reader 2013-11-20 0:30 ` Greg Kroah-Hartman @ 2013-11-20 0:43 ` Linus Torvalds 0 siblings, 0 replies; 11+ messages in thread From: Linus Torvalds @ 2013-11-20 0:43 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Peter Hurley, Linux Kernel Mailing List, Mikael Pettersson, Andrew Morton, linux-serial, # .39.x On Tue, Nov 19, 2013 at 4:30 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > I was waiting for 3.13-rc1 to be out to populate my trees and then send > it on to you. > > But if you want to take it now, great, I have no objection, otherwise I > was going to wait till next week. I don't care deeply - it's marled for stable, but the race it fixes is so unlikely as to be almost irrelevant, so it's not like I need it now. I was just going through emails that came in during travel, and making sure it's not lost. So I'll happily forget about it, knowing that it is in your queue. Linus ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-11-20 0:43 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-05 17:34 [GIT PATCH] TTY/Serial fixes for 3.12-rc4 Greg KH 2013-10-05 18:53 ` Linus Torvalds 2013-10-05 23:57 ` Peter Hurley 2013-10-06 2:11 ` Peter Hurley 2013-10-17 21:12 ` Greg KH 2013-10-23 13:16 ` Peter Hurley 2013-11-07 18:59 ` [PATCH] n_tty: Ensure reader restarts worker for next reader Peter Hurley 2013-11-07 20:01 ` Peter Hurley 2013-11-20 0:21 ` Linus Torvalds 2013-11-20 0:30 ` Greg Kroah-Hartman 2013-11-20 0:43 ` Linus Torvalds
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).