[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dmaengine
Subject:    Re: [PATCH 1/2] serial: sh-sci: Fix TX DMA buffer flushing and workqueue races
From:       Eugeniu Rosca <erosca () de ! adit-jv ! com>
Date:       2019-06-28 12:53:19
Message-ID: 20190628125319.GB10962 () vmlxhi-102 ! adit-jv ! com
[Download RAW message or body]

Hi Geert,

One bikeshedding below.

On Mon, Jun 24, 2019 at 02:35:39PM +0200, Geert Uytterhoeven wrote:
[..]
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index abc705716aa094fd..d4504daff99263f5 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1398,6 +1398,7 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
>  	struct circ_buf *xmit = &port->state->xmit;
>  	unsigned long flags;
>  	dma_addr_t buf;
> +	int head, tail;
>  
>  	/*
>  	 * DMA is idle now.
> @@ -1407,16 +1408,23 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
>  	 * consistent xmit buffer state.
>  	 */
>  	spin_lock_irq(&port->lock);
> -	buf = s->tx_dma_addr + (xmit->tail & (UART_XMIT_SIZE - 1));
> +	head = xmit->head;
> +	tail = xmit->tail;
> +	buf = s->tx_dma_addr + (tail & (UART_XMIT_SIZE - 1));
>  	s->tx_dma_len = min_t(unsigned int,
> -		CIRC_CNT(xmit->head, xmit->tail, UART_XMIT_SIZE),
> -		CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE));
> -	spin_unlock_irq(&port->lock);
> +		CIRC_CNT(head, tail, UART_XMIT_SIZE),
> +		CIRC_CNT_TO_END(head, tail, UART_XMIT_SIZE));
> +	if (!s->tx_dma_len) {
> +		/* Transmit buffer has been flushed */
> +		spin_unlock_irq(&port->lock);
> +		return;

Since we can now return before using the result stored in 'buf', we
could relocate the 'buf' calculation next to the return statement, just
before calling dmaengine_prep_slave_single(), as micro-optimization.

> +	}
>  
>  	desc = dmaengine_prep_slave_single(chan, buf, s->tx_dma_len,
>  					   DMA_MEM_TO_DEV,
>  					   DMA_PREP_INTERRUPT | DMA_CTRL_ACK);

Otherwise:

Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>

-- 
Best Regards,
Eugeniu.
[prev in list] [next in list] [prev in thread] [next in thread]