public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* ioatdma: add ioat_raid_enabled module parameter
@ 2013-07-31 22:05 Brice Goglin
  2013-07-31 22:14 ` Jiang, Dave
  0 siblings, 1 reply; 14+ messages in thread
From: Brice Goglin @ 2013-07-31 22:05 UTC (permalink / raw)
  To: Dan Williams; +Cc: Vinod Koul, Dave Jiang, LKML

ioatdma: add ioat_raid_enabled module parameter

Commit f26df1a1 added a 64-byte alignment requirement for legacy
operations to work around a silicon errata when mixing legacy and
RAID descriptors.
Passing ioat_raid_enabled=0 now disables RAID offload entirely in
the ioatdma driver so that legacy operations (memcpy, etc.) can
work without alignment restrictions anymore.

Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
---
 drivers/dma/ioat/dma_v3.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Index: b/drivers/dma/ioat/dma_v3.c
===================================================================
--- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
+++ b/drivers/dma/ioat/dma_v3.c	2013-07-31 23:47:54.246719841 +0200
@@ -67,6 +67,11 @@
 #include "dma.h"
 #include "dma_v2.h"
 
+static int ioat_raid_enabled = 1;
+module_param(ioat_raid_enabled, int, 0444);
+MODULE_PARM_DESC(ioat_raid_enabled,
+		 "control support of RAID offload (default: 1)");
+
 /* ioat hardware assumes at least two sources for raid operations */
 #define src_cnt_to_sw(x) ((x) + 2)
 #define src_cnt_to_hw(x) ((x) - 2)
@@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
 	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
 	dma->device_free_chan_resources = ioat2_free_chan_resources;
 
-	if (is_xeon_cb32(pdev))
+	if (is_xeon_cb32(pdev) && ioat_raid_enabled)
 		dma->copy_align = 6;
 
 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
@@ -1783,7 +1788,7 @@ int ioat3_dma_probe(struct ioatdma_devic
 
 	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
 
-	if (is_bwd_noraid(pdev))
+	if (!ioat_raid_enabled || is_bwd_noraid(pdev))
 		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
 
 	/* dca is incompatible with raid operations */


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-07-31 22:05 ioatdma: add ioat_raid_enabled module parameter Brice Goglin
@ 2013-07-31 22:14 ` Jiang, Dave
  2013-08-01 17:11   ` Jon Mason
  0 siblings, 1 reply; 14+ messages in thread
From: Jiang, Dave @ 2013-07-31 22:14 UTC (permalink / raw)
  To: Brice Goglin; +Cc: Dan Williams, Koul, Vinod, LKML, Mason, Jon

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2239 bytes --]

I'm ok with enabling this for people that just want to use DMA and not
RAID. 

Acked-by: Dave Jiang <dave.jiang@intel.com>

On Thu, 2013-08-01 at 00:05 +0200, Brice Goglin wrote:
> ioatdma: add ioat_raid_enabled module parameter
> 
> Commit f26df1a1 added a 64-byte alignment requirement for legacy
> operations to work around a silicon errata when mixing legacy and
> RAID descriptors.
> Passing ioat_raid_enabled=0 now disables RAID offload entirely in
> the ioatdma driver so that legacy operations (memcpy, etc.) can
> work without alignment restrictions anymore.
> 
> Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
> ---
>  drivers/dma/ioat/dma_v3.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> Index: b/drivers/dma/ioat/dma_v3.c
> ===================================================================
> --- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
> +++ b/drivers/dma/ioat/dma_v3.c	2013-07-31 23:47:54.246719841 +0200
> @@ -67,6 +67,11 @@
>  #include "dma.h"
>  #include "dma_v2.h"
>  
> +static int ioat_raid_enabled = 1;
> +module_param(ioat_raid_enabled, int, 0444);
> +MODULE_PARM_DESC(ioat_raid_enabled,
> +		 "control support of RAID offload (default: 1)");
> +
>  /* ioat hardware assumes at least two sources for raid operations */
>  #define src_cnt_to_sw(x) ((x) + 2)
>  #define src_cnt_to_hw(x) ((x) - 2)
> @@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
>  	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
>  	dma->device_free_chan_resources = ioat2_free_chan_resources;
>  
> -	if (is_xeon_cb32(pdev))
> +	if (is_xeon_cb32(pdev) && ioat_raid_enabled)
>  		dma->copy_align = 6;
>  
>  	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
> @@ -1783,7 +1788,7 @@ int ioat3_dma_probe(struct ioatdma_devic
>  
>  	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
>  
> -	if (is_bwd_noraid(pdev))
> +	if (!ioat_raid_enabled || is_bwd_noraid(pdev))
>  		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
>  
>  	/* dca is incompatible with raid operations */
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-07-31 22:14 ` Jiang, Dave
@ 2013-08-01 17:11   ` Jon Mason
  2013-08-01 17:15     ` Jiang, Dave
  0 siblings, 1 reply; 14+ messages in thread
From: Jon Mason @ 2013-08-01 17:11 UTC (permalink / raw)
  To: Jiang, Dave; +Cc: Brice Goglin, Dan Williams, Koul, Vinod, LKML

On Wed, Jul 31, 2013 at 03:14:07PM -0700, Jiang, Dave wrote:
> I'm ok with enabling this for people that just want to use DMA and not
> RAID. 

I might be crazy, but I'd be in favor of disabling the RAID offload by
default on non-Atom platforms.

Thanks,
Jon

