DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] dma/idxd: fix burst capacity calculation
@ 2021-12-20 17:05 Bruce Richardson
  2022-01-04 17:16 ` Kevin Laatz
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Bruce Richardson @ 2021-12-20 17:05 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, kevin.laatz, stable

When the maximum burst size supported by HW is less than the available
ring space, incorrect capacity was returned when there was already some
jobs queued up for submission. This was because the capacity calculation
failed to subtract the number of already-enqueued jobs from the max
burst size. After subtraction is done, ensure that any negative values
(which should never occur if the user respects the reported limits), are
clamped to zero.

Fixes: 9459de4edc99 ("dma/idxd: add burst capacity")
Cc: kevin.laatz@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/dma/idxd/idxd_common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/idxd_common.c b/drivers/dma/idxd/idxd_common.c
index fc11b11337..4442d1cbbd 100644
--- a/drivers/dma/idxd/idxd_common.c
+++ b/drivers/dma/idxd/idxd_common.c
@@ -485,7 +485,9 @@ idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
 		write_idx += idxd->desc_ring_mask + 1;
 	used_space = write_idx - idxd->ids_returned;

-	return RTE_MIN((idxd->desc_ring_mask - used_space), idxd->max_batch_size);
+	const int ret = RTE_MIN((idxd->desc_ring_mask - used_space),
+			(idxd->max_batch_size - idxd->batch_size));
+	return ret < 0 ? 0 : (uint16_t)ret;
 }

 int
--
2.32.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] dma/idxd: fix burst capacity calculation
  2021-12-20 17:05 [PATCH] dma/idxd: fix burst capacity calculation Bruce Richardson
@ 2022-01-04 17:16 ` Kevin Laatz
  2022-01-05  1:32   ` Hu, Jiayu
  2022-01-10 13:09 ` Pai G, Sunil
  2022-01-11 13:41 ` [PATCH v2 0/4] fixes for dma/idxd Bruce Richardson
  2 siblings, 1 reply; 17+ messages in thread
From: Kevin Laatz @ 2022-01-04 17:16 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: stable


On 20/12/2021 17:05, Bruce Richardson wrote:
> When the maximum burst size supported by HW is less than the available
> ring space, incorrect capacity was returned when there was already some
> jobs queued up for submission. This was because the capacity calculation
> failed to subtract the number of already-enqueued jobs from the max
> burst size. After subtraction is done, ensure that any negative values
> (which should never occur if the user respects the reported limits), are
> clamped to zero.
>
> Fixes: 9459de4edc99 ("dma/idxd: add burst capacity")
> Cc: kevin.laatz@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   drivers/dma/idxd/idxd_common.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
Acked-by: Kevin Laatz <kevin.laatz@intel.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH] dma/idxd: fix burst capacity calculation
  2022-01-04 17:16 ` Kevin Laatz
@ 2022-01-05  1:32   ` Hu, Jiayu
  0 siblings, 0 replies; 17+ messages in thread
From: Hu, Jiayu @ 2022-01-05  1:32 UTC (permalink / raw)
  To: Laatz, Kevin, Richardson, Bruce, dev; +Cc: stable

Tested-by: Jiayu Hu <jiayu.hu@intel.com>

> -----Original Message-----
> From: Kevin Laatz <kevin.laatz@intel.com>
> Sent: Wednesday, January 5, 2022 1:17 AM
> To: Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: Re: [PATCH] dma/idxd: fix burst capacity calculation
> 
> 
> On 20/12/2021 17:05, Bruce Richardson wrote:
> > When the maximum burst size supported by HW is less than the available
> > ring space, incorrect capacity was returned when there was already
> > some jobs queued up for submission. This was because the capacity
> > calculation failed to subtract the number of already-enqueued jobs
> > from the max burst size. After subtraction is done, ensure that any
> > negative values (which should never occur if the user respects the
> > reported limits), are clamped to zero.
> >
> > Fixes: 9459de4edc99 ("dma/idxd: add burst capacity")
> > Cc: kevin.laatz@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >   drivers/dma/idxd/idxd_common.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> Acked-by: Kevin Laatz <kevin.laatz@intel.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH] dma/idxd: fix burst capacity calculation
  2021-12-20 17:05 [PATCH] dma/idxd: fix burst capacity calculation Bruce Richardson
  2022-01-04 17:16 ` Kevin Laatz
@ 2022-01-10 13:09 ` Pai G, Sunil
  2022-01-10 13:25   ` Bruce Richardson
  2022-01-11 13:41 ` [PATCH v2 0/4] fixes for dma/idxd Bruce Richardson
  2 siblings, 1 reply; 17+ messages in thread
From: Pai G, Sunil @ 2022-01-10 13:09 UTC (permalink / raw)
  To: Richardson, Bruce, dev; +Cc: Richardson, Bruce, Laatz, Kevin, stable, Hu, Jiayu

Hi Bruce, Kevin

This patch seems to have uncovered a bug in the driver.
On applying, the idxd_burst_capacity API seems to return 0 for cases even when there are batch descriptors and ring space available.
Seems like there is a wraparound missing when calculating the descriptor ring space, causing this behavior.  

Below change seems to fix the issue.

@@ -483,7 +496,7 @@ idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
        /* For descriptors, check for wrap-around on write but not read */                   
        if (idxd->ids_returned > write_idx)                                                  
                write_idx += idxd->desc_ring_mask + 1;                                       
-       used_space = write_idx - idxd->ids_returned;                                         
+       used_space = (write_idx - idxd->ids_returned)&idxd->desc_ring_mask;                  

<snipped>

Could we include this fix in the current patch ?

Thanks and regards,
Sunil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] dma/idxd: fix burst capacity calculation
  2022-01-10 13:09 ` Pai G, Sunil
@ 2022-01-10 13:25   ` Bruce Richardson
  2022-01-10 13:44     ` Pai G, Sunil
  0 siblings, 1 reply; 17+ messages in thread
From: Bruce Richardson @ 2022-01-10 13:25 UTC (permalink / raw)
  To: Pai G, Sunil; +Cc: dev, Laatz, Kevin, stable, Hu, Jiayu

On Mon, Jan 10, 2022 at 01:09:02PM +0000, Pai G, Sunil wrote:
> Hi Bruce, Kevin
> 
> This patch seems to have uncovered a bug in the driver.
> On applying, the idxd_burst_capacity API seems to return 0 for cases even when there are batch descriptors and ring space available.
> Seems like there is a wraparound missing when calculating the descriptor ring space, causing this behavior.
> 
> Below change seems to fix the issue.
> 
> @@ -483,7 +496,7 @@ idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
>         /* For descriptors, check for wrap-around on write but not read */
>         if (idxd->ids_returned > write_idx)
>                 write_idx += idxd->desc_ring_mask + 1;
> -       used_space = write_idx - idxd->ids_returned;
> +       used_space = (write_idx - idxd->ids_returned)&idxd->desc_ring_mask;
> 
> <snipped>
> 
> Could we include this fix in the current patch ?
>
Hi Sunil,

what values for the write_idx and ids_returned vars give this error, and
how does masking help? I'd expect masking to increase the number of times
the function returns zero, rather than decreasing it.

