* [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
@ 2014-09-10 21:28 Peter Hurley
2014-09-11 0:03 ` Greg Kroah-Hartman
0 siblings, 1 reply; 10+ messages in thread
From: Peter Hurley @ 2014-09-10 21:28 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, One Thousand Gnomes, linux-serial, linux-kernel,
Peter Hurley
This patch changes user-space behavior (for the better) but I'm not sure
that it's consequence-free. Also, it might not be enough to unwedge the
terminal if the driver got its own flow control state mangled.
Thoughts?
--- >% ---
Subject: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
If terminal flow has been stopped, the terminal can be unwedged
by:
tcflow(fd, TCOOFF);
tcflow(fd, TCOON);
This works because tcflow(TCOOFF) ensures that ->flow_stopped is set,
which allows tcflow(TCOON) to override the terminal flow state in
__start_tty().
Instead, allow unwedging with only:
tcflow(fd, TCOON);
by disregarding the existing ->flow_stopped state.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/tty_ioctl.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 32cee97..c792088 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1157,10 +1157,8 @@ int n_tty_ioctl_helper(struct tty_struct *tty, struct file *file,
break;
case TCOON:
spin_lock_irq(&tty->flow_lock);
- if (tty->flow_stopped) {
- tty->flow_stopped = 0;
- __start_tty(tty);
- }
+ tty->flow_stopped = 0;
+ __start_tty(tty);
spin_unlock_irq(&tty->flow_lock);
break;
case TCIOFF:
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-10 21:28 [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal Peter Hurley
@ 2014-09-11 0:03 ` Greg Kroah-Hartman
2014-09-11 0:11 ` Peter Hurley
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2014-09-11 0:03 UTC (permalink / raw)
To: Peter Hurley; +Cc: Jiri Slaby, One Thousand Gnomes, linux-serial, linux-kernel
On Wed, Sep 10, 2014 at 05:28:19PM -0400, Peter Hurley wrote:
> This patch changes user-space behavior (for the better) but I'm not sure
> that it's consequence-free. Also, it might not be enough to unwedge the
> terminal if the driver got its own flow control state mangled.
>
> Thoughts?
>
> --- >% ---
> Subject: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
>
> If terminal flow has been stopped, the terminal can be unwedged
> by:
> tcflow(fd, TCOOFF);
> tcflow(fd, TCOON);
> This works because tcflow(TCOOFF) ensures that ->flow_stopped is set,
> which allows tcflow(TCOON) to override the terminal flow state in
> __start_tty().
>
> Instead, allow unwedging with only:
> tcflow(fd, TCOON);
> by disregarding the existing ->flow_stopped state.
I don't see the benifit here, what are you trying to solve? Sending one
extra tcflow command?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-11 0:03 ` Greg Kroah-Hartman
@ 2014-09-11 0:11 ` Peter Hurley
2014-09-11 0:24 ` Greg Kroah-Hartman
2014-09-11 10:19 ` One Thousand Gnomes
0 siblings, 2 replies; 10+ messages in thread
From: Peter Hurley @ 2014-09-11 0:11 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, One Thousand Gnomes, linux-serial, linux-kernel
On 09/10/2014 08:03 PM, Greg Kroah-Hartman wrote:
> On Wed, Sep 10, 2014 at 05:28:19PM -0400, Peter Hurley wrote:
>> This patch changes user-space behavior (for the better) but I'm not sure
>> that it's consequence-free. Also, it might not be enough to unwedge the
>> terminal if the driver got its own flow control state mangled.
>>
>> Thoughts?
>>
>> --- >% ---
>> Subject: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
>>
>> If terminal flow has been stopped, the terminal can be unwedged
>> by:
>> tcflow(fd, TCOOFF);
>> tcflow(fd, TCOON);
>> This works because tcflow(TCOOFF) ensures that ->flow_stopped is set,
>> which allows tcflow(TCOON) to override the terminal flow state in
>> __start_tty().
>>
>> Instead, allow unwedging with only:
>> tcflow(fd, TCOON);
>> by disregarding the existing ->flow_stopped state.
>
> I don't see the benifit here, what are you trying to solve? Sending one
> extra tcflow command?
It's not common knowledge (and its certainly counterintuitive) that
turning off output when output is already turned off (ie., tcflow(TCOOFF))
is the required trickery to unwedge a terminal.
Unwedging directly seems the straightforward approach.
Regards,
Peter Hurley
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-11 0:11 ` Peter Hurley
@ 2014-09-11 0:24 ` Greg Kroah-Hartman
2014-09-11 0:45 ` Peter Hurley
2014-09-11 10:19 ` One Thousand Gnomes
1 sibling, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2014-09-11 0:24 UTC (permalink / raw)
To: Peter Hurley; +Cc: Jiri Slaby, One Thousand Gnomes, linux-serial, linux-kernel
On Wed, Sep 10, 2014 at 08:11:14PM -0400, Peter Hurley wrote:
> On 09/10/2014 08:03 PM, Greg Kroah-Hartman wrote:
> > On Wed, Sep 10, 2014 at 05:28:19PM -0400, Peter Hurley wrote:
> >> This patch changes user-space behavior (for the better) but I'm not sure
> >> that it's consequence-free. Also, it might not be enough to unwedge the
> >> terminal if the driver got its own flow control state mangled.
> >>
> >> Thoughts?
> >>
> >> --- >% ---
> >> Subject: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
> >>
> >> If terminal flow has been stopped, the terminal can be unwedged
> >> by:
> >> tcflow(fd, TCOOFF);
> >> tcflow(fd, TCOON);
> >> This works because tcflow(TCOOFF) ensures that ->flow_stopped is set,
> >> which allows tcflow(TCOON) to override the terminal flow state in
> >> __start_tty().
> >>
> >> Instead, allow unwedging with only:
> >> tcflow(fd, TCOON);
> >> by disregarding the existing ->flow_stopped state.
> >
> > I don't see the benifit here, what are you trying to solve? Sending one
> > extra tcflow command?
>
> It's not common knowledge (and its certainly counterintuitive) that
> turning off output when output is already turned off (ie., tcflow(TCOOFF))
> is the required trickery to unwedge a terminal.
>
> Unwedging directly seems the straightforward approach.
What's the odds we break POSIX with this type of change?
I'm all for working around broken hardware in the kernel, but this seems
like a very old issue, if it's even one at all, that we would be
changing for no one who has reported it (that I know of...)
In other words, I'm really leary of changing userspace functionality
without having a real need/reason to.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-11 0:24 ` Greg Kroah-Hartman
@ 2014-09-11 0:45 ` Peter Hurley
2014-09-11 13:56 ` Theodore Ts'o
0 siblings, 1 reply; 10+ messages in thread
From: Peter Hurley @ 2014-09-11 0:45 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, One Thousand Gnomes, linux-serial, linux-kernel
On 09/10/2014 08:24 PM, Greg Kroah-Hartman wrote:
> On Wed, Sep 10, 2014 at 08:11:14PM -0400, Peter Hurley wrote:
>> On 09/10/2014 08:03 PM, Greg Kroah-Hartman wrote:
>>> On Wed, Sep 10, 2014 at 05:28:19PM -0400, Peter Hurley wrote:
>>>> This patch changes user-space behavior (for the better) but I'm not sure
>>>> that it's consequence-free. Also, it might not be enough to unwedge the
>>>> terminal if the driver got its own flow control state mangled.
>>>>
>>>> Thoughts?
>>>>
>>>> --- >% ---
>>>> Subject: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
>>>>
>>>> If terminal flow has been stopped, the terminal can be unwedged
>>>> by:
>>>> tcflow(fd, TCOOFF);
>>>> tcflow(fd, TCOON);
>>>> This works because tcflow(TCOOFF) ensures that ->flow_stopped is set,
>>>> which allows tcflow(TCOON) to override the terminal flow state in
>>>> __start_tty().
>>>>
>>>> Instead, allow unwedging with only:
>>>> tcflow(fd, TCOON);
>>>> by disregarding the existing ->flow_stopped state.
>>>
>>> I don't see the benifit here, what are you trying to solve? Sending one
>>> extra tcflow command?
>>
>> It's not common knowledge (and its certainly counterintuitive) that
>> turning off output when output is already turned off (ie., tcflow(TCOOFF))
>> is the required trickery to unwedge a terminal.
>>
>> Unwedging directly seems the straightforward approach.
>
> What's the odds we break POSIX with this type of change?
SUSv3 and v4 do not specify how tcflow() interacts with flow control
from the remote side, so POSIX breakage is not possible.
> I'm all for working around broken hardware in the kernel, but this seems
> like a very old issue, if it's even one at all, that we would be
> changing for no one who has reported it (that I know of...)
How to unwedge a terminal comes up from time to time.
> In other words, I'm really leary of changing userspace functionality
> without having a real need/reason to.
It's possible that this may cause userspace breakage. Some app
may call tcflow(TCOON) thus accidently overriding the flow control
state when it would have had no effect before.
I'm not invested in this patch so I have no problem if you don't
want to take this.
Regards,
Peter Hurley
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-11 0:45 ` Peter Hurley
@ 2014-09-11 13:56 ` Theodore Ts'o
2014-09-11 15:40 ` Peter Hurley
0 siblings, 1 reply; 10+ messages in thread
From: Theodore Ts'o @ 2014-09-11 13:56 UTC (permalink / raw)
To: Peter Hurley
Cc: Greg Kroah-Hartman, Jiri Slaby, One Thousand Gnomes, linux-serial,
linux-kernel
On Wed, Sep 10, 2014 at 08:45:01PM -0400, Peter Hurley wrote:
> > I'm all for working around broken hardware in the kernel, but this seems
> > like a very old issue, if it's even one at all, that we would be
> > changing for no one who has reported it (that I know of...)
>
> How to unwedge a terminal comes up from time to time.
Are you trying to unwedge a terminal using hardware flow control, or
software flow control?
For software flow control, this is a guarantee that we can make wrt to
the behavior of tcflow(). For hardware flow control, we can't make
any guarantees, whether it's with tcflow(TCOON) or tcflow(TCOOF);
tcflow(TCOON).
> It's possible that this may cause userspace breakage. Some app
> may call tcflow(TCOON) thus accidently overriding the flow control
> state when it would have had no effect before.
It's very likely that an application that would be using tcflow() at
all would first be sending a TCOOFF, and then sending a TCOON. So
this doesn't worry me that much.
Indeed, given that the definition of how TCION and TCIOFF is worded
(send a START or STOP command), it's completely reasonable to
interpret TCOON and TCOOFF as emulating what would happen if the
system received a START or STOP command. (Note though that part of
this is that Posix doesn't define CRTSCTS, so POSIX is entirely silent
on the subject of hardware flow control).
Cheers,
- Ted
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-11 13:56 ` Theodore Ts'o
@ 2014-09-11 15:40 ` Peter Hurley
2014-09-11 15:50 ` Theodore Ts'o
0 siblings, 1 reply; 10+ messages in thread
From: Peter Hurley @ 2014-09-11 15:40 UTC (permalink / raw)
To: Theodore Ts'o, Greg Kroah-Hartman, Jiri Slaby,
One Thousand Gnomes, linux-serial, linux-kernel
On 09/11/2014 09:56 AM, Theodore Ts'o wrote:
> On Wed, Sep 10, 2014 at 08:45:01PM -0400, Peter Hurley wrote:
>>> I'm all for working around broken hardware in the kernel, but this seems
>>> like a very old issue, if it's even one at all, that we would be
>>> changing for no one who has reported it (that I know of...)
>>
>> How to unwedge a terminal comes up from time to time.
>
> Are you trying to unwedge a terminal using hardware flow control, or
> software flow control?
For unwedging software flow control.
Like you point out, unwedging hardware flow control would be more complicated
and less predictable.
> For software flow control, this is a guarantee that we can make wrt to
> the behavior of tcflow(). For hardware flow control, we can't make
> any guarantees, whether it's with tcflow(TCOON) or tcflow(TCOOF);
> tcflow(TCOON).
>
>> It's possible that this may cause userspace breakage. Some app
>> may call tcflow(TCOON) thus accidently overriding the flow control
>> state when it would have had no effect before.
>
> It's very likely that an application that would be using tcflow() at
> all would first be sending a TCOOFF, and then sending a TCOON. So
> this doesn't worry me that much.
>
> Indeed, given that the definition of how TCION and TCIOFF is worded
> (send a START or STOP command), it's completely reasonable to
> interpret TCOON and TCOOFF as emulating what would happen if the
> system received a START or STOP command. (Note though that part of
> this is that Posix doesn't define CRTSCTS, so POSIX is entirely silent
> on the subject of hardware flow control).
This is the basic interpretation I assumed, and most of what the tty core
already did.
Regards,
Peter Hurley
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-11 15:40 ` Peter Hurley
@ 2014-09-11 15:50 ` Theodore Ts'o
0 siblings, 0 replies; 10+ messages in thread
From: Theodore Ts'o @ 2014-09-11 15:50 UTC (permalink / raw)
To: Peter Hurley
Cc: Greg Kroah-Hartman, Jiri Slaby, One Thousand Gnomes, linux-serial,
linux-kernel
On Thu, Sep 11, 2014 at 11:40:20AM -0400, Peter Hurley wrote:
>
> This is the basic interpretation I assumed, and most of what the tty core
> already did.
Yes, I know. :-)
So it's been a long time since I've had anything to do with the tty
code, but FWIW, I'd agree with merging Peter's patch.
Cheers,
- Ted
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-11 0:11 ` Peter Hurley
2014-09-11 0:24 ` Greg Kroah-Hartman
@ 2014-09-11 10:19 ` One Thousand Gnomes
2014-09-11 12:34 ` Peter Hurley
1 sibling, 1 reply; 10+ messages in thread
From: One Thousand Gnomes @ 2014-09-11 10:19 UTC (permalink / raw)
To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
> It's not common knowledge (and its certainly counterintuitive) that
> turning off output when output is already turned off (ie., tcflow(TCOOFF))
> is the required trickery to unwedge a terminal.
>
> Unwedging directly seems the straightforward approach.
Putting the off/on in the man page might be safer still ?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal
2014-09-11 10:19 ` One Thousand Gnomes
@ 2014-09-11 12:34 ` Peter Hurley
0 siblings, 0 replies; 10+ messages in thread
From: Peter Hurley @ 2014-09-11 12:34 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
On 09/11/2014 06:19 AM, One Thousand Gnomes wrote:
>> It's not common knowledge (and its certainly counterintuitive) that
>> turning off output when output is already turned off (ie., tcflow(TCOOFF))
>> is the required trickery to unwedge a terminal.
>>
>> Unwedging directly seems the straightforward approach.
>
> Putting the off/on in the man page might be safer still ?
That's my plan if this patch fails to launch.
Regards,
Peter Hurley
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-09-11 15:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-10 21:28 [RFC] tty: Always allow tcflow(TCOON) to unwedge terminal Peter Hurley
2014-09-11 0:03 ` Greg Kroah-Hartman
2014-09-11 0:11 ` Peter Hurley
2014-09-11 0:24 ` Greg Kroah-Hartman
2014-09-11 0:45 ` Peter Hurley
2014-09-11 13:56 ` Theodore Ts'o
2014-09-11 15:40 ` Peter Hurley
2014-09-11 15:50 ` Theodore Ts'o
2014-09-11 10:19 ` One Thousand Gnomes
2014-09-11 12:34 ` Peter Hurley
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).