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 <[email protected]>
---
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 */
I'm ok with enabling this for people that just want to use DMA and not
RAID.
Acked-by: Dave Jiang <[email protected]>
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 <[email protected]>
> ---
> 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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
>
> 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 <[email protected]>
> > ---
> > 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 */
> >
>
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 <[email protected]>
> >
> > 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 <[email protected]>
> > > ---
> > > 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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
---
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 */
That looks fine.
Acked-by: Dave Jiang <[email protected]>
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 <[email protected]>
> ---
> 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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Fri, 2013-08-02 at 16:57 +0000, Dan Williams wrote:
>
> On 8/2/13 12:34 AM, "Brice Goglin" <[email protected]> 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 <[email protected]>
> >---
> > 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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
---
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 */
On 8/2/13 12:34 AM, "Brice Goglin" <[email protected]> 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 <[email protected]>
>---
> 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
On 8/2/13 10:26 AM, "Brice Goglin" <[email protected]> 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.
On Fri, Aug 02, 2013 at 04:57:44PM +0000, Dan Williams wrote:
>
>
> On 8/2/13 12:34 AM, "Brice Goglin" <[email protected]> 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 <[email protected]>
> >---
> > 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
>
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 <[email protected]>
---
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];
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 <[email protected]>
> ---
> 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];
>
On 8/12/13 11:10 AM, "Jon Mason" <[email protected]> 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