* tty->low_latency + irq context
@ 2006-12-26 0:09 Jiri Slaby
2007-01-02 17:17 ` Hollis Blanchard
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2006-12-26 0:09 UTC (permalink / raw)
To: Linux Kernel Mailing List; +Cc: p.hardwick, hollisb
Hi!
* tty_flip_buffer_push - terminal
* @tty: tty to push
*
* Queue a push of the terminal flip buffers to the line discipline. This
* function must not be called from IRQ context if tty->low_latency is set.
But some drivers (mxser, nozomi, hvsi...) sets low_latency to 1 in _open and
calls tty_flip_buffer_push in isr or in functions, which are called from isr.
Is the comment correct or the drivers?
Moreover, hvsi says:
tty->low_latency = 1; /* avoid throttle/tty_flip_buffer_push race */
thanks,
--
http://www.fi.muni.cz/~xslaby/ Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tty->low_latency + irq context
2006-12-26 0:09 tty->low_latency + irq context Jiri Slaby
@ 2007-01-02 17:17 ` Hollis Blanchard
2007-01-02 18:12 ` Paul Fulghum
2007-01-02 18:38 ` Alan
0 siblings, 2 replies; 6+ messages in thread
From: Hollis Blanchard @ 2007-01-02 17:17 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Linux Kernel Mailing List, p.hardwick
On Tue, 2006-12-26 at 01:08 +0059, Jiri Slaby wrote:
> Hi!
>
> * tty_flip_buffer_push - terminal
> * @tty: tty to push
> *
> * Queue a push of the terminal flip buffers to the line discipline. This
> * function must not be called from IRQ context if tty->low_latency is set.
>
> But some drivers (mxser, nozomi, hvsi...) sets low_latency to 1 in _open and
> calls tty_flip_buffer_push in isr or in functions, which are called from isr.
> Is the comment correct or the drivers?
The comment would be true if tty_flip_buffer_push() attempted to block
with tty->low_latency set, but it doesn't AFAICS. One possibility for
deadlock is if the tty->buf.lock spinlock is taken on behalf of a user
process...
> Moreover, hvsi says:
> tty->low_latency = 1; /* avoid throttle/tty_flip_buffer_push race */
That was a long time ago, but the race is something like this:
* data is received, enough to completely fill the tty buffer
* tty_flip_buffer_push() schedules flush_to_ldisc()
* before flush_to_ldisc() runs, more data is received
* flush_to_ldisc() truncates the incoming data (look for
tty->receive_room)
I don't see how this is supposed to work in general. I suppose most
PC-standard char drivers are not capable of overflowing a tty buffer
before the host can empty it. I wasn't comfortable with hoping for that
condition in my driver.
Setting "low_latency" ensures that throttle will be called immediately
if the tty buffer is filled, avoiding the race.
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: tty->low_latency + irq context
2007-01-02 17:17 ` Hollis Blanchard
@ 2007-01-02 18:12 ` Paul Fulghum
2007-01-02 18:29 ` Paul Fulghum
2007-01-02 18:38 ` Alan
1 sibling, 1 reply; 6+ messages in thread
From: Paul Fulghum @ 2007-01-02 18:12 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: Jiri Slaby, Linux Kernel Mailing List, p.hardwick
On Tue, 2007-01-02 at 11:17 -0600, Hollis Blanchard wrote:
> On Tue, 2006-12-26 at 01:08 +0059, Jiri Slaby wrote:
> > * Queue a push of the terminal flip buffers to the line discipline. This
> > * function must not be called from IRQ context if tty->low_latency is set.
> >
> > But some drivers (mxser, nozomi, hvsi...) sets low_latency to 1 in _open and
> > calls tty_flip_buffer_push in isr or in functions, which are called from isr.
> > Is the comment correct or the drivers?
>
> The comment would be true if tty_flip_buffer_push() attempted to block
> with tty->low_latency set, but it doesn't AFAICS. One possibility for
> deadlock is if the tty->buf.lock spinlock is taken on behalf of a user
> process...
There is no deadlock on tty->buf.lock,
which is always acquired with spin_lock_irqsave()
and is only used by the tty buffering code.
The only deadlock I know of with the current tty buffering code
is calling tty_flip_buffer_push() with low_latency
set and from the ISR of a driver that has a problem
with the line discipline calling back into the driver.
The standard serial core has (or at least had the last time
I looked) this problem with the N_TTY ldisc:
driver gets internal spinlock in ISR
driver calls tty_flip_buffer_push with low_latency = 1
flush_to_ldisc() immediately passes data to line discipline
line discipline calls back into driver
driver tries again to get internal spinlock
With low_latency == 1, flush_to_ldisc() is deferred
until the ISR is complete and the internal spinlock is released.
I forget the exact driver callback that caused this.
--
Paul Fulghum
Microgate Systems, Ltd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tty->low_latency + irq context
2007-01-02 17:17 ` Hollis Blanchard
2007-01-02 18:12 ` Paul Fulghum
@ 2007-01-02 18:38 ` Alan
2007-01-02 19:36 ` Hollis Blanchard
1 sibling, 1 reply; 6+ messages in thread
From: Alan @ 2007-01-02 18:38 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: Jiri Slaby, Linux Kernel Mailing List, p.hardwick
> with tty->low_latency set, but it doesn't AFAICS. One possibility for
> deadlock is if the tty->buf.lock spinlock is taken on behalf of a user
> process...
The case to watch out for is
flip_buffer_push -> ldisc -> driver write of echo/^S/^Q
if you call flip_buffer_push while holding your own lock you may get in
a mess on the echo path.
> * data is received, enough to completely fill the tty buffer
> * tty_flip_buffer_push() schedules flush_to_ldisc()
> * before flush_to_ldisc() runs, more data is received
> * flush_to_ldisc() truncates the incoming data (look for
> tty->receive_room)
>
> I don't see how this is supposed to work in general.
For non fake tty hardware at real speeds it wasn't a problem under about
1Mbit. Current tty layer code just uses memory buffering based on kmalloc
and has a 64K limit instead. Works better SMP, scales better and we no
longer need to do stunts like the flip buffers to scrape 56Kbit on a
386SX16
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tty->low_latency + irq context
2007-01-02 18:38 ` Alan
@ 2007-01-02 19:36 ` Hollis Blanchard
0 siblings, 0 replies; 6+ messages in thread
From: Hollis Blanchard @ 2007-01-02 19:36 UTC (permalink / raw)
To: Alan; +Cc: Jiri Slaby, Linux Kernel Mailing List, p.hardwick
On Tue, 2007-01-02 at 18:38 +0000, Alan wrote:
> > with tty->low_latency set, but it doesn't AFAICS. One possibility
> for
> > deadlock is if the tty->buf.lock spinlock is taken on behalf of a
> user
> > process...
>
> The case to watch out for is
>
> flip_buffer_push -> ldisc -> driver write of echo/^S/^Q
>
> if you call flip_buffer_push while holding your own lock you may get
> in a mess on the echo path.
Agreed. However, that's not what the comment says:
* tty_flip_buffer_push - terminal
* @tty: tty to push
*
* Queue a push of the terminal flip buffers to the line discipline. This
* function must not be called from IRQ context if tty->low_latency is set.
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-01-02 19:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-26 0:09 tty->low_latency + irq context Jiri Slaby
2007-01-02 17:17 ` Hollis Blanchard
2007-01-02 18:12 ` Paul Fulghum
2007-01-02 18:29 ` Paul Fulghum
2007-01-02 18:38 ` Alan
2007-01-02 19:36 ` Hollis Blanchard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox