tty: Fix logic change introduced by wait_event_interruptible_timeout()
PATCH 1/1 tty: Fix logic change introduced by wait_event_interruptible_timeout()
tty: Fix logic change introduced by wait_event_interruptible_timeout()
Commit 5a52bd4a2dcb570333ce6fe2e16cd311650dbdc8 introduced a subtle logic change in tty_wait_until_sent(). The original version would only error out of the 'do { ... } while (timeout)' loop if signal_pending() evaluated to true; a timeout or break due to an empty buffer would fall out of the loop and into the tty->driver->wait_until_sent handling. The current implementation will error out on either a pending signal or an empty buffer, falling through to the tty->driver->wait_until_sent handling only on a timeout.
This change reverts the logic flow in tty_wait_until_sent() to match that prior to the aforementioned commit.
---
Please CC me on any replies; I'm not subscribed to lkml. Thanks.
drivers/char/tty_ioctl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/char/tty_ioctl.c +1 -1
@@ -62,7 +62,7 @@ void tty_wait_until_sent(struct tty_struct * tty, long timeout)if (!timeout) timeout =MAX_SCHEDULE_TIMEOUT ;if (wait_event_interruptible_timeout (tty->write_wait, - !tty->driver->chars_in_buffer (tty), timeout)) + !tty->driver->chars_in_buffer (tty), timeout) <0 )return ;if (tty->driver->wait_until_sent) tty->driver->wait_until_sent (tty, timeout);
Signature · 13 lines
Cory T. Tusar Embedded Systems Engineer Videon Central, Inc. 2171 Sandy Drive State College, PA 16803 (814) 235-1111 x316 (814) 235-1118 fax "There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies, and the other way is to make it so complicated that there are no obvious deficiencies." --Sir Charles Anthony Richard Hoare
Quoted 29 lines · On Thu, 20 Dec 2007 16:39:21 -0500 "Cory T. Tusar" <[email protected]>…
tty: Fix logic change introduced by
wait_event_interruptible_timeout()Commit
5a52bd4a2dcb570333ce6fe2e16cd311650dbdc8introduced a subtle logic change intty_wait_until_sent(). The original version would only error out of the 'do { ... } while (timeout)' loop ifsignal_pending()evaluated to true; a timeout or break due to an empty buffer would fall out of the loop and into thetty->driver->wait_until_senthandling. The current implementation will error out on either a pending signal or an empty buffer, falling through to thetty->driver->wait_until_senthandling only on a timeout.This change reverts the logic flow in
tty_wait_until_sent()to match that prior to the aforementioned commit.Signed-off-by Cory T. Tusar <[email protected]>---
Please CC me on any replies; I'm not subscribed to lkml. Thanks.drivers/char/tty_ioctl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/char/tty_ioctl.c +0
-0
drivers/char/tty_ioctl.c void tty_wait_until_sent(struct tty_struct * tty, long timeout) · line 68
Quoted 8 lines
drivers/char/tty_ioctl.c +1
-1@@ -62,7 +62,7 @@ void tty_wait_until_sent(struct tty_struct * tty, long timeout)if (!timeout) timeout =MAX_SCHEDULE_TIMEOUT ;if (wait_event_interruptible_timeout (tty->write_wait, - !tty->driver->chars_in_buffer (tty), timeout)) + !tty->driver->chars_in_buffer (tty), timeout) <0 )return ;if (tty->driver->wait_until_sent) tty->driver->wait_until_sent (tty, timeout);
OK... So what are the user-visible effects of this regression? The caller will run ->wait_until_sent() even after being signalled? So the signal is not promptly responded to?
Quoted 33 lines · Andrew Morton
Quoted 31 lines · On Thu, 20 Dec 2007 16:39:21 -0500 "Cory T. Tusar" <[email protected]>…
tty: Fix logic change introduced by
wait_event_interruptible_timeout()Commit
5a52bd4a2dcb570333ce6fe2e16cd311650dbdc8introduced a subtle logic change intty_wait_until_sent(). The original version would only error out of the 'do { ... } while (timeout)' loop ifsignal_pending()evaluated to true; a timeout or break due to an empty buffer would fall out of the loop and into thetty->driver->wait_until_senthandling. The current implementation will error out on either a pending signal or an empty buffer, falling through to thetty->driver->wait_until_senthandling only on a timeout.This change reverts the logic flow in
tty_wait_until_sent()to match that prior to the aforementioned commit.Signed-off-by Cory T. Tusar <[email protected]>---
Please CC me on any replies; I'm not subscribed to lkml. Thanks.drivers/char/tty_ioctl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/char/tty_ioctl.c +0
-1@@ -62,7 +62,7 @@ void tty_wait_until_sent(struct tty_struct * tty, long timeout)if (!timeout) timeout =MAX_SCHEDULE_TIMEOUT ; if (wait_event_interruptible_timeout(tty->write_wait, - !tty->driver->chars_in_buffer (tty), timeout))
Quoted 8 lines
drivers/char/tty_ioctl.c void tty_wait_until_sent(struct tty_struct * tty, long timeout) · line 68
Quoted 4 lines
drivers/char/tty_ioctl.c +1
-0+ !tty->driver->chars_in_buffer(tty), timeout) < 0)return ;if (tty->driver->wait_until_sent) tty->driver->wait_until_sent (tty, timeout);OK... So what are the user-visible effects of this regression? The caller will run ->wait_until_sent() even after being signalled? So the signal is not promptly responded to?
No, the ->wait_until_sent() will not be reached if the buffer empties before timeout jiffies have elapsed. This behavior differs from that prior to commit 5a52bd4a2dcb570333ce6fe2e16cd311650dbdc8.
I turned this up while using a little serial download utility to bootstrap an ARM-based eval board. The util worked fine on 2.6.22.x, but consistently failed on 2.6.23.x. Once I'd determined that, I narrowed things down with git bisect, and found the above difference in logic in tty_wait_until_sent() by inspection.
If you'd like a more in-depth analysis of what's breaking on the user-space side, I can certainly investigate further and (hopefully) come up with a simple test case that tickles this issue.
Signature · 13 lines
Cory T. Tusar Embedded Systems Engineer Videon Central, Inc. 2171 Sandy Drive State College, PA 16803 (814) 235-1111 x316 (814) 235-1118 fax "There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies, and the other way is to make it so complicated that there are no obvious deficiencies." --Sir Charles Anthony Richard Hoare
Quoted 11 lines · Cory T. Tusar
tty: Fix logic change introduced by
wait_event_interruptible_timeout()Commit
5a52bd4a2dcb570333ce6fe2e16cd311650dbdc8introduced a subtle logic change intty_wait_until_sent(). The original version would only error out of the 'do { ... } while (timeout)' loop ifsignal_pending()evaluated to true; a timeout or break due to an empty buffer would fall out of the loop and into thetty->driver->wait_until_senthandling. The current implementation will error out on either a pending signal or an empty buffer, falling through to thetty->driver->wait_until_senthandling only on a timeout.
Quoted 4 lines
This change reverts the logic flow in
tty_wait_until_sent()to match that prior to the aforementioned commit.Signed-off-by Cory T. Tusar <[email protected]>
Quoted 12 lines
---
Please CC me on any replies; I'm not subscribed to lkml. Thanks.drivers/char/tty_ioctl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/char/tty_ioctl.c +0
-0
drivers/char/tty_ioctl.c void tty_wait_until_sent(struct tty_struct * tty, long timeout) · line 66
Quoted 6 lines
drivers/char/tty_ioctl.c +1
-1@@ -62,7 +62,7 @@ void tty_wait_until_sent(struct tty_struct * tty, long timeout)if (!timeout) timeout =MAX_SCHEDULE_TIMEOUT ;if (wait_event_interruptible_timeout (tty->write_wait, - !tty->driver->chars_in_buffer (tty), timeout)) + !tty->driver->chars_in_buffer (tty), timeout) <0 )return ;
Thanks for tracking this down.