/Bruce 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH] dma/idxd: fix burst capacity calculation
  2022-01-10 13:25   ` Bruce Richardson
@ 2022-01-10 13:44     ` Pai G, Sunil
  2022-01-10 16:18       ` Bruce Richardson
  0 siblings, 1 reply; 17+ messages in thread
From: Pai G, Sunil @ 2022-01-10 13:44 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev, Laatz, Kevin, stable, Hu, Jiayu

Hi Bruce,

> what values for the write_idx and ids_returned vars give this error, and how
> does masking help? I'd expect masking to increase the number of times the
> function returns zero, rather than decreasing it.


Here are the values from the idxd dump:
dev_capa: 0x500000051 - mem2mem sva handles_errors copy fill
max_vchans_supported: 1
nb_vchans_configured: 1
silent_mode: off
 IDXD Private Data ==
Portal: 0x7ffff7ffb000
Config: { ring_size: 4096 }
Batch ring (sz = 129, max_batches = 128):
62370  62402  62434  62466  62498  62530  62562  62594  62626  62658  62690  62722  62754  62786  62818  62850  62882  62914  62946  62978  63010  63042  63074  6
3106  63138  63170  63202  63234  63266  63298  63330  63362  63394  63426  63458  63490  63522  63554  63586  63618  63650  63682  63714  63746  63778  63810  63842  63874  63906  63938  63970  64002  64034  64066  64098  6413
0  64162  64194  64226  64258  64290  64322  64354  64386  64418  64450  64482  64514  64546  64578  64610  64642  64674  64706  64738  64770  64802  64834  64866  64898  64930  64962  64994  65026  65058  65090  65122  65154  
65186  65218  65250  65282  65314  65346  65378  65410  65442  65474  65506 [rd ptr]  2 [wr ptr]  61442  61474  61506  61538  61570  61602  61634  61666  61698  61730  61762  61794  61826  61858  61890  61922  61954  61986  620
18  62050  62082  62114  62146  62178  62210  62242  62274  62306  62338
  Curr batch: start = 2, size = 0
  IDS: avail = 65506, returned: 65506
max packets per batch from hw: 1024
batches left: 127, ring space left: 4064
idxd->desc_ring_mask: 4095, used_space: 4128, used_space: 4128, idxd->max_batch_size: 1024, idxd->batch_size: 0
write_idx: 4098, idxd->batch_idx_read: 98, idxd->batch_idx_write: 99, idxd->desc_ring_mask - used_space: 65503

relevant data from above:
write_idx: 4098 , IDS returned: 65506, idxd->desc_ring_mask: 4095

without the fix :
used_space = write_idx - idxd->ids_returned; (4098 - 65506)   = -61408

with fix: 
used_space = (write_idx - idxd->ids_returned)& idxd->desc_ring_mask; (4098 - 65506)&4095   = 32 
which seems to match the rd ptr and wr ptr.

Thanks and regards,
Sunil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] dma/idxd: fix burst capacity calculation
  2022-01-10 13:44     ` Pai G, Sunil
