RE: [PATCH] fsldma: fix performance degradation by optimizingspinlock use.

[Shi Xuelin-B29237]

Hi Dan Williams,

Do you have any comment about this patch?

Thanks,
Forrest

-----Original Message-----
From: Ira W. Snyder [mailto:iws@xxxxxxxxxxxxxxxx] 
Sent: 2012å1æ12æ 0:14
To: Shi Xuelin-B29237
Cc: vinod.koul@xxxxxxxxx; dan.j.williams@xxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] fsldma: fix performance degradation by optimizing spinlock use.

On Wed, Jan 11, 2012 at 07:54:55AM +0000, Shi Xuelin-B29237 wrote:
> Hello Iris,
> 
> As we discussed in the previous patch, I add one smp_mb() in fsl_tx_status.
> In my testing with iozone, this smp_mb() could cause 1%~2% performance degradation.
> Anyway it is acceptable for me. Do you have any other comments?
> 

This patch looks fine to me.

Ira

> -----Original Message-----
> From: Shi Xuelin-B29237 
> Sent: 2011å12æ26æ 14:01
> To: iws@xxxxxxxxxxxxxxxx; vinod.koul@xxxxxxxxx; dan.j.williams@xxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: Shi Xuelin-B29237
> Subject: [PATCH] fsldma: fix performance degradation by optimizing spinlock use.
> 
> From: Forrest shi <b29237@xxxxxxxxxxxxx>
> 
>     dma status check function fsl_tx_status is heavily called in
>     a tight loop and the desc lock in fsl_tx_status contended by
>     the dma status update function. this caused the dma performance
>     degrades much.
> 
>     this patch releases the lock in the fsl_tx_status function, and
>     introduce the smp_mb() to avoid possible memory inconsistency.
> 
>     Signed-off-by: Forrest Shi <xuelin.shi@xxxxxxxxxxxxx>
> ---
>  drivers/dma/fsldma.c |    6 +-----
>  1 files changed, 1 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index 8a78154..008fb5e 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -986,15 +986,11 @@ static enum dma_status fsl_tx_status(struct dma_chan *dchan,
>  	struct fsldma_chan *chan = to_fsl_chan(dchan);
>  	dma_cookie_t last_complete;
>  	dma_cookie_t last_used;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&chan->desc_lock, flags);
>  
> 	last_complete = chan->completed_cookie;
> +	smp_mb();
> 	last_used = dchan->cookie;
>  
> -	spin_unlock_irqrestore(&chan->desc_lock, flags);
> -
>  	dma_set_tx_state(txstate, last_complete, last_used, 0);
>  	return dma_async_is_complete(cookie, last_complete, last_used); }
> --
> 1.7.0.4
> 
> 

¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_