> 
> Acked-by: Dave Jiang <dave.jiang@intel.com>
> 
> On Thu, 2013-08-01 at 00:05 +0200, Brice Goglin wrote:
> > ioatdma: add ioat_raid_enabled module parameter
> > 
> > Commit f26df1a1 added a 64-byte alignment requirement for legacy
> > operations to work around a silicon errata when mixing legacy and
> > RAID descriptors.
> > Passing ioat_raid_enabled=0 now disables RAID offload entirely in
> > the ioatdma driver so that legacy operations (memcpy, etc.) can
> > work without alignment restrictions anymore.
> > 
> > Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
> > ---
> >  drivers/dma/ioat/dma_v3.c |    9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > Index: b/drivers/dma/ioat/dma_v3.c
> > ===================================================================
> > --- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
> > +++ b/drivers/dma/ioat/dma_v3.c	2013-07-31 23:47:54.246719841 +0200
> > @@ -67,6 +67,11 @@
> >  #include "dma.h"
> >  #include "dma_v2.h"
> >  
> > +static int ioat_raid_enabled = 1;
> > +module_param(ioat_raid_enabled, int, 0444);
> > +MODULE_PARM_DESC(ioat_raid_enabled,
> > +		 "control support of RAID offload (default: 1)");
> > +
> >  /* ioat hardware assumes at least two sources for raid operations */
> >  #define src_cnt_to_sw(x) ((x) + 2)
> >  #define src_cnt_to_hw(x) ((x) - 2)
> > @@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
> >  	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
> >  	dma->device_free_chan_resources = ioat2_free_chan_resources;
> >  
> > -	if (is_xeon_cb32(pdev))
> > +	if (is_xeon_cb32(pdev) && ioat_raid_enabled)
> >  		dma->copy_align = 6;
> >  
> >  	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
> > @@ -1783,7 +1788,7 @@ int ioat3_dma_probe(struct ioatdma_devic
> >  
> >  	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
> >  
> > -	if (is_bwd_noraid(pdev))
> > +	if (!ioat_raid_enabled || is_bwd_noraid(pdev))
> >  		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
> >  
> >  	/* dca is incompatible with raid operations */
> > 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-01 17:11   ` Jon Mason
@ 2013-08-01 17:15     ` Jiang, Dave
  2013-08-02  7:34       ` Brice Goglin
  0 siblings, 1 reply; 14+ messages in thread
From: Jiang, Dave @ 2013-08-01 17:15 UTC (permalink / raw)
  To: Mason, Jon; +Cc: Brice Goglin, Dan Williams, Koul, Vinod, LKML

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2890 bytes --]

On Thu, 2013-08-01 at 10:11 -0700, Jon Mason wrote:
> On Wed, Jul 31, 2013 at 03:14:07PM -0700, Jiang, Dave wrote:
> > I'm ok with enabling this for people that just want to use DMA and not
> > RAID. 
> 
> I might be crazy, but I'd be in favor of disabling the RAID offload by
> default on non-Atom platforms.
> 

I suppose. Technically it is disabled starting with 3.10 because of the
channel switch issue. I'm ok with this disabled by default for the 3.2
platforms that has broken pq-val. 

> Thanks,
> Jon
> 
> > 
> > Acked-by: Dave Jiang <dave.jiang@intel.com>
> > 
> > On Thu, 2013-08-01 at 00:05 +0200, Brice Goglin wrote:
> > > ioatdma: add ioat_raid_enabled module parameter
> > > 
> > > Commit f26df1a1 added a 64-byte alignment requirement for legacy
> > > operations to work around a silicon errata when mixing legacy and
> > > RAID descriptors.
> > > Passing ioat_raid_enabled=0 now disables RAID offload entirely in
> > > the ioatdma driver so that legacy operations (memcpy, etc.) can
> > > work without alignment restrictions anymore.
> > > 
> > > Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
> > > ---
> > >  drivers/dma/ioat/dma_v3.c |    9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > Index: b/drivers/dma/ioat/dma_v3.c
> > > ===================================================================
> > > --- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
> > > +++ b/drivers/dma/ioat/dma_v3.c	2013-07-31 23:47:54.246719841 +0200
> > > @@ -67,6 +67,11 @@
> > >  #include "dma.h"
> > >  #include "dma_v2.h"
> > >  
> > > +static int ioat_raid_enabled = 1;
> > > +module_param(ioat_raid_enabled, int, 0444);
> > > +MODULE_PARM_DESC(ioat_raid_enabled,
> > > +		 "control support of RAID offload (default: 1)");
> > > +
> > >  /* ioat hardware assumes at least two sources for raid operations */
> > >  #define src_cnt_to_sw(x) ((x) + 2)
> > >  #define src_cnt_to_hw(x) ((x) - 2)
> > > @@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
> > >  	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
> > >  	dma->device_free_chan_resources = ioat2_free_chan_resources;
> > >  
> > > -	if (is_xeon_cb32(pdev))
> > > +	if (is_xeon_cb32(pdev) && ioat_raid_enabled)
> > >  		dma->copy_align = 6;
> > >  
> > >  	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
> > > @@ -1783,7 +1788,7 @@ int ioat3_dma_probe(struct ioatdma_devic
> > >  
> > >  	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
> > >  
> > > -	if (is_bwd_noraid(pdev))
> > > +	if (!ioat_raid_enabled || is_bwd_noraid(pdev))
> > >  		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
> > >  
> > >  	/* dca is incompatible with raid operations */
> > > 
> > 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-01 17:15     ` Jiang, Dave
@ 2013-08-02  7:34       ` Brice Goglin
  2013-08-02 16:14         ` Jiang, Dave
  2013-08-02 16:57         ` Dan Williams
  0 siblings, 2 replies; 14+ messages in thread
From: Brice Goglin @ 2013-08-02  7:34 UTC (permalink / raw)
  To: Jiang, Dave; +Cc: Mason, Jon, Dan Williams, Koul, Vinod, LKML

Le 01/08/2013 19:15, Jiang, Dave a écrit :
> On Thu, 2013-08-01 at 10:11 -0700, Jon Mason wrote:
>> On Wed, Jul 31, 2013 at 03:14:07PM -0700, Jiang, Dave wrote:
>>> I'm ok with enabling this for people that just want to use DMA and not
>>> RAID. 
>> I might be crazy, but I'd be in favor of disabling the RAID offload by
>> default on non-Atom platforms.
>>
> I suppose. Technically it is disabled starting with 3.10 because of the
> channel switch issue. I'm ok with this disabled by default for the 3.2
> platforms that has broken pq-val. 
>

Here's a patch that may do what you guys are saying.

Brice



ioatdma: disable RAID by default when buggy and add module param

Commit f26df1a1 added a 64-byte alignment requirement for legacy
operations to work around a silicon errata when mixing legacy and
RAID descriptors.

RAID offload is now disabled by default on buggy 3.2 platforms.
Passing ioat_raid_enabled=1 force-enables it on all platforms
(previous behavior).
Passing ioat_raid_enabled=0 force-disables it everywhere.

When RAID offload is disabled, legacy operations (memcpy, etc.)
can work again without alignment restrictions.

Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
---
 drivers/dma/ioat/dma_v3.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Index: b/drivers/dma/ioat/dma_v3.c
===================================================================
--- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
+++ b/drivers/dma/ioat/dma_v3.c	2013-08-02 09:28:51.817037742 +0200
@@ -67,6 +67,11 @@
 #include "dma.h"
 #include "dma_v2.h"
 
+static int ioat_raid_enabled = -1;
+module_param(ioat_raid_enabled, int, 0444);
+MODULE_PARM_DESC(ioat_raid_enabled,
+		 "control support of RAID offload (-1=enabled unless broken [default], 0=disabled, 1=enabled)");
+
 /* ioat hardware assumes at least two sources for raid operations */
 #define src_cnt_to_sw(x) ((x) + 2)
 #define src_cnt_to_hw(x) ((x) - 2)
@@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
 	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
 	dma->device_free_chan_resources = ioat2_free_chan_resources;
 
-	if (is_xeon_cb32(pdev))
+	if (ioat_raid_enabled == 1 && is_xeon_cb32(pdev))
 		dma->copy_align = 6;
 
 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
@@ -1783,7 +1788,14 @@ int ioat3_dma_probe(struct ioatdma_devic
 
 	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
 
-	if (is_bwd_noraid(pdev))
+	/* disable RAID if:
+	 *    force-disabled by module param,
+	 * or not force-enabled on buggy 3.2 platforms,
+	 * or not actually supported.
+	 */
+	if (ioat_raid_enabled == 0
+	    || (ioat_raid_enabled != 1 && is_xeon_cb32(pdev))
+	    || is_bwd_noraid(pdev))
 		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
 
 	/* dca is incompatible with raid operations */



^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-02  7:34       ` Brice Goglin
@ 2013-08-02 16:14         ` Jiang, Dave
  2013-08-02 16:57         ` Dan Williams
  1 sibling, 0 replies; 14+ messages in thread
From: Jiang, Dave @ 2013-08-02 16:14 UTC (permalink / raw)
  To: Brice Goglin; +Cc: Mason, Jon, Dan Williams, Koul, Vinod, LKML

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3290 bytes --]

