diff mbox series

[3/5] mbuf: fix free_space setting for dynamic field

Message ID 20200613154922.42379-4-xiaolong.ye@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series small fixes for mbuf's dynamic field/flag feature |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Xiaolong Ye June 13, 2020, 3:49 p.m. UTC 
  The value free_space[i] is used to save the size of biggest aligned
element that can fit in the zone, current implementation has one flaw,
for example, if user registers dynfield1 (size = 4, align = 4, req = 124)
first, the free_space would be as below after registration:

  0070: 08 08 08 08 08 08 08 08
  0078: 08 08 08 08 00 00 00 00

Then if user continues to register dynfield2 (size = 4, align = 4),
free_space would become:

  0070: 00 00 00 00 04 04 04 04
  0078: 04 04 04 04 00 00 00 00

Further request dynfield3 (size = 8, align = 8) would fail to register
due to alignment requirement can't be satisfied, though there is enough
space remained in mbuf.

This patch fixes above issue by saving alignment only in aligned zone,
after the fix, above registrations order can be satisfied, free_space
would be like:

After dynfield1 registration:

  0070: 08 08 08 08 08 08 08 08
  0078: 04 04 04 04 00 00 00 00

After dynfield2 registration:

  0070: 08 08 08 08 08 08 08 08
  0078: 00 00 00 00 00 00 00 00

After dynfield3 registration:

  0070: 00 00 00 00 00 00 00 00
  0078: 00 00 00 00 00 00 00 00

This patch also reduces iterations in process_score() by jumping align
steps in each loop.

Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
Cc: stable@dpdk.org

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 lib/librte_mbuf/rte_mbuf_dyn.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Olivier Matz June 25, 2020, 3:53 p.m. UTC | #1 
On Sat, Jun 13, 2020 at 11:49:19PM +0800, Xiaolong Ye wrote:
> The value free_space[i] is used to save the size of biggest aligned
> element that can fit in the zone, current implementation has one flaw,
> for example, if user registers dynfield1 (size = 4, align = 4, req = 124)
> first, the free_space would be as below after registration:
> 
>   0070: 08 08 08 08 08 08 08 08
>   0078: 08 08 08 08 00 00 00 00
> 
> Then if user continues to register dynfield2 (size = 4, align = 4),
> free_space would become:
> 
>   0070: 00 00 00 00 04 04 04 04
>   0078: 04 04 04 04 00 00 00 00
> 
> Further request dynfield3 (size = 8, align = 8) would fail to register
> due to alignment requirement can't be satisfied, though there is enough
> space remained in mbuf.
> 
> This patch fixes above issue by saving alignment only in aligned zone,
> after the fix, above registrations order can be satisfied, free_space
> would be like:
> 
> After dynfield1 registration:
> 
>   0070: 08 08 08 08 08 08 08 08
>   0078: 04 04 04 04 00 00 00 00
> 
> After dynfield2 registration:
> 
>   0070: 08 08 08 08 08 08 08 08
>   0078: 00 00 00 00 00 00 00 00
> 
> After dynfield3 registration:
> 
>   0070: 00 00 00 00 00 00 00 00
>   0078: 00 00 00 00 00 00 00 00
> 
> This patch also reduces iterations in process_score() by jumping align
> steps in each loop.
> 
> Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thanks!
diff mbox series

Patch

diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
index de7d2eb9a5..fd51e1b68e 100644
--- a/lib/librte_mbuf/rte_mbuf_dyn.c
+++ b/lib/librte_mbuf/rte_mbuf_dyn.c
@@ -67,13 +67,16 @@  process_score(void)
 			shm->free_space[i] = 1;
 	}
 
-	for (off = 0; off < sizeof(struct rte_mbuf); off++) {
+	off = 0;
+	while (off < sizeof(struct rte_mbuf)) {
 		/* get the size of the free zone */
 		for (size = 0; (off + size) < sizeof(struct rte_mbuf) &&
 			     shm->free_space[off + size]; size++)
 			;
-		if (size == 0)
+		if (size == 0) {
+			off++;
 			continue;
+		}
 
 		/* get the alignment of biggest object that can fit in
 		 * the zone at this offset.
@@ -84,8 +87,10 @@  process_score(void)
 			;
 
 		/* save it in free_space[] */
-		for (i = off; i < off + size; i++)
+		for (i = off; i < off + align; i++)
 			shm->free_space[i] = RTE_MAX(align, shm->free_space[i]);
+
+		off += align;
 	}
 }