serial.c: Fix a race condition noted by Stefan Kolb. Between the test if the TX buffer is full and entering a critical section, bytes may be removed from the TX buffer making the wait unnecessary. The unnecessary wait is an inefficiency, but not really a problem. But with USB CDC/ACM it can be a problem because the entire TX buffer may be emptied when we lose the race. If that happens that uart_putxmitchar() can hang waiting for data to be removed from an empty TX buffer.
This commit is contained in:
parent
369c942605
commit
9a8c3572db
@ -208,50 +208,75 @@ static int uart_putxmitchar(FAR uart_dev_t *dev, int ch, bool oktoblock)
|
||||
nexthead = 0;
|
||||
}
|
||||
|
||||
/* Loop until we are able to add the character to the TX buffer */
|
||||
/* Loop until we are able to add the character to the TX buffer. */
|
||||
|
||||
for (; ; )
|
||||
{
|
||||
/* Check if the TX buffer is full */
|
||||
|
||||
if (nexthead != dev->xmit.tail)
|
||||
{
|
||||
/* No.. not full. Add the character to the TX buffer and return. */
|
||||
|
||||
dev->xmit.buffer[dev->xmit.head] = ch;
|
||||
dev->xmit.head = nexthead;
|
||||
return OK;
|
||||
}
|
||||
|
||||
/* The buffer is full and no data is available now. Should be block,
|
||||
* waiting for the hardware to remove some data from the TX
|
||||
* buffer?
|
||||
/* The TX buffer is full. Should be block, waiting for the hardware
|
||||
* to remove some data from the TX buffer?
|
||||
*/
|
||||
|
||||
else if (oktoblock)
|
||||
{
|
||||
/* Inform the interrupt level logic that we are waiting. This and
|
||||
* the following steps must be atomic.
|
||||
/* The following steps must be atomic with respect to serial
|
||||
* interrupt handling.
|
||||
*/
|
||||
|
||||
flags = enter_critical_section();
|
||||
|
||||
/* Check again... In certain race conditions an interrupt may
|
||||
* have occurred between the test at the top of the loop and
|
||||
* entering the critical section and the TX buffer may no longer
|
||||
* be full.
|
||||
*
|
||||
* NOTE: On certain devices, such as USB CDC/ACM, the entire TX
|
||||
* buffer may have been emptied in this race condition. In that
|
||||
* case, the logic would hang below waiting for space in the TX
|
||||
* buffer without this test.
|
||||
*/
|
||||
|
||||
if (nexthead != dev->xmit.tail)
|
||||
{
|
||||
ret = OK;
|
||||
}
|
||||
|
||||
#ifdef CONFIG_SERIAL_REMOVABLE
|
||||
/* Check if the removable device is no longer connected while we
|
||||
* have interrupts off. We do not want the transition to occur
|
||||
* as a race condition before we begin the wait.
|
||||
*/
|
||||
|
||||
if (dev->disconnected)
|
||||
else if (dev->disconnected)
|
||||
{
|
||||
ret = -ENOTCONN;
|
||||
}
|
||||
else
|
||||
#endif
|
||||
else
|
||||
{
|
||||
/* Wait for some characters to be sent from the buffer with
|
||||
* the TX interrupt enabled. When the TX interrupt is
|
||||
* enabled, uart_xmitchars should execute and remove some
|
||||
* of the data from the TX buffer.
|
||||
*/
|
||||
/* Inform the interrupt level logic that we are waiting. */
|
||||
|
||||
dev->xmitwaiting = true;
|
||||
|
||||
/* Wait for some characters to be sent from the buffer with
|
||||
* the TX interrupt enabled. When the TX interrupt is enabled,
|
||||
* uart_xmitchars() should execute and remove some of the data
|
||||
* from the TX buffer.
|
||||
*
|
||||
* NOTE that interrupts will be re-enabled while we wait for
|
||||
* the semaphore.
|
||||
*/
|
||||
|
||||
#ifdef CONFIG_SERIAL_DMA
|
||||
uart_dmatxavail(dev);
|
||||
#endif
|
||||
|
Loading…
x
Reference in New Issue
Block a user