Aksal/nuttx
3
1
Fork 1
mirror of https://github.com/apache/nuttx.git synced 2026-08-09 14:35:08 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

drivers/usbdev/cdcacm: serialize the TX ring drain with the class spinlock

Browse source 
cdcacm_sndpacket() runs from task context and from the bulk IN
completion callback, which may be interrupt context.  cdcuart_dmasend()
advances the xmit tail non-atomically, so a completion arriving mid
setup re-sends the same region and advances the tail past the head,
re-transmitting a ring of stale data.

c497c5feb0 dropped the critical section that used to cover this.
Restore it with priv->lock held across the setup and EP_SUBMIT; the
submit must stay inside to keep request order.  cdcuart_dmasend() now
runs with the lock held, so its own acquisition is removed.

The race needs the writer to keep the ring non-empty across
completions, so it only appears at high sustained write rates.  On
nRF52840, 131072-byte writes were received as ~147600 bytes - one extra
CDCACM_TXBUFSIZE of stale data per hit.  With this change the host
receives exactly what was sent.

Assisted-by: Claude Code
Signed-off-by: raiden00pl <raiden00@railab.me>
This commit is contained in:
raiden00pl 2026-07-21 15:26:35 +02:00 • committed by Xiang Xiao
parent 60abe4744f
commit 58d1d9f9d6
1 changed files with 10 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

14
drivers/usbdev/cdcacm.c
View file

@ -424,9 +424,9 @@ static int cdcacm_sndpacket(FAR struct cdcacm_dev_s *priv)
FAR struct uart_dev_s *dev = &priv->serdev;
FAR struct cdcacm_wrreq_s *wrcontainer;
FAR struct usbdev_req_s *req;
irqstate_t flags;
int ret;
#endif
irqstate_t flags;
#ifdef CONFIG_DEBUG_FEATURES
if (priv == NULL)
@ -485,10 +485,17 @@ static int cdcacm_sndpacket(FAR struct cdcacm_dev_s *priv)
spin_unlock_irqrestore_nopreempt(&priv->lock, flags);
#else
/* Serialize against the write completion callback, which may call
* this from interrupt context.
*/
flags = spin_lock_irqsave(&priv->lock);
if (!sq_empty(&priv->txfree))
{
uart_xmitchars_dma(&priv->serdev);
}
spin_unlock_irqrestore(&priv->lock, flags);
#endif
out:
@ -3003,6 +3010,8 @@ static int cdcuart_release(FAR struct uart_dev_s *dev)
* Description:
* Set up to transfer bytes from the TX circular buffer.
*
* Called from cdcacm_sndpacket() with priv->lock held.
*
****************************************************************************/
static void cdcuart_dmasend(FAR struct uart_dev_s *dev)
@ -3012,7 +3021,6 @@ static void cdcuart_dmasend(FAR struct uart_dev_s *dev)
FAR struct usbdev_ep_s *ep = priv->epbulkin;
FAR struct cdcacm_wrreq_s *wrcontainer;
FAR struct usbdev_req_s *req;
irqstate_t flags;
size_t nbytes;
size_t reqlen;
int ret;
@ -3023,11 +3031,9 @@ static void cdcuart_dmasend(FAR struct uart_dev_s *dev)
/* Peek at the request in the container at the head of the list */
flags = spin_lock_irqsave(&priv->lock);
wrcontainer = (FAR struct cdcacm_wrreq_s *)sq_remfirst(&priv->txfree);
req = wrcontainer->req;
priv->nwrq--;
spin_unlock_irqrestore(&priv->lock, flags);
/* Fill the request with serial TX data */