[PATCH] vhost: fix unnecessary dirty page logging

[PATCH] vhost: fix unnecessary dirty page logging

[xuan.ding]

From: Xuan Ding <xuan.ding@intel.com>

The dirty page logging is only required in vhost enqueue direction for
live migration. This patch removes the unnecessary dirty page logging
in vhost dequeue direction. Otherwise, it will result in a performance
drop. Some if-else judgements are also optimized to improve performance.

Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf refactoring")
Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
---
 lib/vhost/virtio_net.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index e842c35fef..12b7fbe7f9 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1113,27 +1113,27 @@ sync_fill_seg(struct virtio_net *dev, struct vhost_virtqueue *vq,
 			rte_memcpy((void *)((uintptr_t)(buf_addr)),
 				rte_pktmbuf_mtod_offset(m, void *, mbuf_offset),
 				cpy_len);
+			vhost_log_cache_write_iova(dev, vq, buf_iova, cpy_len);
+			PRINT_PACKET(dev, (uintptr_t)(buf_addr), cpy_len, 0);
 		} else {
 			rte_memcpy(rte_pktmbuf_mtod_offset(m, void *, mbuf_offset),
 				(void *)((uintptr_t)(buf_addr)),
 				cpy_len);
 		}
-		vhost_log_cache_write_iova(dev, vq, buf_iova, cpy_len);
-		PRINT_PACKET(dev, (uintptr_t)(buf_addr), cpy_len, 0);
 	} else {
 		if (to_desc) {
 			batch_copy[vq->batch_copy_nb_elems].dst =
 				(void *)((uintptr_t)(buf_addr));
 			batch_copy[vq->batch_copy_nb_elems].src =
 				rte_pktmbuf_mtod_offset(m, void *, mbuf_offset);
+			batch_copy[vq->batch_copy_nb_elems].log_addr = buf_iova;
+			batch_copy[vq->batch_copy_nb_elems].len = cpy_len;
 		} else {
 			batch_copy[vq->batch_copy_nb_elems].dst =
 				rte_pktmbuf_mtod_offset(m, void *, mbuf_offset);
 			batch_copy[vq->batch_copy_nb_elems].src =
 				(void *)((uintptr_t)(buf_addr));
 		}
-		batch_copy[vq->batch_copy_nb_elems].log_addr = buf_iova;
-		batch_copy[vq->batch_copy_nb_elems].len = cpy_len;
 		vq->batch_copy_nb_elems++;
 	}
 }
@@ -2739,18 +2739,14 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
 			if (async_fill_seg(dev, vq, cur, mbuf_offset,
 					   buf_iova + buf_offset, cpy_len, false) < 0)
 				goto error;
+		} else if (likely(hdr && cur == m)) {
+			rte_memcpy(rte_pktmbuf_mtod_offset(cur, void *, mbuf_offset),
+				(void *)((uintptr_t)(buf_addr + buf_offset)),
+				cpy_len);
 		} else {
-			if (hdr && cur == m) {
-				rte_memcpy(rte_pktmbuf_mtod_offset(cur, void *, mbuf_offset),
-					(void *)((uintptr_t)(buf_addr + buf_offset)),
-					cpy_len);
-				vhost_log_cache_write_iova(dev, vq, buf_iova + buf_offset, cpy_len);
-				PRINT_PACKET(dev, (uintptr_t)(buf_addr + buf_offset), cpy_len, 0);
-			} else {
-				sync_fill_seg(dev, vq, cur, mbuf_offset,
-					buf_addr + buf_offset,
-					buf_iova + buf_offset, cpy_len, false);
-			}
+			sync_fill_seg(dev, vq, cur, mbuf_offset,
+				      buf_addr + buf_offset,
+				      buf_iova + buf_offset, cpy_len, false);
 		}
 
 		mbuf_avail  -= cpy_len;
@@ -2804,9 +2800,8 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
 		async_iter_finalize(async);
 		if (hdr)
 			pkts_info[slot_idx].nethdr = *hdr;
-	} else {
-		if (hdr)
-			vhost_dequeue_offload(dev, hdr, m, legacy_ol_flags);
+	} else if (hdr) {
+		vhost_dequeue_offload(dev, hdr, m, legacy_ol_flags);
 	}
 
 	return 0;
-- 
2.17.1