@ 2022-01-10 16:18       ` Bruce Richardson
  0 siblings, 0 replies; 17+ messages in thread
From: Bruce Richardson @ 2022-01-10 16:18 UTC (permalink / raw)
  To: Pai G, Sunil; +Cc: dev, Laatz, Kevin, stable, Hu, Jiayu

On Mon, Jan 10, 2022 at 01:44:06PM +0000, Pai G, Sunil wrote:
> Hi Bruce,
> 
> > what values for the write_idx and ids_returned vars give this error, and how
> > does masking help? I'd expect masking to increase the number of times the
> > function returns zero, rather than decreasing it.
> 
> 
> Here are the values from the idxd dump:
> dev_capa: 0x500000051 - mem2mem sva handles_errors copy fill
> max_vchans_supported: 1
> nb_vchans_configured: 1
> silent_mode: off
>  IDXD Private Data ==
> Portal: 0x7ffff7ffb000
> Config: { ring_size: 4096 }
> Batch ring (sz = 129, max_batches = 128):
> 62370  62402  62434  62466  62498  62530  62562  62594  62626  62658  62690  62722  62754  62786  62818  62850  62882  62914  62946  62978  63010  63042  63074  6
> 3106  63138  63170  63202  63234  63266  63298  63330  63362  63394  63426  63458  63490  63522  63554  63586  63618  63650  63682  63714  63746  63778  63810  63842  63874  63906  63938  63970  64002  64034  64066  64098  6413
> 0  64162  64194  64226  64258  64290  64322  64354  64386  64418  64450  64482  64514  64546  64578  64610  64642  64674  64706  64738  64770  64802  64834  64866  64898  64930  64962  64994  65026  65058  65090  65122  65154
> 65186  65218  65250  65282  65314  65346  65378  65410  65442  65474  65506 [rd ptr]  2 [wr ptr]  61442  61474  61506  61538  61570  61602  61634  61666  61698  61730  61762  61794  61826  61858  61890  61922  61954  61986  620
> 18  62050  62082  62114  62146  62178  62210  62242  62274  62306  62338
>   Curr batch: start = 2, size = 0
>   IDS: avail = 65506, returned: 65506
> max packets per batch from hw: 1024
> batches left: 127, ring space left: 4064
> idxd->desc_ring_mask: 4095, used_space: 4128, used_space: 4128, idxd->max_batch_size: 1024, idxd->batch_size: 0
> write_idx: 4098, idxd->batch_idx_read: 98, idxd->batch_idx_write: 99, idxd->desc_ring_mask - used_space: 65503
> 
> relevant data from above:
> write_idx: 4098 , IDS returned: 65506, idxd->desc_ring_mask: 4095
> 
> without the fix :
> used_space = write_idx - idxd->ids_returned; (4098 - 65506)   = -61408
> 
> with fix:
> used_space = (write_idx - idxd->ids_returned)& idxd->desc_ring_mask; (4098 - 65506)&4095   = 32
> which seems to match the rd ptr and wr ptr.
> 
Thanks, Sunil, that's clear now.
Rather than clamping at the end, I think it may be more logical to clamp
the ids_returned value at the start instead. How about the following diff -
does that also fix it for you?

/Bruce

--- a/drivers/dma/idxd/idxd_common.c
+++ b/drivers/dma/idxd/idxd_common.c
@@ -472,6 +472,7 @@ uint16_t
 idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
 {
        const struct idxd_dmadev *idxd = dev_private;
+       const uint16_t read_idx = idxd->ids_returned & idxd->desc_ring_mask;
        uint16_t write_idx = idxd->batch_start + idxd->batch_size;
        uint16_t used_space;
 
@@ -481,9 +482,9 @@ idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
                return 0;
 
        /* For descriptors, check for wrap-around on write but not read */
-       if (idxd->ids_returned > write_idx)
+       if (read_idx > write_idx)
                write_idx += idxd->desc_ring_mask + 1;
-       used_space = write_idx - idxd->ids_returned;
+       used_space = write_idx - read_idx;
 
        return RTE_MIN((idxd->desc_ring_mask - used_space), idxd->max_batch_size);
 }


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 0/4] fixes for dma/idxd
  2021-12-20 17:05 [PATCH] dma/idxd: fix burst capacity calculation Bruce Richardson
  2022-01-04 17:16 ` Kevin Laatz
  2022-01-10 13:09 ` Pai G, Sunil
@ 2022-01-11 13:41 ` Bruce Richardson
  2022-01-11 13:41   ` [PATCH v2 1/4] dma/idxd: fix burst capacity calculation Bruce Richardson
                     ` (4 more replies)
  2 siblings, 5 replies; 17+ messages in thread
From: Bruce Richardson @ 2022-01-11 13:41 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson

Collection of fixes for idxd driver, including one small enhancement to the
unit tests to help catch future errors too.

V2:
* Changed from single patch for one issue (now patch 1) to multiple
  patches to cover other issues discovered.

Bruce Richardson (4):
  dma/idxd: fix burst capacity calculation
  dma/idxd: fix paths to driver sysfs directory
  dma/idxd: fix wrap-around in burst capacity calculation
  test_dmadev: increase iterations of capacity test case

 app/test/test_dmadev.c            |  7 ++++---
 drivers/dma/idxd/dpdk_idxd_cfg.py | 18 ++++++++++++++----
 drivers/dma/idxd/idxd_common.c    | 10 +++++-----
 3 files changed, 23 insertions(+), 12 deletions(-)