That looks fine. 
Acked-by: Dave Jiang <dave.jiang@intel.com>

On Fri, 2013-08-02 at 09:34 +0200, Brice Goglin wrote:
> Le 01/08/2013 19:15, Jiang, Dave a écrit :
> > On Thu, 2013-08-01 at 10:11 -0700, Jon Mason wrote:
> >> On Wed, Jul 31, 2013 at 03:14:07PM -0700, Jiang, Dave wrote:
> >>> I'm ok with enabling this for people that just want to use DMA and not
> >>> RAID. 
> >> I might be crazy, but I'd be in favor of disabling the RAID offload by
> >> default on non-Atom platforms.
> >>
> > I suppose. Technically it is disabled starting with 3.10 because of the
> > channel switch issue. I'm ok with this disabled by default for the 3.2
> > platforms that has broken pq-val. 
> >
> 
> Here's a patch that may do what you guys are saying.
> 
> Brice
> 
> 
> 
> ioatdma: disable RAID by default when buggy and add module param
> 
> Commit f26df1a1 added a 64-byte alignment requirement for legacy
> operations to work around a silicon errata when mixing legacy and
> RAID descriptors.
> 
> RAID offload is now disabled by default on buggy 3.2 platforms.
> Passing ioat_raid_enabled=1 force-enables it on all platforms
> (previous behavior).
> Passing ioat_raid_enabled=0 force-disables it everywhere.
> 
> When RAID offload is disabled, legacy operations (memcpy, etc.)
> can work again without alignment restrictions.
> 
> Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
> ---
>  drivers/dma/ioat/dma_v3.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> Index: b/drivers/dma/ioat/dma_v3.c
> ===================================================================
> --- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
> +++ b/drivers/dma/ioat/dma_v3.c	2013-08-02 09:28:51.817037742 +0200
> @@ -67,6 +67,11 @@
>  #include "dma.h"
>  #include "dma_v2.h"
>  
> +static int ioat_raid_enabled = -1;
> +module_param(ioat_raid_enabled, int, 0444);
> +MODULE_PARM_DESC(ioat_raid_enabled,
> +		 "control support of RAID offload (-1=enabled unless broken [default], 0=disabled, 1=enabled)");
> +
>  /* ioat hardware assumes at least two sources for raid operations */
>  #define src_cnt_to_sw(x) ((x) + 2)
>  #define src_cnt_to_hw(x) ((x) - 2)
> @@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
>  	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
>  	dma->device_free_chan_resources = ioat2_free_chan_resources;
>  
> -	if (is_xeon_cb32(pdev))
> +	if (ioat_raid_enabled == 1 && is_xeon_cb32(pdev))
>  		dma->copy_align = 6;
>  
>  	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
> @@ -1783,7 +1788,14 @@ int ioat3_dma_probe(struct ioatdma_devic
>  
>  	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
>  
> -	if (is_bwd_noraid(pdev))
> +	/* disable RAID if:
> +	 *    force-disabled by module param,
> +	 * or not force-enabled on buggy 3.2 platforms,
> +	 * or not actually supported.
> +	 */
> +	if (ioat_raid_enabled == 0
> +	    || (ioat_raid_enabled != 1 && is_xeon_cb32(pdev))
> +	    || is_bwd_noraid(pdev))
>  		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
>  
>  	/* dca is incompatible with raid operations */
> 
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-02  7:34       ` Brice Goglin
  2013-08-02 16:14         ` Jiang, Dave
@ 2013-08-02 16:57         ` Dan Williams
  2013-08-02 17:08           ` Jiang, Dave
                             ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Dan Williams @ 2013-08-02 16:57 UTC (permalink / raw)
  To: Brice Goglin, Jiang, Dave; +Cc: Mason, Jon, Koul, Vinod, LKML



On 8/2/13 12:34 AM, "Brice Goglin" <Brice.Goglin@inria.fr> wrote:

>Le 01/08/2013 19:15, Jiang, Dave a écrit :
>> On Thu, 2013-08-01 at 10:11 -0700, Jon Mason wrote:
>>> On Wed, Jul 31, 2013 at 03:14:07PM -0700, Jiang, Dave wrote:
>>>> I'm ok with enabling this for people that just want to use DMA and not
>>>> RAID. 
>>> I might be crazy, but I'd be in favor of disabling the RAID offload by
>>> default on non-Atom platforms.
>>>
>> I suppose. Technically it is disabled starting with 3.10 because of the
>> channel switch issue. I'm ok with this disabled by default for the 3.2
>> platforms that has broken pq-val.
>>
>
>Here's a patch that may do what you guys are saying.
>
>Brice
>
>
>
>ioatdma: disable RAID by default when buggy and add module param
>
>Commit f26df1a1 added a 64-byte alignment requirement for legacy
>operations to work around a silicon errata when mixing legacy and
>RAID descriptors.
>
>RAID offload is now disabled by default on buggy 3.2 platforms.
>Passing ioat_raid_enabled=1 force-enables it on all platforms
>(previous behavior).
>Passing ioat_raid_enabled=0 force-disables it everywhere.
>
>When RAID offload is disabled, legacy operations (memcpy, etc.)
>can work again without alignment restrictions.
>
>Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
>---
> drivers/dma/ioat/dma_v3.c |   16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
>Index: b/drivers/dma/ioat/dma_v3.c
>===================================================================
>--- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
>+++ b/drivers/dma/ioat/dma_v3.c	2013-08-02 09:28:51.817037742 +0200
>@@ -67,6 +67,11 @@
> #include "dma.h"
> #include "dma_v2.h"
> 
>+static int ioat_raid_enabled = -1;
>+module_param(ioat_raid_enabled, int, 0444);
>+MODULE_PARM_DESC(ioat_raid_enabled,
>+		 "control support of RAID offload (-1=enabled unless broken [default],
>0=disabled, 1=enabled)");
>+
> /* ioat hardware assumes at least two sources for raid operations */
> #define src_cnt_to_sw(x) ((x) + 2)
> #define src_cnt_to_hw(x) ((x) - 2)
>@@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
> 	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
> 	dma->device_free_chan_resources = ioat2_free_chan_resources;
> 
>-	if (is_xeon_cb32(pdev))
>+	if (ioat_raid_enabled == 1 && is_xeon_cb32(pdev))
> 		dma->copy_align = 6;

Actually we can delete this now because is_xeon_cb32() already effectively
means that raid offload will not be used.

> 
> 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
>@@ -1783,7 +1788,14 @@ int ioat3_dma_probe(struct ioatdma_devic
> 
> 	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
> 
>-	if (is_bwd_noraid(pdev))
>+	/* disable RAID if:
>+	 *    force-disabled by module param,
>+	 * or not force-enabled on buggy 3.2 platforms,
>+	 * or not actually supported.
>+	 */
>+	if (ioat_raid_enabled == 0
>+	    || (ioat_raid_enabled != 1 && is_xeon_cb32(pdev))
>+	    || is_bwd_noraid(pdev))
> 		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
> 
> 	/* dca is incompatible with raid operations */
>

I like Jon¹s suggestion.  Just make raid disabled by default on non-atom
platforms.  When if a non-atom platform comes along without the previous
restrictions it can add itself to this list.

So let¹s drop the module parameter and just cleanup the 3.2 support to
reflect the current reality of raid being disabled.

--
Dan


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-02 16:57         ` Dan Williams
@ 2013-08-02 17:08           ` Jiang, Dave
  2013-08-02 17:26           ` Brice Goglin
  2013-08-02 18:01           ` Jon Mason
  2 siblings, 0 replies; 14+ messages in thread
From: Jiang, Dave @ 2013-08-02 17:08 UTC (permalink / raw)
  To: Dan Williams; +Cc: Brice Goglin, Mason, Jon, Koul, Vinod, LKML

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3939 bytes --]

On Fri, 2013-08-02 at 16:57 +0000, Dan Williams wrote:
> 
> On 8/2/13 12:34 AM, "Brice Goglin" <Brice.Goglin@inria.fr> wrote:
> 
> >Le 01/08/2013 19:15, Jiang, Dave a écrit :
> >> On Thu, 2013-08-01 at 10:11 -0700, Jon Mason wrote:
> >>> On Wed, Jul 31, 2013 at 03:14:07PM -0700, Jiang, Dave wrote:
> >>>> I'm ok with enabling this for people that just want to use DMA and not
> >>>> RAID. 
> >>> I might be crazy, but I'd be in favor of disabling the RAID offload by
> >>> default on non-Atom platforms.
> >>>
> >> I suppose. Technically it is disabled starting with 3.10 because of the
> >> channel switch issue. I'm ok with this disabled by default for the 3.2
> >> platforms that has broken pq-val.
> >>
> >
> >Here's a patch that may do what you guys are saying.
> >
> >Brice
> >
> >
> >
> >ioatdma: disable RAID by default when buggy and add module param
> >
> >Commit f26df1a1 added a 64-byte alignment requirement for legacy
> >operations to work around a silicon errata when mixing legacy and
> >RAID descriptors.
> >
> >RAID offload is now disabled by default on buggy 3.2 platforms.
> >Passing ioat_raid_enabled=1 force-enables it on all platforms
> >(previous behavior).
> >Passing ioat_raid_enabled=0 force-disables it everywhere.
> >
> >When RAID offload is disabled, legacy operations (memcpy, etc.)
> >can work again without alignment restrictions.
> >
> >Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
> >---
> > drivers/dma/ioat/dma_v3.c |   16 ++++++++++++++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> >Index: b/drivers/dma/ioat/dma_v3.c
> >===================================================================
> >--- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
> >+++ b/drivers/dma/ioat/dma_v3.c	2013-08-02 09:28:51.817037742 +0200
> >@@ -67,6 +67,11 @@
> > #include "dma.h"
> > #include "dma_v2.h"
> > 
> >+static int ioat_raid_enabled = -1;
> >+module_param(ioat_raid_enabled, int, 0444);
> >+MODULE_PARM_DESC(ioat_raid_enabled,
> >+		 "control support of RAID offload (-1=enabled unless broken [default],
> >0=disabled, 1=enabled)");
> >+
> > /* ioat hardware assumes at least two sources for raid operations */
> > #define src_cnt_to_sw(x) ((x) + 2)
> > #define src_cnt_to_hw(x) ((x) - 2)
> >@@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
> > 	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
> > 	dma->device_free_chan_resources = ioat2_free_chan_resources;
> > 
> >-	if (is_xeon_cb32(pdev))
> >+	if (ioat_raid_enabled == 1 && is_xeon_cb32(pdev))
> > 		dma->copy_align = 6;
> 
> Actually we can delete this now because is_xeon_cb32() already effectively
> means that raid offload will not be used.
> 
> > 
> > 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
> >@@ -1783,7 +1788,14 @@ int ioat3_dma_probe(struct ioatdma_devic
> > 
> > 	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
> > 
> >-	if (is_bwd_noraid(pdev))
> >+	/* disable RAID if:
> >+	 *    force-disabled by module param,
> >+	 * or not force-enabled on buggy 3.2 platforms,
> >+	 * or not actually supported.
> >+	 */
> >+	if (ioat_raid_enabled == 0
> >+	    || (ioat_raid_enabled != 1 && is_xeon_cb32(pdev))
> >+	    || is_bwd_noraid(pdev))
> > 		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
> > 
> > 	/* dca is incompatible with raid operations */
> >
> 
> I like Jon¹s suggestion.  Just make raid disabled by default on non-atom
> platforms.  When if a non-atom platform comes along without the previous
> restrictions it can add itself to this list.
> 
> So let¹s drop the module parameter and just cleanup the 3.2 support to
> reflect the current reality of raid being disabled.

Works for me. Then we won't have to worry about the channel switch mess
on 3.2. 

> 
> --
> Dan
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-02 16:57         ` Dan Williams
  2013-08-02 17:08           ` Jiang, Dave
@ 2013-08-02 17:26           ` Brice Goglin
  2013-08-02 17:47             ` Dan Williams
  2013-08-02 18:01           ` Jon Mason
  2 siblings, 1 reply; 14+ messages in thread
From: Brice Goglin @ 2013-08-02 17:26 UTC (permalink / raw)
  To: Dan Williams; +Cc: Jiang, Dave, Mason, Jon, Koul, Vinod, LKML

Le 02/08/2013 18:57, Dan Williams a écrit :
> I like Jon¹s suggestion.  Just make raid disabled by default on non-atom
> platforms.  When if a non-atom platform comes along without the previous
> restrictions it can add itself to this list.
>
> So let¹s drop the module parameter and just cleanup the 3.2 support to
> reflect the current reality of raid being disabled.

So you want this instead ?

Brice


ioatdma: disable RAID on non-Atom platforms and reenable unaligned copies

Disable RAID on non-Atom platform and remove the 64-byte alignement
restriction on legacy DMA operations (introduced in commit f26df1a1
as a workaround for silicon errata).

Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
---
 drivers/dma/ioat/dma_v3.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Index: b/drivers/dma/ioat/dma_v3.c
===================================================================
--- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
+++ b/drivers/dma/ioat/dma_v3.c	2013-08-02 19:17:26.075745688 +0200
@@ -1775,15 +1775,12 @@ int ioat3_dma_probe(struct ioatdma_devic
 	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
 	dma->device_free_chan_resources = ioat2_free_chan_resources;
 
-	if (is_xeon_cb32(pdev))
-		dma->copy_align = 6;
-
 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
 	dma->device_prep_dma_interrupt = ioat3_prep_interrupt_lock;
 
 	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
 
-	if (is_bwd_noraid(pdev))
+	if (is_xeon_cb32(pdev) || is_bwd_noraid(pdev))
 		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
 
 	/* dca is incompatible with raid operations */


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-02 17:26           ` Brice Goglin
@ 2013-08-02 17:47             ` Dan Williams
  2013-08-02 19:18               ` Brice Goglin
  0 siblings, 1 reply; 14+ messages in thread
From: Dan Williams @ 2013-08-02 17:47 UTC (permalink / raw)
  To: Brice Goglin; +Cc: Jiang, Dave, Mason, Jon, Koul, Vinod, LKML



On 8/2/13 10:26 AM, "Brice Goglin" <Brice.Goglin@inria.fr> wrote:

>Le 02/08/2013 18:57, Dan Williams a écrit :
>> I like Jon¹s suggestion.  Just make raid disabled by default on non-atom
>> platforms.  When if a non-atom platform comes along without the previous
>> restrictions it can add itself to this list.
>>
>> So let¹s drop the module parameter and just cleanup the 3.2 support to
>> reflect the current reality of raid being disabled.
>
>So you want this instead ?

Yup, but should also fold in the deletions of the other is_xeon_cb32()
alignment fixups further below.

Actually all the alignment settings can be removed now.

...and the PQ_VAL/XOR_VAL fixup for is_xeon_cb32() can go.



^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-02 16:57         ` Dan Williams
  2013-08-02 17:08           ` Jiang, Dave
  2013-08-02 17:26           ` Brice Goglin
@ 2013-08-02 18:01           ` Jon Mason
  2 siblings, 0 replies; 14+ messages in thread
From: Jon Mason @ 2013-08-02 18:01 UTC (permalink / raw)
  To: Dan Williams; +Cc: Brice Goglin, Jiang, Dave, Koul, Vinod, LKML

On Fri, Aug 02, 2013 at 04:57:44PM +0000, Dan Williams wrote:
> 
> 
> On 8/2/13 12:34 AM, "Brice Goglin" <Brice.Goglin@inria.fr> wrote:
> 
> >Le 01/08/2013 19:15, Jiang, Dave a écrit :
> >> On Thu, 2013-08-01 at 10:11 -0700, Jon Mason wrote:
> >>> On Wed, Jul 31, 2013 at 03:14:07PM -0700, Jiang, Dave wrote:
> >>>> I'm ok with enabling this for people that just want to use DMA and not
> >>>> RAID. 
> >>> I might be crazy, but I'd be in favor of disabling the RAID offload by
> >>> default on non-Atom platforms.
> >>>
> >> I suppose. Technically it is disabled starting with 3.10 because of the
> >> channel switch issue. I'm ok with this disabled by default for the 3.2
> >> platforms that has broken pq-val.
> >>
> >
> >Here's a patch that may do what you guys are saying.
> >
> >Brice
> >
> >
> >
> >ioatdma: disable RAID by default when buggy and add module param
> >
> >Commit f26df1a1 added a 64-byte alignment requirement for legacy
> >operations to work around a silicon errata when mixing legacy and
> >RAID descriptors.
> >
> >RAID offload is now disabled by default on buggy 3.2 platforms.
> >Passing ioat_raid_enabled=1 force-enables it on all platforms
> >(previous behavior).
> >Passing ioat_raid_enabled=0 force-disables it everywhere.
> >
> >When RAID offload is disabled, legacy operations (memcpy, etc.)
> >can work again without alignment restrictions.
> >
> >Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
> >---
> > drivers/dma/ioat/dma_v3.c |   16 ++++++++++++++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> >Index: b/drivers/dma/ioat/dma_v3.c
> >===================================================================
> >--- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
> >+++ b/drivers/dma/ioat/dma_v3.c	2013-08-02 09:28:51.817037742 +0200
> >@@ -67,6 +67,11 @@
> > #include "dma.h"
> > #include "dma_v2.h"
> > 
> >+static int ioat_raid_enabled = -1;
> >+module_param(ioat_raid_enabled, int, 0444);
> >+MODULE_PARM_DESC(ioat_raid_enabled,
> >+		 "control support of RAID offload (-1=enabled unless broken [default],
> >0=disabled, 1=enabled)");
> >+
> > /* ioat hardware assumes at least two sources for raid operations */
> > #define src_cnt_to_sw(x) ((x) + 2)
> > #define src_cnt_to_hw(x) ((x) - 2)
> >@@ -1775,7 +1780,7 @@ int ioat3_dma_probe(struct ioatdma_devic
> > 	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
> > 	dma->device_free_chan_resources = ioat2_free_chan_resources;
> > 
> >-	if (is_xeon_cb32(pdev))
> >+	if (ioat_raid_enabled == 1 && is_xeon_cb32(pdev))
> > 		dma->copy_align = 6;
> 
> Actually we can delete this now because is_xeon_cb32() already effectively
> means that raid offload will not be used.
> 
> > 
> > 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
> >@@ -1783,7 +1788,14 @@ int ioat3_dma_probe(struct ioatdma_devic
> > 
> > 	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
> > 
> >-	if (is_bwd_noraid(pdev))
> >+	/* disable RAID if:
> >+	 *    force-disabled by module param,
> >+	 * or not force-enabled on buggy 3.2 platforms,
> >+	 * or not actually supported.
> >+	 */
> >+	if (ioat_raid_enabled == 0
> >+	    || (ioat_raid_enabled != 1 && is_xeon_cb32(pdev))
> >+	    || is_bwd_noraid(pdev))
> > 		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
> > 
> > 	/* dca is incompatible with raid operations */
> >
> 
> I like Jon¹s suggestion.  Just make raid disabled by default on non-atom
> platforms.  When if a non-atom platform comes along without the previous
> restrictions it can add itself to this list.

Awesome!  This greatly increases NTB performance when using DMA
engines.

Thanks,
Jon

> 
> So let¹s drop the module parameter and just cleanup the 3.2 support to
> reflect the current reality of raid being disabled.
> 
> --
> Dan
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-02 17:47             ` Dan Williams
@ 2013-08-02 19:18               ` Brice Goglin
  2013-08-12 18:10                 ` Jon Mason
  0 siblings, 1 reply; 14+ messages in thread
From: Brice Goglin @ 2013-08-02 19:18 UTC (permalink / raw)
  To: Dan Williams; +Cc: Jiang, Dave, Mason, Jon, Koul, Vinod, LKML

Le 02/08/2013 19:47, Dan Williams a écrit :
> Yup, but should also fold in the deletions of the other is_xeon_cb32()
> alignment fixups further below.
>
> Actually all the alignment settings can be removed now.
>
> ...and the PQ_VAL/XOR_VAL fixup for is_xeon_cb32() can go.

Ok, here's another one, but we're close to the limit of my understanding
of this driver's internals.

Removed all alignment fixups and all is_xeon_cb32() fixups.

Brice



ioatdma: disable RAID on non-Atom platforms and reenable unaligned copies

Disable RAID on non-Atom platform and remove related fixups such as the
64-byte alignement restriction on legacy DMA operations (introduced in
commit f26df1a1 as a workaround for silicon errata).

Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
---
 drivers/dma/ioat/dma_v3.c |   24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

Index: b/drivers/dma/ioat/dma_v3.c
===================================================================
--- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
+++ b/drivers/dma/ioat/dma_v3.c	2013-08-02 21:10:36.560044703 +0200
@@ -1775,15 +1775,12 @@ int ioat3_dma_probe(struct ioatdma_devic
 	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
 	dma->device_free_chan_resources = ioat2_free_chan_resources;
 
-	if (is_xeon_cb32(pdev))
-		dma->copy_align = 6;
-
 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
 	dma->device_prep_dma_interrupt = ioat3_prep_interrupt_lock;
 
 	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
 
-	if (is_bwd_noraid(pdev))
+	if (is_xeon_cb32(pdev) || is_bwd_noraid(pdev))
 		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
 
 	/* dca is incompatible with raid operations */
@@ -1793,7 +1790,6 @@ int ioat3_dma_probe(struct ioatdma_devic
 	if (device->cap & IOAT_CAP_XOR) {
 		is_raid_device = true;
 		dma->max_xor = 8;
-		dma->xor_align = 6;
 
 		dma_cap_set(DMA_XOR, dma->cap_mask);
 		dma->device_prep_dma_xor = ioat3_prep_xor;
@@ -1812,13 +1808,8 @@ int ioat3_dma_probe(struct ioatdma_devic
 
 		if (device->cap & IOAT_CAP_RAID16SS) {
 			dma_set_maxpq(dma, 16, 0);
-			dma->pq_align = 0;
 		} else {
 			dma_set_maxpq(dma, 8, 0);
-			if (is_xeon_cb32(pdev))
-				dma->pq_align = 6;
-			else
-				dma->pq_align = 0;
 		}
 
 		if (!(device->cap & IOAT_CAP_XOR)) {
@@ -1829,13 +1820,8 @@ int ioat3_dma_probe(struct ioatdma_devic
 
 			if (device->cap & IOAT_CAP_RAID16SS) {
 				dma->max_xor = 16;
-				dma->xor_align = 0;
 			} else {
 				dma->max_xor = 8;
-				if (is_xeon_cb32(pdev))
-					dma->xor_align = 6;
-				else
-					dma->xor_align = 0;
 			}
 		}
 	}
@@ -1844,14 +1830,6 @@ int ioat3_dma_probe(struct ioatdma_devic
 	device->cleanup_fn = ioat3_cleanup_event;
 	device->timer_fn = ioat3_timer_event;
 
-	if (is_xeon_cb32(pdev)) {
-		dma_cap_clear(DMA_XOR_VAL, dma->cap_mask);
-		dma->device_prep_dma_xor_val = NULL;
-
-		dma_cap_clear(DMA_PQ_VAL, dma->cap_mask);
-		dma->device_prep_dma_pq_val = NULL;
-	}
-
 	/* starting with CB3.3 super extended descriptors are supported */
 	if (device->cap & IOAT_CAP_RAID16SS) {
 		char pool_name[14];


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-02 19:18               ` Brice Goglin
@ 2013-08-12 18:10                 ` Jon Mason
  2013-08-12 18:13                   ` Dan Williams
  0 siblings, 1 reply; 14+ messages in thread
From: Jon Mason @ 2013-08-12 18:10 UTC (permalink / raw)
  To: Dan Williams, Koul, Vinod; +Cc: Jiang, Dave, Brice Goglin, LKML

On Fri, Aug 02, 2013 at 09:18:03PM +0200, Brice Goglin wrote:
> Le 02/08/2013 19:47, Dan Williams a écrit :
> > Yup, but should also fold in the deletions of the other is_xeon_cb32()
> > alignment fixups further below.
> >
> > Actually all the alignment settings can be removed now.
> >
> > ...and the PQ_VAL/XOR_VAL fixup for is_xeon_cb32() can go.
> 
> Ok, here's another one, but we're close to the limit of my understanding
> of this driver's internals.
> 
> Removed all alignment fixups and all is_xeon_cb32() fixups.
> 
> Brice

Dan/Vinod, I would really like for this to get into 3.12.  It
dovetails very nicely with my patch to use DMA engines in NTB, which I
am aiming for 3.12 as well (though still waiting for some review,
hint-hint).  Is this doable?

Thanks,
Jon

> 
> 
> 
> ioatdma: disable RAID on non-Atom platforms and reenable unaligned copies
> 
> Disable RAID on non-Atom platform and remove related fixups such as the
> 64-byte alignement restriction on legacy DMA operations (introduced in
> commit f26df1a1 as a workaround for silicon errata).
> 
> Signed-off-by: Brice Goglin <Brice.Goglin@inria.fr>
> ---
>  drivers/dma/ioat/dma_v3.c |   24 +-----------------------
>  1 file changed, 1 insertion(+), 23 deletions(-)
> 
> Index: b/drivers/dma/ioat/dma_v3.c
> ===================================================================
> --- a/drivers/dma/ioat/dma_v3.c	2013-07-31 23:06:24.163810000 +0200
> +++ b/drivers/dma/ioat/dma_v3.c	2013-08-02 21:10:36.560044703 +0200
> @@ -1775,15 +1775,12 @@ int ioat3_dma_probe(struct ioatdma_devic
>  	dma->device_alloc_chan_resources = ioat2_alloc_chan_resources;
>  	dma->device_free_chan_resources = ioat2_free_chan_resources;
>  
> -	if (is_xeon_cb32(pdev))
> -		dma->copy_align = 6;
> -
>  	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
>  	dma->device_prep_dma_interrupt = ioat3_prep_interrupt_lock;
>  
>  	device->cap = readl(device->reg_base + IOAT_DMA_CAP_OFFSET);
>  
> -	if (is_bwd_noraid(pdev))
> +	if (is_xeon_cb32(pdev) || is_bwd_noraid(pdev))
>  		device->cap &= ~(IOAT_CAP_XOR | IOAT_CAP_PQ | IOAT_CAP_RAID16SS);
>  
>  	/* dca is incompatible with raid operations */
> @@ -1793,7 +1790,6 @@ int ioat3_dma_probe(struct ioatdma_devic
>  	if (device->cap & IOAT_CAP_XOR) {
>  		is_raid_device = true;
>  		dma->max_xor = 8;
> -		dma->xor_align = 6;
>  
>  		dma_cap_set(DMA_XOR, dma->cap_mask);
>  		dma->device_prep_dma_xor = ioat3_prep_xor;
> @@ -1812,13 +1808,8 @@ int ioat3_dma_probe(struct ioatdma_devic
>  
>  		if (device->cap & IOAT_CAP_RAID16SS) {
>  			dma_set_maxpq(dma, 16, 0);
> -			dma->pq_align = 0;
>  		} else {
>  			dma_set_maxpq(dma, 8, 0);
> -			if (is_xeon_cb32(pdev))
> -				dma->pq_align = 6;
> -			else
> -				dma->pq_align = 0;
>  		}
>  
>  		if (!(device->cap & IOAT_CAP_XOR)) {
> @@ -1829,13 +1820,8 @@ int ioat3_dma_probe(struct ioatdma_devic
>  
>  			if (device->cap & IOAT_CAP_RAID16SS) {
>  				dma->max_xor = 16;
> -				dma->xor_align = 0;
>  			} else {
>  				dma->max_xor = 8;
> -				if (is_xeon_cb32(pdev))
> -					dma->xor_align = 6;
> -				else
> -					dma->xor_align = 0;
>  			}
>  		}
>  	}
> @@ -1844,14 +1830,6 @@ int ioat3_dma_probe(struct ioatdma_devic
>  	device->cleanup_fn = ioat3_cleanup_event;
>  	device->timer_fn = ioat3_timer_event;
>  
> -	if (is_xeon_cb32(pdev)) {
> -		dma_cap_clear(DMA_XOR_VAL, dma->cap_mask);
> -		dma->device_prep_dma_xor_val = NULL;
> -
> -		dma_cap_clear(DMA_PQ_VAL, dma->cap_mask);
> -		dma->device_prep_dma_pq_val = NULL;
> -	}
> -
>  	/* starting with CB3.3 super extended descriptors are supported */
>  	if (device->cap & IOAT_CAP_RAID16SS) {
>  		char pool_name[14];
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: ioatdma: add ioat_raid_enabled module parameter
  2013-08-12 18:10                 ` Jon Mason
@ 2013-08-12 18:13                   ` Dan Williams
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Williams @ 2013-08-12 18:13 UTC (permalink / raw)
  To: Jon Mason, Koul, Vinod; +Cc: Jiang, Dave, Brice Goglin, LKML



On 8/12/13 11:10 AM, "Jon Mason" <jon.mason@intel.com> wrote:

>On Fri, Aug 02, 2013 at 09:18:03PM +0200, Brice Goglin wrote:
>> Le 02/08/2013 19:47, Dan Williams a écrit :
>> > Yup, but should also fold in the deletions of the other is_xeon_cb32()
>> > alignment fixups further below.
>> >
>> > Actually all the alignment settings can be removed now.
>> >
>> > ...and the PQ_VAL/XOR_VAL fixup for is_xeon_cb32() can go.
>> 
>> Ok, here's another one, but we're close to the limit of my understanding
>> of this driver's internals.
>> 
>> Removed all alignment fixups and all is_xeon_cb32() fixups.
>> 
>> Brice
>
>Dan/Vinod, I would really like for this to get into 3.12.  It
>dovetails very nicely with my patch to use DMA engines in NTB, which I
>am aiming for 3.12 as well (though still waiting for some review,
>hint-hint).  Is this doable?

Yes, look for an update in my tree by this weekend.

--
Dan


^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-08-12 18:14 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-31 22:05 ioatdma: add ioat_raid_enabled module parameter Brice Goglin
2013-07-31 22:14 ` Jiang, Dave
2013-08-01 17:11   ` Jon Mason
2013-08-01 17:15     ` Jiang, Dave
2013-08-02  7:34       ` Brice Goglin
2013-08-02 16:14         ` Jiang, Dave
2013-08-02 16:57         ` Dan Williams
2013-08-02 17:08           ` Jiang, Dave
2013-08-02 17:26           ` Brice Goglin
2013-08-02 17:47             ` Dan Williams
2013-08-02 19:18               ` Brice Goglin
2013-08-12 18:10                 ` Jon Mason
2013-08-12 18:13                   ` Dan Williams
2013-08-02 18:01           ` Jon Mason

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox