examples/vhost: fix ioat ring space in callbacks

PATCH In review Cheng Jiang 6 participants 11 messages Mar 17, 2021 – Apr 28, 2021

Share & tools

Cheng Jiang

author

2021-03-17 05:57 UTC #0 lore raw

PATCH 1/1 examples/vhost: fix ioat ring space in callbacks

We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.

Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and check")

Cc [email protected]

Signed-off-by Cheng Jiang <[email protected]>

---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

examples/vhost/ioat.c +1 -1

index 60b73be93..9cb5e0d50 100644
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
 		dma_info->nr++;
 		i++;
 	}

Signature · 1 line

2.29.2

Hu, Jiayu

Reviewed-by on v1 1/1

2021-03-17 06:59 UTC #1 lore raw

Reviewed-by Jiayu Hu <[email protected]>

Quoted 37 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Wednesday, March 17, 2021 1:41 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; Jiang, Cheng1 <[email protected]>; [email protected]
Subject: [PATCH] examples/vhost: fix ioat ring space in callbacks
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and check")
Cc [email protected]
Signed-off-by Cheng Jiang <[email protected]>
---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
examples/vhost/ioat.c +1 -2
index 60b73be93..9cb5e0d50 100644
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
 		dma_info->nr++;
 		i++;
 	}
--
2.29.2

Maxime Coquelin

2021-04-07 07:48 UTC #2 lore raw

In reply to Cheng Jiang

Quoted 18 lines · Cheng Jiang

We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and check")
Cc [email protected]
Signed-off-by Cheng Jiang <[email protected]>
---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
examples/vhost/ioat.c +0 -0
index 60b73be93..9cb5e0d50 100644

examples/vhost/ioat.c open_ioat(const char *value) · line 116

Quoted 5 lines

examples/vhost/ioat.c +1 -1

@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;

That really comforts me in thinking we need a generic abstraction for DMA devices. How is the application developer supposed to know that the DMA driver has such weird limitations?

Can the driver be fixed to have a proper behavior?

Quoted 4 lines

examples/vhost/ioat.c +0 -0
 		dma_info->nr++;
 		i++;
 	}

Hu, Jiayu

2021-04-07 07:54 UTC #3 lore raw

Hi Maxime,

Quoted 42 lines

-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Wednesday, April 7, 2021 3:48 PM
To: Jiang, Cheng1 <[email protected]>; Xia, Chenbo <[email protected]>; Thomas Monjalon <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; [email protected]
Subject: Re: [PATCH] examples/vhost: fix ioat ring space in callbacks
Quoted 6 lines · Cheng Jiang
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and
check")
Quoted 12 lines
Cc [email protected]
Signed-off-by Cheng Jiang <[email protected]>
---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
examples/vhost/ioat.c +0 -0
index 60b73be93..9cb5e0d50 100644
examples/vhost/ioat.c open_ioat(const char *value) · line 116
Quoted 5 lines
examples/vhost/ioat.c +1 -1
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
That really comforts me in thinking we need a generic abstraction for DMA devices. How is the application developer supposed to know that the DMA driver has such weird limitations?

Quoted 1 line

Can the driver be fixed to have a proper behavior?

Here is the patch of providing capacity check API for DMA:
http://patches.dpdk.org/project/dpdk/patch/[email protected]/

Thanks,
Jiayu

Quoted 5 lines

Quoted 5 lines
examples/vhost/ioat.c +0 -0
 		dma_info->nr++;
 		i++;
 	}

Maxime Coquelin

Reviewed-by

2021-04-13 08:51 UTC #4 lore raw

Quoted 44 lines · Jiayu

Hi Maxime,
Quoted 42 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Wednesday, April 7, 2021 3:48 PM
To: Jiang, Cheng1 <[email protected]>; Xia, Chenbo <[email protected]>; Thomas Monjalon <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; [email protected]
Subject: Re: [PATCH] examples/vhost: fix ioat ring space in callbacks
Quoted 6 lines · Cheng Jiang
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and
check")
Quoted 12 lines
Cc [email protected]
Signed-off-by Cheng Jiang <[email protected]>
---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
examples/vhost/ioat.c +0 -0
index 60b73be93..9cb5e0d50 100644
examples/vhost/ioat.c open_ioat(const char *value) · line 116
Quoted 5 lines
examples/vhost/ioat.c +1 -1
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
That really comforts me in thinking we need a generic abstraction for DMA devices. How is the application developer supposed to know that the DMA driver has such weird limitations?

