Re: uart : lost characters when system is busy

[Matthieu CASTET <matthieu.castet@parrot.com>]

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