--
2.32.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/4] dma/idxd: fix burst capacity calculation
  2022-01-11 13:41 ` [PATCH v2 0/4] fixes for dma/idxd Bruce Richardson
@ 2022-01-11 13:41   ` Bruce Richardson
  2022-01-11 13:41   ` [PATCH v2 2/4] dma/idxd: fix paths to driver sysfs directory Bruce Richardson
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Bruce Richardson @ 2022-01-11 13:41 UTC (permalink / raw)
  To: dev
  Cc: Bruce Richardson, kevin.laatz, stable, Jiayu Hu, Conor Walsh,
	Chengwen Feng

When the maximum burst size supported by HW is less than the available
ring space, incorrect capacity was returned when there was already some
jobs queued up for submission. This was because the capacity calculation
failed to subtract the number of already-enqueued jobs from the max
burst size. After subtraction is done, ensure that any negative values
(which should never occur if the user respects the reported limits), are
clamped to zero.

Fixes: 9459de4edc99 ("dma/idxd: add burst capacity")
Cc: kevin.laatz@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Kevin Laatz <kevin.laatz@intel.com>
Tested-by: Jiayu Hu <jiayu.hu@intel.com>
---
 drivers/dma/idxd/idxd_common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/idxd_common.c b/drivers/dma/idxd/idxd_common.c
index fc11b11337..4442d1cbbd 100644
--- a/drivers/dma/idxd/idxd_common.c
+++ b/drivers/dma/idxd/idxd_common.c
@@ -485,7 +485,9 @@ idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
 		write_idx += idxd->desc_ring_mask + 1;
 	used_space = write_idx - idxd->ids_returned;
 
-	return RTE_MIN((idxd->desc_ring_mask - used_space), idxd->max_batch_size);
+	const int ret = RTE_MIN((idxd->desc_ring_mask - used_space),
+			(idxd->max_batch_size - idxd->batch_size));
+	return ret < 0 ? 0 : (uint16_t)ret;
 }
 
 int
-- 
2.32.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 2/4] dma/idxd: fix paths to driver sysfs directory
  2022-01-11 13:41 ` [PATCH v2 0/4] fixes for dma/idxd Bruce Richardson
  2022-01-11 13:41   ` [PATCH v2 1/4] dma/idxd: fix burst capacity calculation Bruce Richardson
@ 2022-01-11 13:41   ` Bruce Richardson
  2022-01-11 16:50     ` Kevin Laatz
  2022-01-11 13:41   ` [PATCH v2 3/4] dma/idxd: fix wrap-around in burst capacity calculation Bruce Richardson
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Bruce Richardson @ 2022-01-11 13:41 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable, Kevin Laatz, Radu Nicolau

Recent kernel changes[1][2] mean that we cannot guarantee that the paths
in sysfs used for creating/binding a DSA or workqueue instance will be
as given in the utility script, since they are now "compatibility-mode
only". Update script to support both new paths and compatibility ones.

[1] https://lore.kernel.org/all/162637445139.744545.6008938867943724701.stgit@djiang5-desk3.ch.intel.com/
[2] https://lore.kernel.org/all/162637468705.744545.4399080971745974435.stgit@djiang5-desk3.ch.intel.com/

Fixes: 01863b9d2354 ("raw/ioat: include example configuration script")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/dma/idxd/dpdk_idxd_cfg.py | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/idxd/dpdk_idxd_cfg.py b/drivers/dma/idxd/dpdk_idxd_cfg.py
index fcc27822ef..34537cb980 100755
--- a/drivers/dma/idxd/dpdk_idxd_cfg.py
+++ b/drivers/dma/idxd/dpdk_idxd_cfg.py
@@ -29,9 +29,17 @@ def write_values(self, values):
                 f.write(str(contents))
 
 
+def get_drv_dir(dtype):
+    "Get the sysfs path for the driver, either 'idxd' or 'user'"
+    drv_dir = "/sys/bus/dsa/drivers/" + dtype
+    if not os.path.exists(drv_dir):
+        return "/sys/bus/dsa/drivers/dsa"
+    return drv_dir
+
+
 def reset_device(dsa_id):
     "Reset the DSA device and all its queues"
-    drv_dir = SysfsDir("/sys/bus/dsa/drivers/dsa")
+    drv_dir = SysfsDir(get_drv_dir("idxd"))
     drv_dir.write_values({"unbind": f"dsa{dsa_id}"})
 
 
@@ -58,7 +66,6 @@ def get_dsa_id(pci):
 def configure_dsa(dsa_id, queues, prefix):
     "Configure the DSA instance with appropriate number of queues"
     dsa_dir = SysfsDir(f"/sys/bus/dsa/devices/dsa{dsa_id}")
-    drv_dir = SysfsDir("/sys/bus/dsa/drivers/dsa")
 
     max_groups = dsa_dir.read_int("max_groups")
     max_engines = dsa_dir.read_int("max_engines")
@@ -85,9 +92,12 @@ def configure_dsa(dsa_id, queues, prefix):
                              "size": int(max_work_queues_size / nb_queues)})
 
     # enable device and then queues
-    drv_dir.write_values({"bind": f"dsa{dsa_id}"})
+    idxd_dir = SysfsDir(get_drv_dir("idxd"))
+    idxd_dir.write_values({"bind": f"dsa{dsa_id}"})
+
+    user_dir = SysfsDir(get_drv_dir("user"))
     for q in range(nb_queues):
-        drv_dir.write_values({"bind": f"wq{dsa_id}.{q}"})
+        user_dir.write_values({"bind": f"wq{dsa_id}.{q}"})
 
 
 def main(args):
-- 
2.32.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 3/4] dma/idxd: fix wrap-around in burst capacity calculation
  2022-01-11 13:41 ` [PATCH v2 0/4] fixes for dma/idxd Bruce Richardson
  2022-01-11 13:41   ` [PATCH v2 1/4] dma/idxd: fix burst capacity calculation Bruce Richardson
  2022-01-11 13:41   ` [PATCH v2 2/4] dma/idxd: fix paths to driver sysfs directory Bruce Richardson
@ 2022-01-11 13:41   ` Bruce Richardson
  2022-01-11 13:45     ` Pai G, Sunil
  2022-01-11 16:50     ` Kevin Laatz
  2022-01-11 13:41   ` [PATCH v2 4/4] test_dmadev: increase iterations of capacity test case Bruce Richardson
  2022-01-20 13:06   ` [PATCH v2 0/4] fixes for dma/idxd Thomas Monjalon
  4 siblings, 2 replies; 17+ messages in thread
From: Bruce Richardson @ 2022-01-11 13:41 UTC (permalink / raw)
  To: dev
  Cc: Bruce Richardson, kevin.laatz, stable, Sunil Pai G, Conor Walsh,
	Chengwen Feng

The burst capacity calculation code assumes that the write and read
(i.e. ids_returned) values both wrap at the ring-size, but the read
value instead wraps as UINT16_MAX. Therefore, instead of just adding
ring-size to the write value in case the read is greater, we need to
just always mask the result to ensure a correct, in-range, value.

Fixes: 9459de4edc99 ("dma/idxd: add burst capacity")
Cc: kevin.laatz@intel.com
Cc: stable@dpdk.org

Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/dma/idxd/idxd_common.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/idxd/idxd_common.c b/drivers/dma/idxd/idxd_common.c
index 4442d1cbbd..ea6413cc7a 100644
--- a/drivers/dma/idxd/idxd_common.c
+++ b/drivers/dma/idxd/idxd_common.c
@@ -480,10 +480,8 @@ idxd_burst_capacity(const void *dev_private, uint16_t vchan __rte_unused)
 			idxd->batch_idx_write + 1 == idxd->batch_idx_read)
 		return 0;
 
-	/* For descriptors, check for wrap-around on write but not read */
-	if (idxd->ids_returned > write_idx)
-		write_idx += idxd->desc_ring_mask + 1;
-	used_space = write_idx - idxd->ids_returned;
+	/* Subtract and mask to get in correct range */
+	used_space = (write_idx - idxd->ids_returned) & idxd->desc_ring_mask;
 
 	const int ret = RTE_MIN((idxd->desc_ring_mask - used_space),
 			(idxd->max_batch_size - idxd->batch_size));
