tty_flip_buffer() from atomic context when low_latency==1

tty_flip_buffer() from atomic context when low_latency==1

[Grant Edwards]

I've been looking at various drivers to try to figure out how to
receive a block of serial data in an atomic context.  All the examples
I can find buffer the receive data with calls to
tty_insert_flip_string() or tty_insert_flip_char(), then they call
tty_flip_buffer_push().

But, many drivers appear to call tty_flip_buffer_push() from an atomic
context. If that is done with tty->low_latency==1 it turns into a call
to flush_to_ldisc(), which then calls disc->ops->receive_buf(), which
must not be called from an atomic context.

Is the low_latency flag being forced to 0 somewhere behind the scenes
when tty_flip_buffer_push() is being called from an atomic context?

-- 
Grant Edwards               grant.b.edwards        Yow! I'm receiving a coded
                                  at               message from EUBIE BLAKE!!
                              gmail.com            

Re: tty_flip_buffer() from atomic context when low_latency==1

[Alan Cox]

> But, many drivers appear to call tty_flip_buffer_push() from an atomic
> context. If that is done with tty->low_latency==1 it turns into a call
> to flush_to_ldisc(), which then calls disc->ops->receive_buf(), which
> must not be called from an atomic context.

It's covered under "So don't do that then".

> Is the low_latency flag being forced to 0 somewhere behind the scenes
> when tty_flip_buffer_push() is being called from an atomic context?

No. The driver authors are assumed to have a modicum of competence and if
they don't usually a shower of lock debug messages enlightens
appropriately.

Alan

Re: tty_flip_buffer() from atomic context when low_latency==1

[Grant Edwards]

On 2011-08-19, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

>> But, many drivers appear to call tty_flip_buffer_push() from an atomic
>> context. If that is done with tty->low_latency==1 it turns into a call
>> to flush_to_ldisc(), which then calls disc->ops->receive_buf(), which
>> must not be called from an atomic context.
>
> It's covered under "So don't do that then".

I don't do that, and wasn't asking if I should.

My point was that most of the drivers in the linux kernels sources
_do_ appear to do that, and I don't see how they avoid problems.

For exaple, 8250.c calls tty_flip_buffer_push() from receive_chars(),
calld from serial8250_handle_port(), called from
serial8250_interrupt().

How are problems avoided when tty->low_latency is set?

Is tty->low_latency never allowed to be set for the 8250 driver?

-- 
Grant Edwards               grant.b.edwards        Yow! VICARIOUSLY experience
                                  at               some reason to LIVE!!
                              gmail.com            

Re: tty_flip_buffer() from atomic context when low_latency==1

[Alan Cox]

On Fri, 19 Aug 2011 20:44:26 +0000 (UTC)
Grant Edwards <grant.b.edwards@gmail.com> wrote:

> On 2011-08-19, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> >> But, many drivers appear to call tty_flip_buffer_push() from an atomic
> >> context. If that is done with tty->low_latency==1 it turns into a call
> >> to flush_to_ldisc(), which then calls disc->ops->receive_buf(), which
> >> must not be called from an atomic context.
> >
> > It's covered under "So don't do that then".
> 
> I don't do that, and wasn't asking if I should.
> 
> My point was that most of the drivers in the linux kernels sources
> _do_ appear to do that, and I don't see how they avoid problems.
> 
> For exaple, 8250.c calls tty_flip_buffer_push() from receive_chars(),
> calld from serial8250_handle_port(), called from
> serial8250_interrupt().
> 
> How are problems avoided when tty->low_latency is set?
> 
> Is tty->low_latency never allowed to be set for the 8250 driver?

Several drivers certainly allow someone with superuser privileges to
misconfigure them. I've never worried about chasing all those down.

It might actually be worth supporting a tty->low_latency mode which uses
a tty layer private workqueue that is set to real time priority rather
than the system work queue.

Alan

Re: tty_flip_buffer() from atomic context when low_latency==1

[Grant Edwards]