RE: [PATCH] vhost: fix unnecessary dirty page logging

[Hu, Jiayu]

> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Thursday, July 7, 2022 2:55 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> Jiang, Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> Subject: [PATCH] vhost: fix unnecessary dirty page logging
> 
> From: Xuan Ding <xuan.ding@intel.com>
> 
> The dirty page logging is only required in vhost enqueue direction for live
> migration. This patch removes the unnecessary dirty page logging in vhost
> dequeue direction. Otherwise, it will result in a performance drop. Some if-
> else judgements are also optimized to improve performance.
> 
> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
> refactoring")
> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>  lib/vhost/virtio_net.c | 31 +++++++++++++------------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index
> e842c35fef..12b7fbe7f9 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -1113,27 +1113,27 @@ sync_fill_seg(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  			rte_memcpy((void *)((uintptr_t)(buf_addr)),
>  				rte_pktmbuf_mtod_offset(m, void *,
> mbuf_offset),
>  				cpy_len);
> +			vhost_log_cache_write_iova(dev, vq, buf_iova,
> cpy_len);
> +			PRINT_PACKET(dev, (uintptr_t)(buf_addr), cpy_len,
> 0);
>  		} else {
>  			rte_memcpy(rte_pktmbuf_mtod_offset(m, void *,
> mbuf_offset),
>  				(void *)((uintptr_t)(buf_addr)),
>  				cpy_len);
>  		}
> -		vhost_log_cache_write_iova(dev, vq, buf_iova, cpy_len);
> -		PRINT_PACKET(dev, (uintptr_t)(buf_addr), cpy_len, 0);
>  	} else {
>  		if (to_desc) {
>  			batch_copy[vq->batch_copy_nb_elems].dst =
>  				(void *)((uintptr_t)(buf_addr));
>  			batch_copy[vq->batch_copy_nb_elems].src =
>  				rte_pktmbuf_mtod_offset(m, void *,
> mbuf_offset);
> +			batch_copy[vq->batch_copy_nb_elems].log_addr =
> buf_iova;
> +			batch_copy[vq->batch_copy_nb_elems].len =
> cpy_len;
>  		} else {
>  			batch_copy[vq->batch_copy_nb_elems].dst =
>  				rte_pktmbuf_mtod_offset(m, void *,
> mbuf_offset);
>  			batch_copy[vq->batch_copy_nb_elems].src =
>  				(void *)((uintptr_t)(buf_addr));
>  		}
> -		batch_copy[vq->batch_copy_nb_elems].log_addr =
> buf_iova;
> -		batch_copy[vq->batch_copy_nb_elems].len = cpy_len;
>  		vq->batch_copy_nb_elems++;
>  	}
>  }
> @@ -2739,18 +2739,14 @@ desc_to_mbuf(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  			if (async_fill_seg(dev, vq, cur, mbuf_offset,
>  					   buf_iova + buf_offset, cpy_len,
> false) < 0)
>  				goto error;
> +		} else if (likely(hdr && cur == m)) {
> +			rte_memcpy(rte_pktmbuf_mtod_offset(cur, void *,
> mbuf_offset),
> +				(void *)((uintptr_t)(buf_addr + buf_offset)),
> +				cpy_len);
>  		} else {
> -			if (hdr && cur == m) {
> -				rte_memcpy(rte_pktmbuf_mtod_offset(cur,
> void *, mbuf_offset),
> -					(void *)((uintptr_t)(buf_addr +
> buf_offset)),
> -					cpy_len);
> -				vhost_log_cache_write_iova(dev, vq,
> buf_iova + buf_offset, cpy_len);
> -				PRINT_PACKET(dev, (uintptr_t)(buf_addr +
> buf_offset), cpy_len, 0);
> -			} else {
> -				sync_fill_seg(dev, vq, cur, mbuf_offset,
> -					buf_addr + buf_offset,
> -					buf_iova + buf_offset, cpy_len, false);
> -			}
> +			sync_fill_seg(dev, vq, cur, mbuf_offset,
> +				      buf_addr + buf_offset,
> +				      buf_iova + buf_offset, cpy_len, false);
>  		}
> 
>  		mbuf_avail  -= cpy_len;
> @@ -2804,9 +2800,8 @@ desc_to_mbuf(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  		async_iter_finalize(async);
>  		if (hdr)
>  			pkts_info[slot_idx].nethdr = *hdr;
> -	} else {
> -		if (hdr)
> -			vhost_dequeue_offload(dev, hdr, m,
> legacy_ol_flags);
> +	} else if (hdr) {
> +		vhost_dequeue_offload(dev, hdr, m, legacy_ol_flags);
>  	}
> 
>  	return 0;
> --
> 2.17.1

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