Quoted 4 lines

Quoted 1 line
Can the driver be fixed to have a proper behavior?
Here is the patch of providing capacity check API for DMA:
http://patches.dpdk.org/project/dpdk/patch/[email protected]/

OK, thanks for the pointer.

While this new API is being reviewed, and for LTS, let's pick your patch. As soon as Bruce patch is merged, please send a new patch on top to make use of this API.

Reviewed-by Maxime Coquelin <[email protected]>

Thanks,
Maxime

Quoted 8 lines

Thanks,
Jiayu
Quoted 5 lines
Quoted 5 lines
examples/vhost/ioat.c +0 -0
 		dma_info->nr++;
 		i++;
 	}

Jiang, Cheng1

author

2021-04-13 09:55 UTC #5 lore raw

Hi,

Quoted 61 lines

-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Tuesday, April 13, 2021 4:51 PM
To: Hu, Jiayu <[email protected]>; Jiang, Cheng1 <[email protected]>; Xia, Chenbo <[email protected]>; Thomas Monjalon <[email protected]>
Cc: [email protected]; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; [email protected]
Subject: Re: [PATCH] examples/vhost: fix ioat ring space in callbacks
Quoted 44 lines · Jiayu
Hi Maxime,
Quoted 42 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Wednesday, April 7, 2021 3:48 PM
To: Jiang, Cheng1 <[email protected]>; Xia, Chenbo <[email protected]>; Thomas Monjalon <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; [email protected]
Subject: Re: [PATCH] examples/vhost: fix ioat ring space in callbacks
Quoted 6 lines · Cheng Jiang
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and
check")
Quoted 12 lines
Cc [email protected]
Signed-off-by Cheng Jiang <[email protected]>
---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
examples/vhost/ioat.c index renamed from examples/vhost/ioat.c +0 -0
60b73be93..9cb5e0d50 100644
examples/vhost/ioat.c open_ioat(const char *value) · line 116
Quoted 5 lines
examples/vhost/ioat.c +1 -1
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
That really comforts me in thinking we need a generic abstraction for DMA devices. How is the application developer supposed to know that the DMA driver has such weird limitations?
Quoted 4 lines
Quoted 1 line
Can the driver be fixed to have a proper behavior?
Here is the patch of providing capacity check API for DMA:
http://patches.dpdk.org/project/dpdk/patch/20210318182042.43658-6-

Quoted 8 lines

bruc
Quoted 1 line
[email protected]/
OK, thanks for the pointer.
While this new API is being reviewed, and for LTS, let's pick your patch. As soon as Bruce patch is merged, please send a new patch on top to make use of this API.

Sure, thanks a lot.
Cheng

Quoted 14 lines

Reviewed-by Maxime Coquelin <[email protected]>
Thanks,
Maxime
Quoted 8 lines
Thanks,
Jiayu
Quoted 5 lines
Quoted 5 lines
examples/vhost/ioat.c +0 -0
 		dma_info->nr++;
 		i++;
 	}

Thomas Monjalon

2021-04-07 08:26 UTC #6 lore raw

In reply to Maxime Coquelin

07/04/2021 09:47, Maxime Coquelin:

Quoted 15 lines · Cheng Jiang

Quoted 13 lines
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and check")
Cc [email protected]
Signed-off-by Cheng Jiang <[email protected]>
---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 14 lines

examples/vhost/ioat.c open_ioat(const char *value) · line 116
Quoted 10 lines
examples/vhost/ioat.c +1 -1
index 60b73be93..9cb5e0d50 100644
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
That really comforts me in thinking we need a generic abstraction for DMA devices. How is the application developer supposed to know that the DMA driver has such weird limitations?

Having a generic DMA API may be interesting. Do you know any other HW candidate for such an API? Do you think rte_memcpy can be used as a SW driver?

Thomas Monjalon

2021-04-07 08:43 UTC #7 lore raw

