uart : lost characters when system is busy

uart : lost characters when system is busy

[Matthieu CASTET]

Hi,

the linux uart layer can loose some characters if the system is busy.

uart_throttle/uart_unthrottle is called from a workqueue.
If the system is busy, and the uart receive lot's of data, we fill the tty
buffer, but the workqueue doesn't run and we never have a chance to call
uart_throttle. So the uart is never slow down.

And because most uart driver call uart_insert_char (that doesn't return if
tty_insert_flip_char manage to push the character), we never detect that there
are some lost characters.


A workaround could be to check the buffer threshold in tty_flip_buffer_push and
call throttle callback if needed.


Matthieu

Re: uart : lost characters when system is busy

[Alan Cox]

> uart_throttle/uart_unthrottle is called from a workqueue.
> If the system is busy, and the uart receive lot's of data, we fill the tty
> buffer, but the workqueue doesn't run and we never have a chance to call
> uart_throttle. So the uart is never slow down.

You should have around 64K of buffering (actually n_tty worst case
should be 63.5Kbyte) that's a lot of slack so what is holding off the
work queue for so long on your problem system ? I think that should be
answered first as it sounds like some other part of your kernel is
misbehaving.

> A workaround could be to check the buffer threshold in tty_flip_buffer_push and
> call throttle callback if needed.

tty_flip_buffer_push can be called from an IRQ, the throttle callback
needs to be able to sleep.

What might work if it is needed though is to provide a tty_is_throttled()
method that a driver can use to check the instantaneous throttle state.
Trouble is that will require a lot of care on the drivers part to deal
with asynchronus throttle/unthrottle events while peering at the state in
its IRQ handler as well.

Anyway - question for the case you hit, what tasks or work held off the
serial work queue for 63.5Kbytes of data ?

Re: uart : lost characters when system is busy

[Matthieu CASTET]

Alan Cox a écrit :
>> uart_throttle/uart_unthrottle is called from a workqueue.
>> If the system is busy, and the uart receive lot's of data, we fill the tty
>> buffer, but the workqueue doesn't run and we never have a chance to call
>> uart_throttle. So the uart is never slow down.
> 
> You should have around 64K of buffering (actually n_tty worst case
> should be 63.5Kbyte) that's a lot of slack so what is holding off the
> work queue for so long on your problem system ? I think that should be
> answered first as it sounds like some other part of your kernel is
> misbehaving.
The uart is connected to a BT chip, and it is configured to 3 Mbps.
64K buffer is 166 ms.

Some task on the system have higher priority than the worker thread (rt priority
or cgroup), and can preempt it more than 166 ms.

Also I believe some virtual usb uart (3G dongle, cdc-acm) that use higher rate
(more than 10 Mbps)  will have the same problem. They will fill the buffer very
quickly.

> 
>> A workaround could be to check the buffer threshold in tty_flip_buffer_push and
>> call throttle callback if needed.
> 
> tty_flip_buffer_push can be called from an IRQ, the throttle callback
> needs to be able to sleep.
> 
> What might work if it is needed though is to provide a tty_is_throttled()
> method that a driver can use to check the instantaneous throttle state.
> Trouble is that will require a lot of care on the drivers part to deal
> with asynchronus throttle/unthrottle events while peering at the state in
> its IRQ handler as well.
I think tty_is_throttled() is better than checking tty_insert_flip_char status,
because we know earlier that the fifo is becoming full and we don't have to save
the remaining data somewhere.

For example the "USB: cdc-acm: Prevent loss of data when filling tty buffer"
patch may be changed to something like :

    /* throttle device if requested by tty */
    spin_lock_irqsave(&acm->read_lock, flags);
    acm->throttled = acm->throttle_req;
    if (!acm->throttled && !acm->susp_count && !tty_is_throttled()) {
        spin_unlock_irqrestore(&acm->read_lock, flags);
        acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
    } else {
        spin_unlock_irqrestore(&acm->read_lock, flags);
    }

Matthieu
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: uart : lost characters when system is busy

[Alan Cox]

> Also I believe some virtual usb uart (3G dongle, cdc-acm) that use higher rate
> (more than 10 Mbps)  will have the same problem. They will fill the buffer very
> quickly.

PPP doesn't use throttling so those dongles which are use modem emulation
will flow control at the IP level which is probably better anyway.

> For example the "USB: cdc-acm: Prevent loss of data when filling tty buffer"
> patch may be changed to something like :
> 
>     /* throttle device if requested by tty */
>     spin_lock_irqsave(&acm->read_lock, flags);
>     acm->throttled = acm->throttle_req;
>     if (!acm->throttled && !acm->susp_count && !tty_is_throttled()) {
>         spin_unlock_irqrestore(&acm->read_lock, flags);
>         acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
>     } else {
>         spin_unlock_irqrestore(&acm->read_lock, flags);
>     }

Except this now races the handling in the throttle/unthrottle events. In
particular you could have an unthrottle queued to occur such that by the
time it occurs we've throttled again, plus the ldisc may be quite slack
in signalling it to the tty layer.

For that example a second approach would be to provide

static int tty_buffer_room(struct tty *tty)
{
	return 65536 - tty->buf.memory_used;
}

and you could then buffer against the queue size, which is easy to
monitor in IRQ state, but would mean that you'd need to be sure that you
always had some buffer posted or some way to recover from the no buffers
case (eg adding

	if (tty-ops->buffer_free)
		tty->ops->buffer_free(tty)

to tty_buffer_free())

At that point it is throttling against the queue that is handled in
interrupt space.

Care to prototype those bits in your tree and see if it sorts your use
case ?

Alan