Thanks,
Jiayu

RE: [PATCH] vhost: fix unnecessary dirty page logging

[Xia, Chenbo]

> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Thursday, July 7, 2022 2:55 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>; Jiang,
> Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> Subject: [PATCH] vhost: fix unnecessary dirty page logging
> 
> From: Xuan Ding <xuan.ding@intel.com>
> 
> The dirty page logging is only required in vhost enqueue direction for
> live migration. This patch removes the unnecessary dirty page logging
> in vhost dequeue direction. Otherwise, it will result in a performance
> drop. Some if-else judgements are also optimized to improve performance.
> 
> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
> refactoring")
> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

Although it's late in release, we can consider to merge this as I see it impacts
the performance by 5%:
http://inbox.dpdk.org/dev/BYAPR11MB2711F13CDA2B0A4535A6591EFE839@BYAPR11MB2711.namprd11.prod.outlook.com/T/#t

But also, it will be good to know the performance issue is solved by sharing the
test results.

Thanks,
Chenbo

Re: [PATCH] vhost: fix unnecessary dirty page logging

[Maxime Coquelin]

On 7/7/22 11:51, Xia, Chenbo wrote:
>> -----Original Message-----
>> From: Ding, Xuan <xuan.ding@intel.com>
>> Sent: Thursday, July 7, 2022 2:55 PM
>> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
>> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
>> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>; Jiang,
>> Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
>> Subject: [PATCH] vhost: fix unnecessary dirty page logging
>>
>> From: Xuan Ding <xuan.ding@intel.com>
>>
>> The dirty page logging is only required in vhost enqueue direction for
>> live migration. This patch removes the unnecessary dirty page logging
>> in vhost dequeue direction. Otherwise, it will result in a performance
>> drop. Some if-else judgements are also optimized to improve performance.
>>
>> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
>> refactoring")
>> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
>>
>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
>> ---
> 
> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
> 
> Although it's late in release, we can consider to merge this as I see it impacts
> the performance by 5%:
> http://inbox.dpdk.org/dev/BYAPR11MB2711F13CDA2B0A4535A6591EFE839@BYAPR11MB2711.namprd11.prod.outlook.com/T/#t

Yes, I raised we need it in -rc4 at today's Release status meeting.
I'll review it today.

> But also, it will be good to know the performance issue is solved by sharing the
> test results.

Yes, Intel performance results would be appreciated.

> Thanks,
> Chenbo
> 

Thanks,
Maxime

RE: [PATCH] vhost: fix unnecessary dirty page logging

[Ding, Xuan]

Hi,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: 2022年7月7日 19:31
> To: Xia, Chenbo <chenbo.xia@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>; Jiang,
> Cheng1 <cheng1.jiang@intel.com>
> Subject: Re: [PATCH] vhost: fix unnecessary dirty page logging
> 
> 
> 
> On 7/7/22 11:51, Xia, Chenbo wrote:
> >> -----Original Message-----
> >> From: Ding, Xuan <xuan.ding@intel.com>
> >> Sent: Thursday, July 7, 2022 2:55 PM
> >> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> >> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> >> Jiang,
> >> Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> >> Subject: [PATCH] vhost: fix unnecessary dirty page logging
> >>
> >> From: Xuan Ding <xuan.ding@intel.com>
> >>
> >> The dirty page logging is only required in vhost enqueue direction
> >> for live migration. This patch removes the unnecessary dirty page
> >> logging in vhost dequeue direction. Otherwise, it will result in a
> >> performance drop. Some if-else judgements are also optimized to improve
> performance.
> >>
> >> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
> >> refactoring")
> >> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> >>
> >> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> >> ---
> >
> > Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
> >
> > Although it's late in release, we can consider to merge this as I see
> > it impacts the performance by 5%:
> >
> http://inbox.dpdk.org/dev/BYAPR11MB2711F13CDA2B0A4535A6591EFE839@B
> YAPR
> > 11MB2711.namprd11.prod.outlook.com/T/#t
> 
> Yes, I raised we need it in -rc4 at today's Release status meeting.
> I'll review it today.
> 
> > But also, it will be good to know the performance issue is solved by
> > sharing the test results.
> 
> Yes, Intel performance results would be appreciated. 