-- 
2.32.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 4/4] test_dmadev: increase iterations of capacity test case
  2022-01-11 13:41 ` [PATCH v2 0/4] fixes for dma/idxd Bruce Richardson
                     ` (2 preceding siblings ...)
  2022-01-11 13:41   ` [PATCH v2 3/4] dma/idxd: fix wrap-around in burst capacity calculation Bruce Richardson
@ 2022-01-11 13:41   ` Bruce Richardson
  2022-01-11 16:50     ` Kevin Laatz
  2022-01-20 13:06   ` [PATCH v2 0/4] fixes for dma/idxd Thomas Monjalon
  4 siblings, 1 reply; 17+ messages in thread
From: Bruce Richardson @ 2022-01-11 13:41 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Chengwen Feng, Kevin Laatz

To ensure we catch any bugs in calculation due to wrap-around of the id
values, increase the number of iterations of the burst_capacity test.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/test_dmadev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index b206db27ae..db5aff701c 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -686,10 +686,11 @@ test_burst_capacity(int16_t dev_id, uint16_t vchan)
 	/* to test capacity, we enqueue elements and check capacity is reduced
 	 * by one each time - rebaselining the expected value after each burst
 	 * as the capacity is only for a burst. We enqueue multiple bursts to
-	 * fill up half the ring, before emptying it again. We do this twice to
-	 * ensure that we get to test scenarios where we get ring wrap-around
+	 * fill up half the ring, before emptying it again. We do this multiple
+	 * times to ensure that we get to test scenarios where we get ring
+	 * wrap-around and wrap-around of the ids returned (at UINT16_MAX).
 	 */
-	for (iter = 0; iter < 2; iter++) {
+	for (iter = 0; iter < 2 * (((int)UINT16_MAX + 1) / ring_space); iter++) {
 		for (i = 0; i < (ring_space / (2 * CAP_TEST_BURST_SIZE)) + 1; i++) {
 			cap = rte_dma_burst_capacity(dev_id, vchan);
 
-- 
2.32.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2 3/4] dma/idxd: fix wrap-around in burst capacity calculation
  2022-01-11 13:41   ` [PATCH v2 3/4] dma/idxd: fix wrap-around in burst capacity calculation Bruce Richardson
@ 2022-01-11 13:45     ` Pai G, Sunil
  2022-01-11 16:50     ` Kevin Laatz
  1 sibling, 0 replies; 17+ messages in thread
From: Pai G, Sunil @ 2022-01-11 13:45 UTC (permalink / raw)
  To: Richardson, Bruce, dev; +Cc: Laatz, Kevin, stable, Walsh, Conor, Chengwen Feng

Tested-by: Sunil Pai G <sunil.pai.g@intel.com>

<snipped>

Thanks and regards,
Sunil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/4] dma/idxd: fix paths to driver sysfs directory
  2022-01-11 13:41   ` [PATCH v2 2/4] dma/idxd: fix paths to driver sysfs directory Bruce Richardson
@ 2022-01-11 16:50     ` Kevin Laatz
  0 siblings, 0 replies; 17+ messages in thread
From: Kevin Laatz @ 2022-01-11 16:50 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: stable, Radu Nicolau

On 11/01/2022 13:41, Bruce Richardson wrote:
> Recent kernel changes[1][2] mean that we cannot guarantee that the paths
> in sysfs used for creating/binding a DSA or workqueue instance will be
> as given in the utility script, since they are now "compatibility-mode
> only". Update script to support both new paths and compatibility ones.
>
> [1] https://lore.kernel.org/all/162637445139.744545.6008938867943724701.stgit@djiang5-desk3.ch.intel.com/
> [2] https://lore.kernel.org/all/162637468705.744545.4399080971745974435.stgit@djiang5-desk3.ch.intel.com/
>
> Fixes: 01863b9d2354 ("raw/ioat: include example configuration script")
> Cc: stable@dpdk.org
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   drivers/dma/idxd/dpdk_idxd_cfg.py | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>

Acked-by: Kevin Laatz <kevin.laatz@intel.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 3/4] dma/idxd: fix wrap-around in burst capacity calculation
  2022-01-11 13:41   ` [PATCH v2 3/4] dma/idxd: fix wrap-around in burst capacity calculation Bruce Richardson
  2022-01-11 13:45     ` Pai G, Sunil
@ 2022-01-11 16:50     ` Kevin Laatz
  1 sibling, 0 replies; 17+ messages in thread
From: Kevin Laatz @ 2022-01-11 16:50 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: stable, Sunil Pai G, Conor Walsh, Chengwen Feng

On 11/01/2022 13:41, Bruce Richardson wrote:
> The burst capacity calculation code assumes that the write and read
> (i.e. ids_returned) values both wrap at the ring-size, but the read
> value instead wraps as UINT16_MAX. Therefore, instead of just adding
> ring-size to the write value in case the read is greater, we need to
> just always mask the result to ensure a correct, in-range, value.
>
> Fixes: 9459de4edc99 ("dma/idxd: add burst capacity")
> Cc: kevin.laatz@intel.com
> Cc: stable@dpdk.org
>
> Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   drivers/dma/idxd/idxd_common.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>

Acked-by: Kevin Laatz <kevin.laatz@intel.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/4] test_dmadev: increase iterations of capacity test case
  2022-01-11 13:41   ` [PATCH v2 4/4] test_dmadev: increase iterations of capacity test case Bruce Richardson
@ 2022-01-11 16:50     ` Kevin Laatz
  0 siblings, 0 replies; 17+ messages in thread
From: Kevin Laatz @ 2022-01-11 16:50 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: Chengwen Feng

On 11/01/2022 13:41, Bruce Richardson wrote:
> To ensure we catch any bugs in calculation due to wrap-around of the id
> values, increase the number of iterations of the burst_capacity test.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   app/test/test_dmadev.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>

Acked-by: Kevin Laatz <kevin.laatz@intel.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/4] fixes for dma/idxd
  2022-01-11 13:41 ` [PATCH v2 0/4] fixes for dma/idxd Bruce Richardson
                     ` (3 preceding siblings ...)
  2022-01-11 13:41   ` [PATCH v2 4/4] test_dmadev: increase iterations of capacity test case Bruce Richardson
@ 2022-01-20 13:06   ` Thomas Monjalon
  4 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2022-01-20 13:06 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

