diff mbox series

[1/2] mempool: check driver enqueue result in one place

Message ID 20221009111154.213253-1-andrew.rybchenko@oktetlabs.ru (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [1/2] mempool: check driver enqueue result in one place |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andrew Rybchenko Oct. 9, 2022, 11:11 a.m. UTC 
  Enqueue operation must not fail. Move corresponding debug check
from one particular case to dequeue operation helper in order
to do it for all invocations.

Log critical message with useful information instead of rte_panic().

Make rte_mempool_do_generic_put() implementation more readable and
fix incosistency when return value is not checked in one place and
checked in another.

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 lib/mempool/rte_mempool.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Morten Brørup Oct. 9, 2022, 1:01 p.m. UTC | #1 
> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Sunday, 9 October 2022 13.12
> 
> Enqueue operation must not fail. Move corresponding debug check
> from one particular case to dequeue operation helper in order
> to do it for all invocations.
> 
> Log critical message with useful information instead of rte_panic().
> 
> Make rte_mempool_do_generic_put() implementation more readable and
> fix incosistency when return value is not checked in one place and
> checked in another.
> 
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---

Moving the debug check to cover all invocations is an improvement. Well spotted!

I have considered if panicking would still be appropriate instead of logging, and have come to the conclusion that I agree with that modification too.

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
diff mbox series

Patch

diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 4c4af2a8ed..95d64901e5 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -786,12 +786,19 @@  rte_mempool_ops_enqueue_bulk(struct rte_mempool *mp, void * const *obj_table,
 		unsigned n)
 {
 	struct rte_mempool_ops *ops;
+	int ret;
 
 	RTE_MEMPOOL_STAT_ADD(mp, put_common_pool_bulk, 1);
 	RTE_MEMPOOL_STAT_ADD(mp, put_common_pool_objs, n);
 	rte_mempool_trace_ops_enqueue_bulk(mp, obj_table, n);
 	ops = rte_mempool_get_ops(mp->ops_index);
-	return ops->enqueue(mp, obj_table, n);
+	ret = ops->enqueue(mp, obj_table, n);
+#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+	if (unlikely(ret < 0))
+		RTE_LOG(CRIT, MEMPOOL, "cannot enqueue %u objects to mempool %s\n",
+			n, mp->name);
+#endif
+	return ret;
 }
 
 /**
@@ -1351,12 +1358,7 @@  rte_mempool_do_generic_put(struct rte_mempool *mp, void * const *obj_table,
 ring_enqueue:
 
 	/* push remaining objects in ring */
-#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
-	if (rte_mempool_ops_enqueue_bulk(mp, obj_table, n) < 0)
-		rte_panic("cannot put objects in mempool\n");
-#else
 	rte_mempool_ops_enqueue_bulk(mp, obj_table, n);
-#endif
 }