On 2011-08-19, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Fri, 19 Aug 2011 20:44:26 +0000 (UTC)
> Grant Edwards <grant.b.edwards@gmail.com> wrote:
>
>> On 2011-08-19, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> 
>> >> But, many drivers appear to call tty_flip_buffer_push() from an atomic
>> >> context. If that is done with tty->low_latency==1 it turns into a call
>> >> to flush_to_ldisc(), which then calls disc->ops->receive_buf(), which
>> >> must not be called from an atomic context.
>> >
>> > It's covered under "So don't do that then".
>> 
>> I don't do that, and wasn't asking if I should.
>> 
>> My point was that most of the drivers in the linux kernels sources
>> _do_ appear to do that, and I don't see how they avoid problems.
>> 
>> For exaple, 8250.c calls tty_flip_buffer_push() from receive_chars(),
>> calld from serial8250_handle_port(), called from
>> serial8250_interrupt().
>> 
>> How are problems avoided when tty->low_latency is set?
>> 
>> Is tty->low_latency never allowed to be set for the 8250 driver?
>
> Several drivers certainly allow someone with superuser privileges to
> misconfigure them.

You don't need to be root to set the low_latency flag on a tty device.
Normal users are allowed to do that:

./linux/serial_core.h:361

#define UPF_USR_MASK  ((__force upf_t) (UPF_SPD_MASK|UPF_LOW_LATENCY))

> I've never worried about chasing all those down.

To me, it looks like anybody with permission for the serial port can
set the low latency flag and potentially cause a "scheduling while
atomic" crash.

I've gotten it to happen with other drivers that call
tty_flip_buffer_push() with tty->low_latency==1, but haven't been able
to make it happen with the 8250 driver -- I don't know why.  I've
verified that a normal user doing "setserial /dev/ttyS0 low_latency"
sets the low_latency flag, and it's 1 immediately before and after the
call to tty_flip_buffer_push().  Maybe I just haven't run it long
enough...

> It might actually be worth supporting a tty->low_latency mode which
> uses a tty layer private workqueue that is set to real time priority
> rather than the system work queue.

I don't understand why the low_latency flag even exists when setting
it apparently breaks most all of the serial drivers.

-- 
Grant Edwards               grant.b.edwards        Yow! You should all JUMP
                                  at               UP AND DOWN for TWO HOURS
                              gmail.com            while I decide on a NEW
                                                   CAREER!!

Re: tty_flip_buffer() from atomic context when low_latency==1

[Alan Cox]

> I don't understand why the low_latency flag even exists when setting
> it apparently breaks most all of the serial drivers.

Mostly history. I'd not realised random users could fiddle with it - they
shouldn't be able to do that so that wants fixing.

Historically it made sense as with a 100Hz clock some algorithms with
drivers that delayed stuff a lot got really slow (eg non windowed
versions of firmware download protocols, kermit, xmodem etc)

Nowdays the kernel is a bit more sophisticated and we should possibly
re-arrange it so that it selects different workqueues or similar -
low_latency meaning 'use a private hard rt work queue' perhaps.

Or indeed possibly making ttys use threaded IRQs

Re: tty_flip_buffer() from atomic context when low_latency==1

[Grant Edwards]

On 2011-08-19, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> I don't understand why the low_latency flag even exists when setting
>> it apparently breaks most all of the serial drivers.
>
> Mostly history. I'd not realised random users could fiddle with it - they
> shouldn't be able to do that so that wants fixing.
>
> Historically it made sense as with a 100Hz clock some algorithms with
> drivers that delayed stuff a lot got really slow (eg non windowed
> versions of firmware download protocols, kermit, xmodem etc)

I know why it used to be there, and "back in the day" setting it made
a very noticable difference in the latency on the receive data path.
In some situations, setting the low_latency flag could increase by a a
factor of 10 the number of command/reply transactions you could do per
second (particularly at high baud rates and commands/repsonses are
only a couple bytes).

> Nowdays the kernel is a bit more sophisticated and we should possibly
> re-arrange it so that it selects different workqueues or similar -
> low_latency meaning 'use a private hard rt work queue' perhaps.
>
> Or indeed possibly making ttys use threaded IRQs

For most of the drivers I'm maintaining, I've decided I'm going to
have to force low_latency = 0.  There is one driver that has one mode
where the receive data is handled in a non-atomic context, so that one
can still honor the low_latency flag, but the rest will have to force
it to 0.

[Why this only became a problem recently, I don't know.]

-- 
Grant Edwards               grant.b.edwards        Yow! Am I in Milwaukee?
                                  at               
                              gmail.com            

Re: tty_flip_buffer() from atomic context when low_latency==1

[Alan Cox]

> [Why this only became a problem recently, I don't know.]

In a word - multiprocessing. In 2.4 and 2.4 the tty layer locking was
very broken, in 2.6 its rather better but the side effect of that is all
the unsafe uses of low_latency now blow up and get caught instead of
working most of the time then going bang.

Alan