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