* [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters @ 2020-07-13 7:15 Cheng Jiang 2020-07-13 9:31 ` Bruce Richardson ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Cheng Jiang @ 2020-07-13 7:15 UTC (permalink / raw) To: bruce.richardson, dev; +Cc: patrick.fu, Cheng Jiang Added a flag which controls whether rte_ioat_enqueue_copy and rte_ioat_completed_copies function should process handle parameters to improve the performance when handle parameters are not necessary to use. This is targeting 20.11 release. Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> --- drivers/raw/ioat/ioat_rawdev.c | 1 + drivers/raw/ioat/rte_ioat_rawdev.h | 14 +++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index 87fd088aa..5bf030785 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -57,6 +57,7 @@ ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) return -EINVAL; ioat->ring_size = params->ring_size; + ioat->hdls_enable = params->hdls_enable; if (ioat->desc_ring != NULL) { rte_memzone_free(ioat->desc_mz); ioat->desc_ring = NULL; diff --git a/drivers/raw/ioat/rte_ioat_rawdev.h b/drivers/raw/ioat/rte_ioat_rawdev.h index f765a6557..daca04dd3 100644 --- a/drivers/raw/ioat/rte_ioat_rawdev.h +++ b/drivers/raw/ioat/rte_ioat_rawdev.h @@ -35,6 +35,7 @@ */ struct rte_ioat_rawdev_config { unsigned short ring_size; + bool hdls_enable; }; /** @@ -52,6 +53,8 @@ struct rte_ioat_rawdev { unsigned short ring_size; struct rte_ioat_generic_hw_desc *desc_ring; + + bool hdls_enable; /* control if handles need to be copied */ __m128i *hdls; /* completion handles for returning to user */ @@ -121,8 +124,10 @@ rte_ioat_enqueue_copy(int dev_id, phys_addr_t src, phys_addr_t dst, desc->u.control_raw = (uint32_t)((!!fence << 4) | (!(write & 0xF)) << 3); desc->src_addr = src; desc->dest_addr = dst; + if (ioat->hdls_enable) + ioat->hdls[write] = _mm_set_epi64x((int64_t)dst_hdl, + (int64_t)src_hdl); - ioat->hdls[write] = _mm_set_epi64x((int64_t)dst_hdl, (int64_t)src_hdl); rte_prefetch0(&ioat->desc_ring[ioat->next_write & mask]); ioat->enqueued++; @@ -208,6 +213,11 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, if (count > max_copies) count = max_copies; + ioat->next_read = read + count; + ioat->completed += count; + if (!ioat->hdls_enable) + return count; + for (; i < count - 1; i += 2, read += 2) { __m128i hdls0 = _mm_load_si128(&ioat->hdls[read & mask]); __m128i hdls1 = _mm_load_si128(&ioat->hdls[(read + 1) & mask]); @@ -223,8 +233,6 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, dst_hdls[i] = hdls[1]; } - ioat->next_read = read; - ioat->completed += count; return count; } -- 2.27.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters 2020-07-13 7:15 [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters Cheng Jiang @ 2020-07-13 9:31 ` Bruce Richardson 2020-07-13 9:55 ` Bruce Richardson ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Bruce Richardson @ 2020-07-13 9:31 UTC (permalink / raw) To: Cheng Jiang; +Cc: dev, patrick.fu On Mon, Jul 13, 2020 at 07:15:19AM +0000, Cheng Jiang wrote: > Added a flag which controls whether rte_ioat_enqueue_copy > and rte_ioat_completed_copies function should process > handle parameters to improve the performance when handle > parameters are not necessary to use. This is targeting > 20.11 release. > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > --- Thanks for this. I'm also preparing some changes to this driver for 20.11 release, so I'll base my changes on top of this one. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters 2020-07-13 7:15 [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters Cheng Jiang 2020-07-13 9:31 ` Bruce Richardson @ 2020-07-13 9:55 ` Bruce Richardson 2020-07-13 9:59 ` Bruce Richardson 2020-07-13 13:28 ` Bruce Richardson 2020-07-15 2:16 ` [dpdk-dev] [PATCH 20.11 v2] raw/ioat: add " Cheng Jiang 3 siblings, 1 reply; 9+ messages in thread From: Bruce Richardson @ 2020-07-13 9:55 UTC (permalink / raw) To: Cheng Jiang; +Cc: dev, patrick.fu On Mon, Jul 13, 2020 at 07:15:19AM +0000, Cheng Jiang wrote: > Added a flag which controls whether rte_ioat_enqueue_copy > and rte_ioat_completed_copies function should process > handle parameters to improve the performance when handle > parameters are not necessary to use. This is targeting > 20.11 release. > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > --- > drivers/raw/ioat/ioat_rawdev.c | 1 + > drivers/raw/ioat/rte_ioat_rawdev.h | 14 +++++++++++--- > 2 files changed, 12 insertions(+), 3 deletions(-) > <snip> > @@ -208,6 +213,11 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, > if (count > max_copies) > count = max_copies; > > + ioat->next_read = read + count; > + ioat->completed += count; > + if (!ioat->hdls_enable) > + return count; > + > for (; i < count - 1; i += 2, read += 2) { > __m128i hdls0 = _mm_load_si128(&ioat->hdls[read & mask]); > __m128i hdls1 = _mm_load_si128(&ioat->hdls[(read + 1) & mask]); > @@ -223,8 +233,6 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, > dst_hdls[i] = hdls[1]; > } > > - ioat->next_read = read; > - ioat->completed += count; > return count; > } This change I think may cause problems if we ever want to have one thread enqueuing and another taking completions. The next_read and completed counters should really only be updated after we have finished reading the completed handles array. Therefore, for safety, I tihnk it might be better to keep the updates in their original places and put an "end:" label before them. Then the "return count" in the middle of the function can be "goto end;" Regards, /Bruce ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters 2020-07-13 9:55 ` Bruce Richardson @ 2020-07-13 9:59 ` Bruce Richardson 0 siblings, 0 replies; 9+ messages in thread From: Bruce Richardson @ 2020-07-13 9:59 UTC (permalink / raw) To: Cheng Jiang; +Cc: dev, patrick.fu On Mon, Jul 13, 2020 at 10:55:30AM +0100, Bruce Richardson wrote: > On Mon, Jul 13, 2020 at 07:15:19AM +0000, Cheng Jiang wrote: > > Added a flag which controls whether rte_ioat_enqueue_copy > > and rte_ioat_completed_copies function should process > > handle parameters to improve the performance when handle > > parameters are not necessary to use. This is targeting > > 20.11 release. > > > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > > --- > > drivers/raw/ioat/ioat_rawdev.c | 1 + > > drivers/raw/ioat/rte_ioat_rawdev.h | 14 +++++++++++--- > > 2 files changed, 12 insertions(+), 3 deletions(-) > > > <snip> > > @@ -208,6 +213,11 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, > > if (count > max_copies) > > count = max_copies; > > > > + ioat->next_read = read + count; > > + ioat->completed += count; > > + if (!ioat->hdls_enable) > > + return count; > > + > > for (; i < count - 1; i += 2, read += 2) { > > __m128i hdls0 = _mm_load_si128(&ioat->hdls[read & mask]); > > __m128i hdls1 = _mm_load_si128(&ioat->hdls[(read + 1) & mask]); > > @@ -223,8 +233,6 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, > > dst_hdls[i] = hdls[1]; > > } > > > > - ioat->next_read = read; > > - ioat->completed += count; > > return count; > > } > > This change I think may cause problems if we ever want to have one thread > enqueuing and another taking completions. The next_read and completed > counters should really only be updated after we have finished reading the > completed handles array. Therefore, for safety, I tihnk it might be better > to keep the updates in their original places and put an "end:" label before > them. Then the "return count" in the middle of the function can be "goto > end;" > A further suggestion to the changes to this function: if we are not actually returning completion handles, then there is no need to limit the count to "max_copies". Therefore move the "return count" or "goto end" above the previous length check, and update the doxygen comments to indicate that max_copies is ignored if "hdls_enable" is false, and that the final two parameters can also be NULL when calling the function in this case. /Bruce ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters 2020-07-13 7:15 [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters Cheng Jiang 2020-07-13 9:31 ` Bruce Richardson 2020-07-13 9:55 ` Bruce Richardson @ 2020-07-13 13:28 ` Bruce Richardson 2020-07-14 10:45 ` Jiang, Cheng1 2020-07-15 2:16 ` [dpdk-dev] [PATCH 20.11 v2] raw/ioat: add " Cheng Jiang 3 siblings, 1 reply; 9+ messages in thread From: Bruce Richardson @ 2020-07-13 13:28 UTC (permalink / raw) To: Cheng Jiang; +Cc: dev, patrick.fu On Mon, Jul 13, 2020 at 07:15:19AM +0000, Cheng Jiang wrote: > Added a flag which controls whether rte_ioat_enqueue_copy > and rte_ioat_completed_copies function should process > handle parameters to improve the performance when handle > parameters are not necessary to use. This is targeting > 20.11 release. > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > --- > drivers/raw/ioat/ioat_rawdev.c | 1 + > drivers/raw/ioat/rte_ioat_rawdev.h | 14 +++++++++++--- > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c > index 87fd088aa..5bf030785 100644 > --- a/drivers/raw/ioat/ioat_rawdev.c > +++ b/drivers/raw/ioat/ioat_rawdev.c > @@ -57,6 +57,7 @@ ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) > return -EINVAL; > > ioat->ring_size = params->ring_size; > + ioat->hdls_enable = params->hdls_enable; > if (ioat->desc_ring != NULL) { > rte_memzone_free(ioat->desc_mz); > ioat->desc_ring = NULL; > diff --git a/drivers/raw/ioat/rte_ioat_rawdev.h b/drivers/raw/ioat/rte_ioat_rawdev.h > index f765a6557..daca04dd3 100644 > --- a/drivers/raw/ioat/rte_ioat_rawdev.h > +++ b/drivers/raw/ioat/rte_ioat_rawdev.h > @@ -35,6 +35,7 @@ > */ > struct rte_ioat_rawdev_config { > unsigned short ring_size; > + bool hdls_enable; > }; > You need a doxygen comment on the new structure member (and add to existing one). While it's fairly clear what ring_size parameter should do, the hdls_enable one needs a proper explanation. I also think you might want to investigate changing it to "hdls_disable" so that the default zero-value is the current behaviour. Requiring it to be set as 1 means that existing code will likely recompile but fail to work. For example, the ioat_rawdev_autotest fails on completion checks now. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters 2020-07-13 13:28 ` Bruce Richardson @ 2020-07-14 10:45 ` Jiang, Cheng1 0 siblings, 0 replies; 9+ messages in thread From: Jiang, Cheng1 @ 2020-07-14 10:45 UTC (permalink / raw) To: Richardson, Bruce; +Cc: dev, Fu, Patrick Hi Bruce, Thank you very much for these comments which really make sense for me. I will correct these problems in the next version. Thanks again. Cheng > -----Original Message----- > From: Bruce Richardson <bruce.richardson@intel.com> > Sent: Monday, July 13, 2020 9:28 PM > To: Jiang, Cheng1 <cheng1.jiang@intel.com> > Cc: dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com> > Subject: Re: [PATCH 20.11] raw/ioat: added a flag to control copying handle > parameters > > On Mon, Jul 13, 2020 at 07:15:19AM +0000, Cheng Jiang wrote: > > Added a flag which controls whether rte_ioat_enqueue_copy and > > rte_ioat_completed_copies function should process handle parameters to > > improve the performance when handle parameters are not necessary to > > use. This is targeting > > 20.11 release. > > > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > > --- > > drivers/raw/ioat/ioat_rawdev.c | 1 + > > drivers/raw/ioat/rte_ioat_rawdev.h | 14 +++++++++++--- > > 2 files changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/raw/ioat/ioat_rawdev.c > > b/drivers/raw/ioat/ioat_rawdev.c index 87fd088aa..5bf030785 100644 > > --- a/drivers/raw/ioat/ioat_rawdev.c > > +++ b/drivers/raw/ioat/ioat_rawdev.c > > @@ -57,6 +57,7 @@ ioat_dev_configure(const struct rte_rawdev *dev, > rte_rawdev_obj_t config) > > return -EINVAL; > > > > ioat->ring_size = params->ring_size; > > + ioat->hdls_enable = params->hdls_enable; > > if (ioat->desc_ring != NULL) { > > rte_memzone_free(ioat->desc_mz); > > ioat->desc_ring = NULL; > > diff --git a/drivers/raw/ioat/rte_ioat_rawdev.h > > b/drivers/raw/ioat/rte_ioat_rawdev.h > > index f765a6557..daca04dd3 100644 > > --- a/drivers/raw/ioat/rte_ioat_rawdev.h > > +++ b/drivers/raw/ioat/rte_ioat_rawdev.h > > @@ -35,6 +35,7 @@ > > */ > > struct rte_ioat_rawdev_config { > > unsigned short ring_size; > > + bool hdls_enable; > > }; > > > You need a doxygen comment on the new structure member (and add to > existing one). While it's fairly clear what ring_size parameter should do, the > hdls_enable one needs a proper explanation. > > I also think you might want to investigate changing it to "hdls_disable" > so that the default zero-value is the current behaviour. Requiring it to be set > as 1 means that existing code will likely recompile but fail to work. > For example, the ioat_rawdev_autotest fails on completion checks now. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH 20.11 v2] raw/ioat: add a flag to control copying handle parameters 2020-07-13 7:15 [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters Cheng Jiang ` (2 preceding siblings ...) 2020-07-13 13:28 ` Bruce Richardson @ 2020-07-15 2:16 ` Cheng Jiang 2020-07-17 12:49 ` Bruce Richardson 3 siblings, 1 reply; 9+ messages in thread From: Cheng Jiang @ 2020-07-15 2:16 UTC (permalink / raw) To: bruce.richardson, dev; +Cc: patrick.fu, Cheng Jiang Add a flag which controls whether rte_ioat_enqueue_copy and rte_ioat_completed_copies function should process handle parameters to improve the performance when handle parameters are not necessary to use. This is targeting 20.11 release. Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> --- v2: * optimized the logic of some codes * added some comments --- drivers/raw/ioat/ioat_rawdev.c | 1 + drivers/raw/ioat/rte_ioat_rawdev.h | 29 ++++++++++++++++++++++------- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index 87fd088aa..d70e47d52 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -57,6 +57,7 @@ ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) return -EINVAL; ioat->ring_size = params->ring_size; + ioat->hdls_disable = params->hdls_disable; if (ioat->desc_ring != NULL) { rte_memzone_free(ioat->desc_mz); ioat->desc_ring = NULL; diff --git a/drivers/raw/ioat/rte_ioat_rawdev.h b/drivers/raw/ioat/rte_ioat_rawdev.h index f765a6557..cf0e634f3 100644 --- a/drivers/raw/ioat/rte_ioat_rawdev.h +++ b/drivers/raw/ioat/rte_ioat_rawdev.h @@ -31,10 +31,13 @@ * * This structure is to be passed as the ".dev_private" parameter when * calling the rte_rawdev_get_info() and rte_rawdev_configure() APIs on - * an ioat rawdev instance. + * an ioat rawdev instance. The member hdls_disable controls if handles + * need to be copied when calling the rte_ioat_enqueue_copy() and + * rte_ioat_completed_copies() APIs. */ struct rte_ioat_rawdev_config { unsigned short ring_size; + bool hdls_disable; }; /** @@ -52,6 +55,7 @@ struct rte_ioat_rawdev { unsigned short ring_size; struct rte_ioat_generic_hw_desc *desc_ring; + bool hdls_disable; __m128i *hdls; /* completion handles for returning to user */ @@ -84,10 +88,12 @@ struct rte_ioat_rawdev { * The length of the data to be copied * @param src_hdl * An opaque handle for the source data, to be returned when this operation - * has been completed and the user polls for the completion details + * has been completed and the user polls for the completion details if + * hdls_disable is false * @param dst_hdl * An opaque handle for the destination data, to be returned when this * operation has been completed and the user polls for the completion details + * if hdls_disable is false * @param fence * A flag parameter indicating that hardware should not begin to perform any * subsequently enqueued copy operations until after this operation has @@ -121,8 +127,10 @@ rte_ioat_enqueue_copy(int dev_id, phys_addr_t src, phys_addr_t dst, desc->u.control_raw = (uint32_t)((!!fence << 4) | (!(write & 0xF)) << 3); desc->src_addr = src; desc->dest_addr = dst; + if (!ioat->hdls_disable) + ioat->hdls[write] = _mm_set_epi64x((int64_t)dst_hdl, + (int64_t)src_hdl); - ioat->hdls[write] = _mm_set_epi64x((int64_t)dst_hdl, (int64_t)src_hdl); rte_prefetch0(&ioat->desc_ring[ioat->next_write & mask]); ioat->enqueued++; @@ -168,9 +176,11 @@ rte_ioat_get_last_completed(struct rte_ioat_rawdev *ioat, int *error) /** * Returns details of copy operations that have been completed * - * Returns to the caller the user-provided "handles" for the copy operations - * which have been completed by the hardware, and not already returned by - * a previous call to this API. + * If the hdls_disable is false, the function will return to the caller the + * user-provided "handles" for the copy operations which have been completed + * by the hardware, and not already returned by a previous call to this API. + * If the hdls_disable is true, the max_copies will be ignored, and that the + * src_hdls and dst_hdls can be NULL when calling the function. * * @param dev_id * The rawdev device id of the ioat instance @@ -205,6 +215,11 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, return -1; } + if (ioat->hdls_disable) { + read += count; + goto end; + } + if (count > max_copies) count = max_copies; @@ -222,7 +237,7 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, src_hdls[i] = hdls[0]; dst_hdls[i] = hdls[1]; } - +end: ioat->next_read = read; ioat->completed += count; return count; -- 2.27.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11 v2] raw/ioat: add a flag to control copying handle parameters 2020-07-15 2:16 ` [dpdk-dev] [PATCH 20.11 v2] raw/ioat: add " Cheng Jiang @ 2020-07-17 12:49 ` Bruce Richardson 2020-07-21 10:15 ` Bruce Richardson 0 siblings, 1 reply; 9+ messages in thread From: Bruce Richardson @ 2020-07-17 12:49 UTC (permalink / raw) To: Cheng Jiang; +Cc: dev, patrick.fu On Wed, Jul 15, 2020 at 02:16:15AM +0000, Cheng Jiang wrote: > Add a flag which controls whether rte_ioat_enqueue_copy > and rte_ioat_completed_copies function should process > handle parameters to improve the performance when handle > parameters are not necessary to use. This is targeting > 20.11 release. > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > --- > v2: > * optimized the logic of some codes > * added some comments > --- > drivers/raw/ioat/ioat_rawdev.c | 1 + > drivers/raw/ioat/rte_ioat_rawdev.h | 29 ++++++++++++++++++++++------- > 2 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c > index 87fd088aa..d70e47d52 100644 > --- a/drivers/raw/ioat/ioat_rawdev.c > +++ b/drivers/raw/ioat/ioat_rawdev.c > @@ -57,6 +57,7 @@ ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) > return -EINVAL; > > ioat->ring_size = params->ring_size; > + ioat->hdls_disable = params->hdls_disable; > if (ioat->desc_ring != NULL) { > rte_memzone_free(ioat->desc_mz); > ioat->desc_ring = NULL; > diff --git a/drivers/raw/ioat/rte_ioat_rawdev.h b/drivers/raw/ioat/rte_ioat_rawdev.h > index f765a6557..cf0e634f3 100644 > --- a/drivers/raw/ioat/rte_ioat_rawdev.h > +++ b/drivers/raw/ioat/rte_ioat_rawdev.h > @@ -31,10 +31,13 @@ > * > * This structure is to be passed as the ".dev_private" parameter when > * calling the rte_rawdev_get_info() and rte_rawdev_configure() APIs on > - * an ioat rawdev instance. > + * an ioat rawdev instance. The member hdls_disable controls if handles > + * need to be copied when calling the rte_ioat_enqueue_copy() and > + * rte_ioat_completed_copies() APIs. > */ > struct rte_ioat_rawdev_config { > unsigned short ring_size; > + bool hdls_disable; > }; > > /** > @@ -52,6 +55,7 @@ struct rte_ioat_rawdev { > > unsigned short ring_size; > struct rte_ioat_generic_hw_desc *desc_ring; > + bool hdls_disable; > __m128i *hdls; /* completion handles for returning to user */ > > > @@ -84,10 +88,12 @@ struct rte_ioat_rawdev { > * The length of the data to be copied > * @param src_hdl > * An opaque handle for the source data, to be returned when this operation > - * has been completed and the user polls for the completion details > + * has been completed and the user polls for the completion details if > + * hdls_disable is false > * @param dst_hdl > * An opaque handle for the destination data, to be returned when this > * operation has been completed and the user polls for the completion details > + * if hdls_disable is false Minor nit - rather than adding "if hdls_disable is false" to the sentence, I think it would be clearer to just add on as an extra sentence: "NOTE: If hdls_disable configuration option is set, this parameter is ignored" > * @param fence > * A flag parameter indicating that hardware should not begin to perform any > * subsequently enqueued copy operations until after this operation has > @@ -121,8 +127,10 @@ rte_ioat_enqueue_copy(int dev_id, phys_addr_t src, phys_addr_t dst, > desc->u.control_raw = (uint32_t)((!!fence << 4) | (!(write & 0xF)) << 3); > desc->src_addr = src; > desc->dest_addr = dst; > + if (!ioat->hdls_disable) > + ioat->hdls[write] = _mm_set_epi64x((int64_t)dst_hdl, > + (int64_t)src_hdl); > > - ioat->hdls[write] = _mm_set_epi64x((int64_t)dst_hdl, (int64_t)src_hdl); > rte_prefetch0(&ioat->desc_ring[ioat->next_write & mask]); > > ioat->enqueued++; > @@ -168,9 +176,11 @@ rte_ioat_get_last_completed(struct rte_ioat_rawdev *ioat, int *error) > /** > * Returns details of copy operations that have been completed > * > - * Returns to the caller the user-provided "handles" for the copy operations > - * which have been completed by the hardware, and not already returned by > - * a previous call to this API. > + * If the hdls_disable is false, the function will return to the caller the > + * user-provided "handles" for the copy operations which have been completed > + * by the hardware, and not already returned by a previous call to this API. > + * If the hdls_disable is true, the max_copies will be ignored, and that the > + * src_hdls and dst_hdls can be NULL when calling the function. Again I think this might be better left as originally stated, with the additional case of "hdls_disable" being set left till the end. Rather than referring to "true/false" set/not-set are better terms to use. > * > * @param dev_id > * The rawdev device id of the ioat instance > @@ -205,6 +215,11 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, > return -1; > } > > + if (ioat->hdls_disable) { > + read += count; > + goto end; > + } > + > if (count > max_copies) > count = max_copies; > > @@ -222,7 +237,7 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies, > src_hdls[i] = hdls[0]; > dst_hdls[i] = hdls[1]; > } > - > +end: > ioat->next_read = read; > ioat->completed += count; > return count; > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11 v2] raw/ioat: add a flag to control copying handle parameters 2020-07-17 12:49 ` Bruce Richardson @ 2020-07-21 10:15 ` Bruce Richardson 0 siblings, 0 replies; 9+ messages in thread From: Bruce Richardson @ 2020-07-21 10:15 UTC (permalink / raw) To: Cheng Jiang, dev; +Cc: patrick.fu On Fri, Jul 17, 2020 at 01:49:37PM +0100, Bruce Richardson wrote: > On Wed, Jul 15, 2020 at 02:16:15AM +0000, Cheng Jiang wrote: > > Add a flag which controls whether rte_ioat_enqueue_copy > > and rte_ioat_completed_copies function should process > > handle parameters to improve the performance when handle > > parameters are not necessary to use. This is targeting > > 20.11 release. > > > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > > --- With Cheng's agreement, this patch has now been included in the bigger ioat update patchset for 20.11 [1], and does not need to be tracked separately. [1] http://inbox.dpdk.org/dev/20200721095140.719297-1-bruce.richardson@intel.com/ Regards, /Bruce ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-07-21 10:15 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-13 7:15 [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control copying handle parameters Cheng Jiang 2020-07-13 9:31 ` Bruce Richardson 2020-07-13 9:55 ` Bruce Richardson 2020-07-13 9:59 ` Bruce Richardson 2020-07-13 13:28 ` Bruce Richardson 2020-07-14 10:45 ` Jiang, Cheng1 2020-07-15 2:16 ` [dpdk-dev] [PATCH 20.11 v2] raw/ioat: add " Cheng Jiang 2020-07-17 12:49 ` Bruce Richardson 2020-07-21 10:15 ` Bruce Richardson
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).