This fix patch is for the issue reported at 22.07-rc3. The refactoring patch brings a 3%~5% perf drop in vhost sync path.
With fix patch, the perf drop introduced by refactoring is solved.

However, the testing result shows there still exists ~5% packed ring perf drop compared with 22.03.
We find the improving checksum offload patch series in 22.07 may contribute to the packed ring perf drop.
Because we always do checksum checks in PMD.

Could you help to double check this patch series, is it as expected?
Your assistance is really appreciated.

Regards,
Xuan

> 
> > Thanks,
> > Chenbo
> >
> 
> Thanks,
> Maxime

RE: [PATCH] vhost: fix unnecessary dirty page logging

[He, Xingguang]

> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Friday, July 8, 2022 3:04 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> Jiang, Cheng1 <cheng1.jiang@intel.com>
> Subject: RE: [PATCH] vhost: fix unnecessary dirty page logging
> 
> Hi,
> 
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: 2022年7月7日 19:31
> > To: Xia, Chenbo <chenbo.xia@intel.com>; Ding, Xuan
> > <xuan.ding@intel.com>
> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> > <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> > Jiang,
> > Cheng1 <cheng1.jiang@intel.com>
> > Subject: Re: [PATCH] vhost: fix unnecessary dirty page logging
> >
> >
> >
> > On 7/7/22 11:51, Xia, Chenbo wrote:
> > >> -----Original Message-----
> > >> From: Ding, Xuan <xuan.ding@intel.com>
> > >> Sent: Thursday, July 7, 2022 2:55 PM
> > >> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> > >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> > >> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> > >> Jiang,
> > >> Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> > >> Subject: [PATCH] vhost: fix unnecessary dirty page logging
> > >>
> > >> From: Xuan Ding <xuan.ding@intel.com>
> > >>
> > >> The dirty page logging is only required in vhost enqueue direction
> > >> for live migration. This patch removes the unnecessary dirty page
> > >> logging in vhost dequeue direction. Otherwise, it will result in a
> > >> performance drop. Some if-else judgements are also optimized to
> > >> improve
> > performance.
> > >>
> > >> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
> > >> refactoring")
> > >> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> > >>
> > >> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > >> ---
> > >
> > > Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
> > >
> > > Although it's late in release, we can consider to merge this as I
> > > see it impacts the performance by 5%:
> > >
> >
> http://inbox.dpdk.org/dev/BYAPR11MB2711F13CDA2B0A4535A6591EFE839
> @B
> > YAPR
> > > 11MB2711.namprd11.prod.outlook.com/T/#t
> >
> > Yes, I raised we need it in -rc4 at today's Release status meeting.
> > I'll review it today.
> >
> > > But also, it will be good to know the performance issue is solved by
> > > sharing the test results.
> >
> > Yes, Intel performance results would be appreciated.
> 
> This fix patch is for the issue reported at 22.07-rc3. The refactoring patch
> brings a 3%~5% perf drop in vhost sync path.
> With fix patch, the perf drop introduced by refactoring is solved.

The fix is for a critical issue, I tested dpdk22.07-rc3 with the patch, 
the performance drop introduced by refactoring have been fixed 
and no other regressions were seen as a result of the patch, 
but the packed ring still exist drop around 5%.  

Tested-by: Xingguang He<xingguang.he@intel.com>

> 
> However, the testing result shows there still exists ~5% packed ring perf drop
> compared with 22.03.
> We find the improving checksum offload patch series in 22.07 may contribute
> to the packed ring perf drop.
> Because we always do checksum checks in PMD.

I tested the performance of Vhost before and after checksum offload patch series, 
there exist  ~5% performance drop.

> 
> Could you help to double check this patch series, is it as expected?
> Your assistance is really appreciated.
> 
> Regards,
> Xuan
> 
> >
> > > Thanks,
> > > Chenbo
> > >
> >
> > Thanks,
> > Maxime

RE: [PATCH] vhost: fix unnecessary dirty page logging

[He, Xingguang]

> -----Original Message-----
> From: He, Xingguang <xingguang.he@intel.com>
> Sent: Friday, July 8, 2022 3:53 PM
> To: Ding, Xuan <xuan.ding@intel.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Yang, YvonneX
> <yvonnex.yang@intel.com>; Jiang, Cheng1 <cheng1.jiang@intel.com>
> Subject: RE: [PATCH] vhost: fix unnecessary dirty page logging
> 
> > -----Original Message-----
> > From: Ding, Xuan <xuan.ding@intel.com>
> > Sent: Friday, July 8, 2022 3:04 PM
> > To: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> > <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> > <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> > Jiang, Cheng1 <cheng1.jiang@intel.com>
> > Subject: RE: [PATCH] vhost: fix unnecessary dirty page logging
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > > Sent: 2022年7月7日 19:31
> > > To: Xia, Chenbo <chenbo.xia@intel.com>; Ding, Xuan
> > > <xuan.ding@intel.com>
> > > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> > > <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> > > Jiang,
> > > Cheng1 <cheng1.jiang@intel.com>
> > > Subject: Re: [PATCH] vhost: fix unnecessary dirty page logging
> > >
> > >
> > >
> > > On 7/7/22 11:51, Xia, Chenbo wrote:
> > > >> -----Original Message-----
> > > >> From: Ding, Xuan <xuan.ding@intel.com>
> > > >> Sent: Thursday, July 7, 2022 2:55 PM
> > > >> To: maxime.coquelin@redhat.com; Xia, Chenbo
> > > >> <chenbo.xia@intel.com>
> > > >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> > > >> <xingguang.he@intel.com>; Yang, YvonneX
> <yvonnex.yang@intel.com>;
> > > >> Jiang,
> > > >> Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> > > >> Subject: [PATCH] vhost: fix unnecessary dirty page logging
> > > >>
> > > >> From: Xuan Ding <xuan.ding@intel.com>
> > > >>
> > > >> The dirty page logging is only required in vhost enqueue
> > > >> direction for live migration. This patch removes the unnecessary
> > > >> dirty page logging in vhost dequeue direction. Otherwise, it will
> > > >> result in a performance drop. Some if-else judgements are also
> > > >> optimized to improve
> > > performance.
> > > >>
> > > >> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
> > > >> refactoring")
> > > >> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> > > >>
> > > >> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > > >> ---
> > > >
> > > > Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
> > > >
> > > > Although it's late in release, we can consider to merge this as I
> > > > see it impacts the performance by 5%:
> > > >
> > >
> >
> http://inbox.dpdk.org/dev/BYAPR11MB2711F13CDA2B0A4535A6591EFE839
> > @B
> > > YAPR
> > > > 11MB2711.namprd11.prod.outlook.com/T/#t
> > >
> > > Yes, I raised we need it in -rc4 at today's Release status meeting.
> > > I'll review it today.
> > >
> > > > But also, it will be good to know the performance issue is solved
> > > > by sharing the test results.
> > >
> > > Yes, Intel performance results would be appreciated.
> >
> > This fix patch is for the issue reported at 22.07-rc3. The refactoring
> > patch brings a 3%~5% perf drop in vhost sync path.
> > With fix patch, the perf drop introduced by refactoring is solved.
> 
> The fix is for a critical issue, I tested dpdk22.07-rc3 with the patch, the
> performance drop introduced by refactoring have been fixed and no other
> regressions were seen as a result of the patch, but the packed ring still exist
> drop around 5%.
> 
> Tested-by: Xingguang He<xingguang.he@intel.com>
> 
> >
> > However, the testing result shows there still exists ~5% packed ring
> > perf drop compared with 22.03.
> > We find the improving checksum offload patch series in 22.07 may
> > contribute to the packed ring perf drop.
> > Because we always do checksum checks in PMD.
> 
> I tested the performance of Vhost before and after checksum offload patch
> series, there exist  ~5% performance drop.

Sorry, the drop of checksum offload patch series is ~3%. 
The drop between DPDK22.03 and DPDK22.07-rc3 is ~5%.  

> 
> >
> > Could you help to double check this patch series, is it as expected?
> > Your assistance is really appreciated.
> >
> > Regards,
> > Xuan
> >
> > >
> > > > Thanks,
> > > > Chenbo
> > > >
> > >
> > > Thanks,
> > > Maxime