11/01/2022 14:41, Bruce Richardson:
> Collection of fixes for idxd driver, including one small enhancement to the
> unit tests to help catch future errors too.
> 
> V2:
> * Changed from single patch for one issue (now patch 1) to multiple
>   patches to cover other issues discovered.
> 
> Bruce Richardson (4):
>   dma/idxd: fix burst capacity calculation
>   dma/idxd: fix paths to driver sysfs directory
>   dma/idxd: fix wrap-around in burst capacity calculation
>   test_dmadev: increase iterations of capacity test case

Applied, thanks




^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-01-20 13:07 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-20 17:05 [PATCH] dma/idxd: fix burst capacity calculation Bruce Richardson
2022-01-04 17:16 ` Kevin Laatz
2022-01-05  1:32   ` Hu, Jiayu
2022-01-10 13:09 ` Pai G, Sunil
2022-01-10 13:25   ` Bruce Richardson
2022-01-10 13:44     ` Pai G, Sunil
2022-01-10 16:18       ` Bruce Richardson
2022-01-11 13:41 ` [PATCH v2 0/4] fixes for dma/idxd Bruce Richardson
2022-01-11 13:41   ` [PATCH v2 1/4] dma/idxd: fix burst capacity calculation Bruce Richardson
2022-01-11 13:41   ` [PATCH v2 2/4] dma/idxd: fix paths to driver sysfs directory Bruce Richardson
2022-01-11 16:50     ` Kevin Laatz
2022-01-11 13:41   ` [PATCH v2 3/4] dma/idxd: fix wrap-around in burst capacity calculation Bruce Richardson
2022-01-11 13:45     ` Pai G, Sunil
2022-01-11 16:50     ` Kevin Laatz
2022-01-11 13:41   ` [PATCH v2 4/4] test_dmadev: increase iterations of capacity test case Bruce Richardson
2022-01-11 16:50     ` Kevin Laatz
2022-01-20 13:06   ` [PATCH v2 0/4] fixes for dma/idxd Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).