diff mbox

fsldma: fix performance degradation by optimizing spinlock use.

Message ID DBB740589CE8814680DECFE34BE197AB1665A3@039-SN1MPN1-006.039d.mgd.msft.net (mailing list archive)
State Not Applicable
Delegated to: Kumar Gala
Headers show

Commit Message

b29237@freescale.com Jan. 11, 2012, 7:54 a.m. UTC 
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?

Thanks,
Forrest

-----Original Message-----
From: Shi Xuelin-B29237 
Sent: 2011年12月26日 14:01
To: iws@ovro.caltech.edu; vinod.koul@intel.com; dan.j.williams@intel.com; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
Cc: Shi Xuelin-B29237
Subject: [PATCH] fsldma: fix performance degradation by optimizing spinlock use.

From: Forrest shi <b29237@freescale.com>

    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@freescale.com>
---
 drivers/dma/fsldma.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

Comments

Ira Snyder Jan. 11, 2012, 4:13 p.m. UTC | #1 
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@ovro.caltech.edu; vinod.koul@intel.com; dan.j.williams@intel.com; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Cc: Shi Xuelin-B29237
> Subject: [PATCH] fsldma: fix performance degradation by optimizing spinlock use.
> 
> From: Forrest shi <b29237@freescale.com>
> 
>     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@freescale.com>
> ---
>  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
> 
>
b29237@freescale.com April 26, 2012, 2:03 a.m. UTC | #2 
Hi Dan Williams,

Do you have any comment about this patch?

Thanks,
Forrest

-----Original Message-----
From: Ira W. Snyder [mailto:iws@ovro.caltech.edu] 

Sent: 2012年1月12日 0:14
To: Shi Xuelin-B29237
Cc: vinod.koul@intel.com; dan.j.williams@intel.com; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
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@ovro.caltech.edu; vinod.koul@intel.com; dan.j.williams@intel.com; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org

> Cc: Shi Xuelin-B29237

> Subject: [PATCH] fsldma: fix performance degradation by optimizing spinlock use.

> 

> From: Forrest shi <b29237@freescale.com>

> 

>     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@freescale.com>

> ---

>  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

> 

>
Li Yang-R58472 April 26, 2012, 2:59 a.m. UTC | #3 
Hi Dan and Vinod,

Please help to pick this patch up.  It addresses a great performance regression for RAID offloading.

Acked-by: Li Yang <leoli@freescale.com>


- Leo

> -----Original Message-----

> From: Shi Xuelin-B29237

> Sent: Thursday, April 26, 2012 10:04 AM

> To: dan.j.williams@intel.com

> Cc: vinod.koul@intel.com; linuxppc-dev@lists.ozlabs.org; linux-

> kernel@vger.kernel.org; Ira W. Snyder; Li Yang-R58472

> Subject: RE: [PATCH] fsldma: fix performance degradation by optimizing

> spinlock use.

> 

> Hi Dan Williams,

> 

> Do you have any comment about this patch?

> 

> Thanks,

> Forrest

> 

> -----Original Message-----

> From: Ira W. Snyder [mailto:iws@ovro.caltech.edu]

> Sent: 2012年1月12日 0:14

> To: Shi Xuelin-B29237

> Cc: vinod.koul@intel.com; dan.j.williams@intel.com; linuxppc-

> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org

> 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@ovro.caltech.edu; vinod.koul@intel.com;

> dan.j.williams@intel.com; linuxppc-dev@lists.ozlabs.org; linux-

> kernel@vger.kernel.org

> > Cc: Shi Xuelin-B29237

> > Subject: [PATCH] fsldma: fix performance degradation by optimizing

> spinlock use.

> >

> > From: Forrest shi <b29237@freescale.com>

> >

> >     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@freescale.com>

> > ---

> >  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

> >

> >
diff mbox

Patch

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