Skip to contentSkip to conversation

uart : lost characters when system is busy

DISCUSSION Matthieu CASTET 2 participants 4 messages Jun 10, 2011 TTY layer and serial drivers

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 4 lines

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.

Quoted 2 lines

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 ?

#2 lore raw
Quoted 10 lines · Alan Cox
Quoted 4 lines

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.

Quoted 4 lines
Quoted 3 lines

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

Quoted 8 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 3 lines

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.

Quoted 12 lines

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