+Cc more people to start a discussion about a potential DMA API. If you think it is interesting, we can start a fresh discussion thread.

07/04/2021 10:26, Thomas Monjalon:

Quoted 7 lines

07/04/2021 09:47, Maxime Coquelin:
Quoted 6 lines · Cheng Jiang
Quoted 4 lines
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.

[...]

Quoted 16 lines

Quoted 4 lines
Quoted 4 lines
examples/vhost/ioat.c +0 -0
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
Quoted 8 lines
examples/vhost/ioat.c open_ioat(const char *value) · line 116
Quoted 4 lines
examples/vhost/ioat.c +1 -1
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
That really comforts me in thinking we need a generic abstraction for DMA devices. How is the application developer supposed to know that the DMA driver has such weird limitations?
Having a generic DMA API may be interesting. Do you know any other HW candidate for such an API? Do you think rte_memcpy can be used as a SW driver?

Liang Ma

2021-04-18 15:10 UTC #8 lore raw

Quoted 2 lines · Thomas Monjalon

+Cc more people to start a discussion about a potential DMA API. If you think it is interesting, we can start a fresh discussion thread.

+1 that's ineresting to have a abstract layer of DMA offload engine.
I would like join the discussion. please add me in the cc list in the new thread.

Quoted 28 lines

07/04/2021 10:26, Thomas Monjalon:
Quoted 7 lines
07/04/2021 09:47, Maxime Coquelin:
Quoted 6 lines · Cheng Jiang
Quoted 4 lines
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
[...]
Quoted 16 lines
Quoted 4 lines
Quoted 4 lines
examples/vhost/ioat.c +0 -0
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
Quoted 8 lines
examples/vhost/ioat.c open_ioat(const char *value) · line 116
Quoted 4 lines
examples/vhost/ioat.c +1 -1
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
That really comforts me in thinking we need a generic abstraction for DMA devices. How is the application developer supposed to know that the DMA driver has such weird limitations?
Having a generic DMA API may be interesting. Do you know any other HW candidate for such an API? Do you think rte_memcpy can be used as a SW driver?

Maxime Coquelin

2021-04-07 08:48 UTC #9 lore raw

In reply to Thomas Monjalon

Quoted 31 lines · Thomas Monjalon

07/04/2021 09:47, Maxime Coquelin:
Quoted 15 lines · Cheng Jiang
Quoted 13 lines
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and check")
Cc [email protected]
Signed-off-by Cheng Jiang <[email protected]>
---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 14 lines
examples/vhost/ioat.c open_ioat(const char *value) · line 116
Quoted 10 lines
examples/vhost/ioat.c +1 -1
index 60b73be93..9cb5e0d50 100644
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
That really comforts me in thinking we need a generic abstraction for DMA devices. How is the application developer supposed to know that the DMA driver has such weird limitations?

Quoted 3 lines

Having a generic DMA API may be interesting. Do you know any other HW candidate for such an API? Do you think rte_memcpy can be used as a SW driver?

Yes, I guess we could create a vdev driver with MEM_TO_MEM capability using rte_memcpy().

Note that IOAT in the Kernel is supported by the DMA framework.

Regards,
Maxime

Xia, Chenbo

2021-04-28 02:09 UTC #10 lore raw

In reply to Cheng Jiang

Quoted 36 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Wednesday, March 17, 2021 1:41 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; Jiang, Cheng1 <[email protected]>; [email protected]
Subject: [PATCH] examples/vhost: fix ioat ring space in callbacks
We use ioat ring space for determining if ioat callbacks can enqueue a packet to ioat device. But there is one slot can't be used in ioat ring due to the ioat driver design, so we need to reduce one slot in ioat ring to prevent ring size mismatch in ioat callbacks.
Fixes 2aa47e94bfb2 ("examples/vhost: add ioat ring space count and check")
Cc [email protected]
Signed-off-by Cheng Jiang <[email protected]>
---
 examples/vhost/ioat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
examples/vhost/ioat.c +1 -2
index 60b73be93..9cb5e0d50 100644
@@ -113,7 +113,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE - 1;
 		dma_info->nr++;
 		i++;
 	}
--

Quoted 1 line

2.29.2

Patch applied to next-virtio/main, Thanks