Re: [PATCH] vhost: fix unnecessary dirty page logging

[Maxime Coquelin]

Hi,

On 7/8/22 09:04, Ding, Xuan wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: 2022年7月7日 19:31
>> To: Xia, Chenbo <chenbo.xia@intel.com>; Ding, Xuan <xuan.ding@intel.com>
>> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
>> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>; Jiang,
>> Cheng1 <cheng1.jiang@intel.com>
>> Subject: Re: [PATCH] vhost: fix unnecessary dirty page logging
>>
>>
>>
>> On 7/7/22 11:51, Xia, Chenbo wrote:
>>>> -----Original Message-----
>>>> From: Ding, Xuan <xuan.ding@intel.com>
>>>> Sent: Thursday, July 7, 2022 2:55 PM
>>>> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
>>>> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
>>>> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
>>>> Jiang,
>>>> Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
>>>> Subject: [PATCH] vhost: fix unnecessary dirty page logging
>>>>
>>>> From: Xuan Ding <xuan.ding@intel.com>
>>>>
>>>> The dirty page logging is only required in vhost enqueue direction
>>>> for live migration. This patch removes the unnecessary dirty page
>>>> logging in vhost dequeue direction. Otherwise, it will result in a
>>>> performance drop. Some if-else judgements are also optimized to improve
>> performance.
>>>>
>>>> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
>>>> refactoring")
>>>> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
>>>>
>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
>>>> ---
>>>
>>> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
>>>
>>> Although it's late in release, we can consider to merge this as I see
>>> it impacts the performance by 5%:
>>>
>> http://inbox.dpdk.org/dev/BYAPR11MB2711F13CDA2B0A4535A6591EFE839@B
>> YAPR
>>> 11MB2711.namprd11.prod.outlook.com/T/#t
>>
>> Yes, I raised we need it in -rc4 at today's Release status meeting.
>> I'll review it today.
>>
>>> But also, it will be good to know the performance issue is solved by
>>> sharing the test results.
>>
>> Yes, Intel performance results would be appreciated.
> 
> This fix patch is for the issue reported at 22.07-rc3. The refactoring patch brings a 3%~5% perf drop in vhost sync path.
> With fix patch, the perf drop introduced by refactoring is solved.

Good.

> However, the testing result shows there still exists ~5% packed ring perf drop compared with 22.03.
> We find the improving checksum offload patch series in 22.07 may contribute to the packed ring perf drop.
> Because we always do checksum checks in PMD.
> 
> Could you help to double check this patch series, is it as expected?
> Your assistance is really appreciated.

I will look again, but if your analysis is right, there's not much we
can do. The series is filling gaps in the checksum offload support.

I will not have time to work on it for v22.07, we're too late in the
cycle. What surprises me is that only packed ring is impacted.

Regards,
Maxime

> 
> Regards,
> Xuan
> 
>>
>>> Thanks,
>>> Chenbo
>>>
>>
>> Thanks,
>> Maxime
> 

RE: [PATCH] vhost: fix unnecessary dirty page logging

