* [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
* 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
* [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
* 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 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
* [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 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).