* Re: [dpdk-dev] [PATCH] examples/vhost: add error propagation in ioat ops [not found] <20210131123628.47883-1-Cheng1.jiang@intel.com> @ 2021-02-03 9:48 ` Maxime Coquelin 2021-02-03 11:52 ` Jiang, Cheng1 2021-02-03 14:29 ` [dpdk-dev] [PATCH v2] " Cheng Jiang 1 sibling, 1 reply; 7+ messages in thread From: Maxime Coquelin @ 2021-02-03 9:48 UTC (permalink / raw) To: Cheng Jiang, chenbo.xia; +Cc: dev, jiayu.hu, yvonnex.yang, yinan.wang On 1/31/21 1:36 PM, Cheng Jiang wrote: > It makes more sense to add error propagation for rte_ioat_completed_ops. > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > --- > examples/vhost/ioat.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c > index dbad28d43..60b73be93 100644 > --- a/examples/vhost/ioat.c > +++ b/examples/vhost/ioat.c > @@ -22,7 +22,6 @@ struct packet_tracker { > > struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; > > - > int > open_ioat(const char *value) > { > @@ -129,7 +128,7 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id, > struct rte_vhost_async_status *opaque_data, uint16_t count) > { > uint32_t i_desc; > - int dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id; > + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id; This change from uint16_t to int does not seem related to the purpose of this patch. Is it? > struct rte_vhost_iov_iter *src = NULL; > struct rte_vhost_iov_iter *dst = NULL; > unsigned long i_seg; > @@ -182,10 +181,17 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id, > unsigned short mask = MAX_ENQUEUED_SIZE - 1; > unsigned short i; > > - int dev_id = dma_bind[vid].dmas[queue_id * 2 > + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 > + VIRTIO_RXQ].dev_id; > n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump); > - if (n_seg <= 0) > + if (n_seg < 0) { > + RTE_LOG(ERR, > + VHOST_DATA, > + "fail to poll completed buf on IOAT device %u", > + dev_id); > + return 0; > + } > + if (n_seg == 0) > return 0; > > cb_tracker[dev_id].ioat_space += n_seg; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] examples/vhost: add error propagation in ioat ops 2021-02-03 9:48 ` [dpdk-dev] [PATCH] examples/vhost: add error propagation in ioat ops Maxime Coquelin @ 2021-02-03 11:52 ` Jiang, Cheng1 2021-02-03 12:48 ` Maxime Coquelin 0 siblings, 1 reply; 7+ messages in thread From: Jiang, Cheng1 @ 2021-02-03 11:52 UTC (permalink / raw) To: Maxime Coquelin, Xia, Chenbo; +Cc: dev, Hu, Jiayu, Yang, YvonneX, Wang, Yinan Hi, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Wednesday, February 3, 2021 5:49 PM > To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo > <chenbo.xia@intel.com> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Yang, YvonneX > <yvonnex.yang@intel.com>; Wang, Yinan <yinan.wang@intel.com> > Subject: Re: [PATCH] examples/vhost: add error propagation in ioat ops > > > > On 1/31/21 1:36 PM, Cheng Jiang wrote: > > It makes more sense to add error propagation for rte_ioat_completed_ops. > > > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > > --- > > examples/vhost/ioat.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c index > > dbad28d43..60b73be93 100644 > > --- a/examples/vhost/ioat.c > > +++ b/examples/vhost/ioat.c > > @@ -22,7 +22,6 @@ struct packet_tracker { > > > > struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; > > > > - > > int > > open_ioat(const char *value) > > { > > @@ -129,7 +128,7 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id, > > struct rte_vhost_async_status *opaque_data, uint16_t count) > { > > uint32_t i_desc; > > - int dev_id = dma_bind[vid].dmas[queue_id * 2 + > VIRTIO_RXQ].dev_id; > > + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + > > +VIRTIO_RXQ].dev_id; > > This change from uint16_t to int does not seem related to the purpose of this > patch. Is it? Yes, this change has nothing to do with adding the log, I just noticed uint16_t is more appropriate and changed here. I can only add the log if you think change the type of dev_id here is not ok. Thanks. Cheng > > > struct rte_vhost_iov_iter *src = NULL; > > struct rte_vhost_iov_iter *dst = NULL; > > unsigned long i_seg; > > @@ -182,10 +181,17 @@ ioat_check_completed_copies_cb(int vid, > uint16_t queue_id, > > unsigned short mask = MAX_ENQUEUED_SIZE - 1; > > unsigned short i; > > > > - int dev_id = dma_bind[vid].dmas[queue_id * 2 > > + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 > > + VIRTIO_RXQ].dev_id; > > n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump); > > - if (n_seg <= 0) > > + if (n_seg < 0) { > > + RTE_LOG(ERR, > > + VHOST_DATA, > > + "fail to poll completed buf on IOAT > device %u", > > + dev_id); > > + return 0; > > + } > > + if (n_seg == 0) > > return 0; > > > > cb_tracker[dev_id].ioat_space += n_seg; > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] examples/vhost: add error propagation in ioat ops 2021-02-03 11:52 ` Jiang, Cheng1 @ 2021-02-03 12:48 ` Maxime Coquelin 2021-02-03 13:10 ` Jiang, Cheng1 0 siblings, 1 reply; 7+ messages in thread From: Maxime Coquelin @ 2021-02-03 12:48 UTC (permalink / raw) To: Jiang, Cheng1, Xia, Chenbo; +Cc: dev, Hu, Jiayu, Yang, YvonneX, Wang, Yinan On 2/3/21 12:52 PM, Jiang, Cheng1 wrote: > Hi, > >> -----Original Message----- >> From: Maxime Coquelin <maxime.coquelin@redhat.com> >> Sent: Wednesday, February 3, 2021 5:49 PM >> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo >> <chenbo.xia@intel.com> >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Yang, YvonneX >> <yvonnex.yang@intel.com>; Wang, Yinan <yinan.wang@intel.com> >> Subject: Re: [PATCH] examples/vhost: add error propagation in ioat ops >> >> >> >> On 1/31/21 1:36 PM, Cheng Jiang wrote: >>> It makes more sense to add error propagation for rte_ioat_completed_ops. >>> >>> Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> >>> --- >>> examples/vhost/ioat.c | 14 ++++++++++---- >>> 1 file changed, 10 insertions(+), 4 deletions(-) >>> >>> diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c index >>> dbad28d43..60b73be93 100644 >>> --- a/examples/vhost/ioat.c >>> +++ b/examples/vhost/ioat.c >>> @@ -22,7 +22,6 @@ struct packet_tracker { >>> >>> struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; >>> >>> - >>> int >>> open_ioat(const char *value) >>> { >>> @@ -129,7 +128,7 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id, >>> struct rte_vhost_async_status *opaque_data, uint16_t count) >> { >>> uint32_t i_desc; >>> - int dev_id = dma_bind[vid].dmas[queue_id * 2 + >> VIRTIO_RXQ].dev_id; >>> + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + >>> +VIRTIO_RXQ].dev_id; >> >> This change from uint16_t to int does not seem related to the purpose of this >> patch. Is it? > > Yes, this change has nothing to do with adding the log, I just noticed uint16_t is more appropriate and changed here. > I can only add the log if you think change the type of dev_id here is not ok. Better to have two commits since these are two different changes. At least please mention it in the commit message. Thanks, Maxime > Thanks. > Cheng > >> >>> struct rte_vhost_iov_iter *src = NULL; >>> struct rte_vhost_iov_iter *dst = NULL; >>> unsigned long i_seg; >>> @@ -182,10 +181,17 @@ ioat_check_completed_copies_cb(int vid, >> uint16_t queue_id, >>> unsigned short mask = MAX_ENQUEUED_SIZE - 1; >>> unsigned short i; >>> >>> - int dev_id = dma_bind[vid].dmas[queue_id * 2 >>> + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 >>> + VIRTIO_RXQ].dev_id; >>> n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump); >>> - if (n_seg <= 0) >>> + if (n_seg < 0) { >>> + RTE_LOG(ERR, >>> + VHOST_DATA, >>> + "fail to poll completed buf on IOAT >> device %u", >>> + dev_id); >>> + return 0; >>> + } >>> + if (n_seg == 0) >>> return 0; >>> >>> cb_tracker[dev_id].ioat_space += n_seg; >>> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] examples/vhost: add error propagation in ioat ops 2021-02-03 12:48 ` Maxime Coquelin @ 2021-02-03 13:10 ` Jiang, Cheng1 0 siblings, 0 replies; 7+ messages in thread From: Jiang, Cheng1 @ 2021-02-03 13:10 UTC (permalink / raw) To: Maxime Coquelin, Xia, Chenbo; +Cc: dev, Hu, Jiayu, Yang, YvonneX, Wang, Yinan > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Wednesday, February 3, 2021 8:49 PM > To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo > <chenbo.xia@intel.com> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Yang, YvonneX > <yvonnex.yang@intel.com>; Wang, Yinan <yinan.wang@intel.com> > Subject: Re: [PATCH] examples/vhost: add error propagation in ioat ops > > > > On 2/3/21 12:52 PM, Jiang, Cheng1 wrote: > > Hi, > > > >> -----Original Message----- > >> From: Maxime Coquelin <maxime.coquelin@redhat.com> > >> Sent: Wednesday, February 3, 2021 5:49 PM > >> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo > >> <chenbo.xia@intel.com> > >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Yang, YvonneX > >> <yvonnex.yang@intel.com>; Wang, Yinan <yinan.wang@intel.com> > >> Subject: Re: [PATCH] examples/vhost: add error propagation in ioat > >> ops > >> > >> > >> > >> On 1/31/21 1:36 PM, Cheng Jiang wrote: > >>> It makes more sense to add error propagation for > rte_ioat_completed_ops. > >>> > >>> Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > >>> --- > >>> examples/vhost/ioat.c | 14 ++++++++++---- > >>> 1 file changed, 10 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c index > >>> dbad28d43..60b73be93 100644 > >>> --- a/examples/vhost/ioat.c > >>> +++ b/examples/vhost/ioat.c > >>> @@ -22,7 +22,6 @@ struct packet_tracker { > >>> > >>> struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; > >>> > >>> - > >>> int > >>> open_ioat(const char *value) > >>> { > >>> @@ -129,7 +128,7 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id, > >>> struct rte_vhost_async_status *opaque_data, uint16_t count) > >> { > >>> uint32_t i_desc; > >>> - int dev_id = dma_bind[vid].dmas[queue_id * 2 + > >> VIRTIO_RXQ].dev_id; > >>> + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + > >>> +VIRTIO_RXQ].dev_id; > >> > >> This change from uint16_t to int does not seem related to the purpose > >> of this patch. Is it? > > > > Yes, this change has nothing to do with adding the log, I just noticed > uint16_t is more appropriate and changed here. > > I can only add the log if you think change the type of dev_id here is not ok. > > Better to have two commits since these are two different changes. > At least please mention it in the commit message. Sure, I'll mention it in the commit message. Thanks, Cheng > > Thanks, > Maxime > > > > Thanks. > > Cheng > > > >> > >>> struct rte_vhost_iov_iter *src = NULL; > >>> struct rte_vhost_iov_iter *dst = NULL; > >>> unsigned long i_seg; > >>> @@ -182,10 +181,17 @@ ioat_check_completed_copies_cb(int vid, > >> uint16_t queue_id, > >>> unsigned short mask = MAX_ENQUEUED_SIZE - 1; > >>> unsigned short i; > >>> > >>> - int dev_id = dma_bind[vid].dmas[queue_id * 2 > >>> + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 > >>> + VIRTIO_RXQ].dev_id; > >>> n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump); > >>> - if (n_seg <= 0) > >>> + if (n_seg < 0) { > >>> + RTE_LOG(ERR, > >>> + VHOST_DATA, > >>> + "fail to poll completed buf on IOAT > >> device %u", > >>> + dev_id); > >>> + return 0; > >>> + } > >>> + if (n_seg == 0) > >>> return 0; > >>> > >>> cb_tracker[dev_id].ioat_space += n_seg; > >>> > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v2] examples/vhost: add error propagation in ioat ops [not found] <20210131123628.47883-1-Cheng1.jiang@intel.com> 2021-02-03 9:48 ` [dpdk-dev] [PATCH] examples/vhost: add error propagation in ioat ops Maxime Coquelin @ 2021-02-03 14:29 ` Cheng Jiang 2021-02-03 16:43 ` Maxime Coquelin 2021-02-03 17:19 ` Maxime Coquelin 1 sibling, 2 replies; 7+ messages in thread From: Cheng Jiang @ 2021-02-03 14:29 UTC (permalink / raw) To: maxime.coquelin, chenbo.xia; +Cc: dev, jiayu.hu, yvonnex.yang, Cheng Jiang It makes more sense to add error propagation for rte_ioat_completed_ops. And change the type of dev_id from int to uint16_t. Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> --- v2: mentioned dev_id type changes in git log examples/vhost/ioat.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c index dbad28d43..60b73be93 100644 --- a/examples/vhost/ioat.c +++ b/examples/vhost/ioat.c @@ -22,7 +22,6 @@ struct packet_tracker { struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; - int open_ioat(const char *value) { @@ -129,7 +128,7 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id, struct rte_vhost_async_status *opaque_data, uint16_t count) { uint32_t i_desc; - int dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id; + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id; struct rte_vhost_iov_iter *src = NULL; struct rte_vhost_iov_iter *dst = NULL; unsigned long i_seg; @@ -182,10 +181,17 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id, unsigned short mask = MAX_ENQUEUED_SIZE - 1; unsigned short i; - int dev_id = dma_bind[vid].dmas[queue_id * 2 + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id; n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump); - if (n_seg <= 0) + if (n_seg < 0) { + RTE_LOG(ERR, + VHOST_DATA, + "fail to poll completed buf on IOAT device %u", + dev_id); + return 0; + } + if (n_seg == 0) return 0; cb_tracker[dev_id].ioat_space += n_seg; -- 2.29.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] examples/vhost: add error propagation in ioat ops 2021-02-03 14:29 ` [dpdk-dev] [PATCH v2] " Cheng Jiang @ 2021-02-03 16:43 ` Maxime Coquelin 2021-02-03 17:19 ` Maxime Coquelin 1 sibling, 0 replies; 7+ messages in thread From: Maxime Coquelin @ 2021-02-03 16:43 UTC (permalink / raw) To: Cheng Jiang, chenbo.xia; +Cc: dev, jiayu.hu, yvonnex.yang On 2/3/21 3:29 PM, Cheng Jiang wrote: > It makes more sense to add error propagation for rte_ioat_completed_ops. > And change the type of dev_id from int to uint16_t. I would reword it to: " This patch adds add error propagation for rte_ioat_completed_ops call, and also changes dev_id type from int to uint16_t. " Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> > --- > v2: mentioned dev_id type changes in git log > > examples/vhost/ioat.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c > index dbad28d43..60b73be93 100644 > --- a/examples/vhost/ioat.c > +++ b/examples/vhost/ioat.c > @@ -22,7 +22,6 @@ struct packet_tracker { > > struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; > > - > int > open_ioat(const char *value) > { > @@ -129,7 +128,7 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id, > struct rte_vhost_async_status *opaque_data, uint16_t count) > { > uint32_t i_desc; > - int dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id; > + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id; > struct rte_vhost_iov_iter *src = NULL; > struct rte_vhost_iov_iter *dst = NULL; > unsigned long i_seg; > @@ -182,10 +181,17 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id, > unsigned short mask = MAX_ENQUEUED_SIZE - 1; > unsigned short i; > > - int dev_id = dma_bind[vid].dmas[queue_id * 2 > + uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 > + VIRTIO_RXQ].dev_id; > n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump); > - if (n_seg <= 0) > + if (n_seg < 0) { > + RTE_LOG(ERR, > + VHOST_DATA, > + "fail to poll completed buf on IOAT device %u", > + dev_id); > + return 0; > + } > + if (n_seg == 0) > return 0; > > cb_tracker[dev_id].ioat_space += n_seg; > -- > 2.29.2 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] examples/vhost: add error propagation in ioat ops 2021-02-03 14:29 ` [dpdk-dev] [PATCH v2] " Cheng Jiang 2021-02-03 16:43 ` Maxime Coquelin @ 2021-02-03 17:19 ` Maxime Coquelin 1 sibling, 0 replies; 7+ messages in thread From: Maxime Coquelin @ 2021-02-03 17:19 UTC (permalink / raw) To: Cheng Jiang, chenbo.xia; +Cc: dev, jiayu.hu, yvonnex.yang On 2/3/21 3:29 PM, Cheng Jiang wrote: > |It makes more sense to add error propagation for > rte_ioat_completed_ops. And change the type of dev_id from int to > uint16_t. Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com> --- v2: > mentioned dev_id type changes in git log examples/vhost/ioat.c | 14 > ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)| Applied to dpdk-next-virtio/main with commit message fixed. Thanks, Maxime ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-03 17:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210131123628.47883-1-Cheng1.jiang@intel.com>
2021-02-03 9:48 ` [dpdk-dev] [PATCH] examples/vhost: add error propagation in ioat ops Maxime Coquelin
2021-02-03 11:52 ` Jiang, Cheng1
2021-02-03 12:48 ` Maxime Coquelin
2021-02-03 13:10 ` Jiang, Cheng1
2021-02-03 14:29 ` [dpdk-dev] [PATCH v2] " Cheng Jiang
2021-02-03 16:43 ` Maxime Coquelin
2021-02-03 17:19 ` Maxime Coquelin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.