[Ding, Xuan]

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: 2022年7月8日 15:58
> To: Ding, Xuan <xuan.ding@intel.com>; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>; Jiang,
> Cheng1 <cheng1.jiang@intel.com>
> Subject: Re: [PATCH] vhost: fix unnecessary dirty page logging
> 
> Hi,
> 
> On 7/8/22 09:04, Ding, Xuan wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: 2022年7月7日 19:31
> >> To: Xia, Chenbo <chenbo.xia@intel.com>; Ding, Xuan
> >> <xuan.ding@intel.com>
> >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> >> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> >> Jiang,
> >> Cheng1 <cheng1.jiang@intel.com>
> >> Subject: Re: [PATCH] vhost: fix unnecessary dirty page logging
> >>
> >>
> >>
> >> On 7/7/22 11:51, Xia, Chenbo wrote:
> >>>> -----Original Message-----
> >>>> From: Ding, Xuan <xuan.ding@intel.com>
> >>>> Sent: Thursday, July 7, 2022 2:55 PM
> >>>> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> >>>> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> >>>> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> >>>> Jiang,
> >>>> Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> >>>> Subject: [PATCH] vhost: fix unnecessary dirty page logging
> >>>>
> >>>> From: Xuan Ding <xuan.ding@intel.com>
> >>>>
> >>>> The dirty page logging is only required in vhost enqueue direction
> >>>> for live migration. This patch removes the unnecessary dirty page
> >>>> logging in vhost dequeue direction. Otherwise, it will result in a
> >>>> performance drop. Some if-else judgements are also optimized to
> >>>> improve
> >> performance.
> >>>>
> >>>> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
> >>>> refactoring")
> >>>> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> >>>>
> >>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> >>>> ---
> >>>
> >>> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
> >>>
> >>> Although it's late in release, we can consider to merge this as I
> >>> see it impacts the performance by 5%:
> >>>
> >>
> http://inbox.dpdk.org/dev/BYAPR11MB2711F13CDA2B0A4535A6591EFE839@B
> >> YAPR
> >>> 11MB2711.namprd11.prod.outlook.com/T/#t
> >>
> >> Yes, I raised we need it in -rc4 at today's Release status meeting.
> >> I'll review it today.
> >>
> >>> But also, it will be good to know the performance issue is solved by
> >>> sharing the test results.
> >>
> >> Yes, Intel performance results would be appreciated.
> >
> > This fix patch is for the issue reported at 22.07-rc3. The refactoring patch
> brings a 3%~5% perf drop in vhost sync path.
> > With fix patch, the perf drop introduced by refactoring is solved.
> 
> Good.
> 
> > However, the testing result shows there still exists ~5% packed ring perf drop
> compared with 22.03.
> > We find the improving checksum offload patch series in 22.07 may contribute
> to the packed ring perf drop.
> > Because we always do checksum checks in PMD.
> >
> > Could you help to double check this patch series, is it as expected?
> > Your assistance is really appreciated.
> 
> I will look again, but if your analysis is right, there's not much we can do. The
> series is filling gaps in the checksum offload support.
 
Yes, I think so.

> 
> I will not have time to work on it for v22.07, we're too late in the cycle. What
> surprises me is that only packed ring is impacted.
 
From the testing results, the packed ring is more sensitive to the patch series,
while split ring shows a much lower drop in performance.

Thanks for your support!

Regards,
Xuan

> 
> Regards,
> Maxime
> 
> >
> > Regards,
> > Xuan
> >
> >>
> >>> Thanks,
> >>> Chenbo
> >>>
> >>
> >> Thanks,
> >> Maxime
> >

RE: [PATCH] vhost: fix unnecessary dirty page logging

[He, Xingguang]

Hi,

> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Friday, July 8, 2022 4:09 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> Jiang, Cheng1 <cheng1.jiang@intel.com>
> Subject: RE: [PATCH] vhost: fix unnecessary dirty page logging
> 
> 
> 
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: 2022年7月8日 15:58
> > To: Ding, Xuan <xuan.ding@intel.com>; Xia, Chenbo
> > <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> > <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> > Jiang,
> > Cheng1 <cheng1.jiang@intel.com>
> > Subject: Re: [PATCH] vhost: fix unnecessary dirty page logging
> >
> > Hi,
> >
> > On 7/8/22 09:04, Ding, Xuan wrote:
> > > Hi,
> > >
> > >> -----Original Message-----
> > >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > >> Sent: 2022年7月7日 19:31
> > >> To: Xia, Chenbo <chenbo.xia@intel.com>; Ding, Xuan
> > >> <xuan.ding@intel.com>
> > >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> > >> <xingguang.he@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> > >> Jiang,
> > >> Cheng1 <cheng1.jiang@intel.com>
> > >> Subject: Re: [PATCH] vhost: fix unnecessary dirty page logging
> > >>
> > >>
> > >>
> > >> On 7/7/22 11:51, Xia, Chenbo wrote:
> > >>>> -----Original Message-----
> > >>>> From: Ding, Xuan <xuan.ding@intel.com>
> > >>>> Sent: Thursday, July 7, 2022 2:55 PM
> > >>>> To: maxime.coquelin@redhat.com; Xia, Chenbo
> > >>>> <chenbo.xia@intel.com>
> > >>>> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; He, Xingguang
> > >>>> <xingguang.he@intel.com>; Yang, YvonneX
> <yvonnex.yang@intel.com>;
> > >>>> Jiang,
> > >>>> Cheng1 <cheng1.jiang@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> > >>>> Subject: [PATCH] vhost: fix unnecessary dirty page logging
> > >>>>
> > >>>> From: Xuan Ding <xuan.ding@intel.com>
> > >>>>
> > >>>> The dirty page logging is only required in vhost enqueue
> > >>>> direction for live migration. This patch removes the unnecessary
> > >>>> dirty page logging in vhost dequeue direction. Otherwise, it will
> > >>>> result in a performance drop. Some if-else judgements are also
> > >>>> optimized to improve
> > >> performance.
> > >>>>
> > >>>> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf
> > >>>> refactoring")
> > >>>> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> > >>>>
> > >>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > >>>> ---
> > >>>
> > >>> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
> > >>>
> > >>> Although it's late in release, we can consider to merge this as I
> > >>> see it impacts the performance by 5%:
> > >>>
> > >>
> >
> http://inbox.dpdk.org/dev/BYAPR11MB2711F13CDA2B0A4535A6591EFE839
> @B
> > >> YAPR
> > >>> 11MB2711.namprd11.prod.outlook.com/T/#t
> > >>
> > >> Yes, I raised we need it in -rc4 at today's Release status meeting.
> > >> I'll review it today.
> > >>
> > >>> But also, it will be good to know the performance issue is solved
> > >>> by sharing the test results.
> > >>
> > >> Yes, Intel performance results would be appreciated.
> > >
> > > This fix patch is for the issue reported at 22.07-rc3. The
> > > refactoring patch
> > brings a 3%~5% perf drop in vhost sync path.
> > > With fix patch, the perf drop introduced by refactoring is solved.
> >
> > Good.
> >
> > > However, the testing result shows there still exists ~5% packed ring
> > > perf drop
> > compared with 22.03.
> > > We find the improving checksum offload patch series in 22.07 may
> > > contribute
> > to the packed ring perf drop.
> > > Because we always do checksum checks in PMD.
> > >
> > > Could you help to double check this patch series, is it as expected?
> > > Your assistance is really appreciated.
> >
> > I will look again, but if your analysis is right, there's not much we
> > can do. The series is filling gaps in the checksum offload support.

What is the result of your investigation? As dpdk-22.07 will be released soon, 
I'd like to know your advice about this issue. Thanks.

> 
> Yes, I think so.
> 
> >
> > I will not have time to work on it for v22.07, we're too late in the
> > cycle. What surprises me is that only packed ring is impacted.
> 
> From the testing results, the packed ring is more sensitive to the patch series,
> while split ring shows a much lower drop in performance.
> 
> Thanks for your support!
> 

Regards,
Xingguang

> Regards,
> Xuan
> 
> >
> > Regards,
> > Maxime
> >
> > >
> > > Regards,
> > > Xuan
> > >
> > >>
> > >>> Thanks,
> > >>> Chenbo
> > >>>
> > >>
> > >> Thanks,
> > >> Maxime
> > >

Re: [PATCH] vhost: fix unnecessary dirty page logging

[Maxime Coquelin]

On 7/7/22 08:55, xuan.ding@intel.com wrote:
> From: Xuan Ding <xuan.ding@intel.com>
> 
> The dirty page logging is only required in vhost enqueue direction for
> live migration. This patch removes the unnecessary dirty page logging
> in vhost dequeue direction. Otherwise, it will result in a performance
> drop. Some if-else judgements are also optimized to improve performance.
> 
> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf refactoring")
> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>   lib/vhost/virtio_net.c | 31 +++++++++++++------------------
>   1 file changed, 13 insertions(+), 18 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [PATCH] vhost: fix unnecessary dirty page logging

[Maxime Coquelin]

On 7/7/22 08:55, xuan.ding@intel.com wrote:
> From: Xuan Ding <xuan.ding@intel.com>
> 
> The dirty page logging is only required in vhost enqueue direction for
> live migration. This patch removes the unnecessary dirty page logging
> in vhost dequeue direction. Otherwise, it will result in a performance
> drop. Some if-else judgements are also optimized to improve performance.
> 
> Fixes: 6d823bb302c7 ("vhost: prepare sync for descriptor to mbuf refactoring")
> Fixes: b6eee3e83402 ("vhost: fix sync dequeue offload")
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>   lib/vhost/virtio_net.c | 31 +++++++++++++------------------
>   1 file changed, 13 insertions(+), 18 deletions(-)
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime