RE: [RFC v2 09/14] rcu: introduce a logging helper

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, December 8, 2023 9:00 AM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; ferruh.yigit@amd.com;
> bruce.richardson@intel.com; stephen@networkplumber.org;
> mb@smartsharesystems.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: [RFC v2 09/14] rcu: introduce a logging helper
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Thank you, looks good
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

> ---
>  lib/rcu/rte_rcu_qsbr.c | 62 ++++++++++++++++--------------------------
>  lib/rcu/rte_rcu_qsbr.h |  1 +
>  2 files changed, 24 insertions(+), 39 deletions(-)
> 
> diff --git a/lib/rcu/rte_rcu_qsbr.c b/lib/rcu/rte_rcu_qsbr.c index
> 41a44be4b9..5b6530788a 100644
> --- a/lib/rcu/rte_rcu_qsbr.c
> +++ b/lib/rcu/rte_rcu_qsbr.c
> @@ -19,6 +19,9 @@
>  #include "rte_rcu_qsbr.h"
>  #include "rcu_qsbr_pvt.h"
> 
> +#define RCU_LOG(level, fmt, args...) \
> +	RTE_LOG(level, RCU, "%s(): " fmt "\n", __func__, ## args)
> +
>  /* Get the memory size of QSBR variable */  size_t
> rte_rcu_qsbr_get_memsize(uint32_t max_threads) @@ -26,9 +29,7 @@
> rte_rcu_qsbr_get_memsize(uint32_t max_threads)
>  	size_t sz;
> 
>  	if (max_threads == 0) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid max_threads %u\n",
> -			__func__, max_threads);
> +		RCU_LOG(ERR, "Invalid max_threads %u", max_threads);
>  		rte_errno = EINVAL;
> 
>  		return 1;
> @@ -52,8 +53,7 @@ rte_rcu_qsbr_init(struct rte_rcu_qsbr *v, uint32_t
> max_threads)
>  	size_t sz;
> 
>  	if (v == NULL) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid input parameter\n", __func__);
> +		RCU_LOG(ERR, "Invalid input parameter");
>  		rte_errno = EINVAL;
> 
>  		return 1;
> @@ -85,8 +85,7 @@ rte_rcu_qsbr_thread_register(struct rte_rcu_qsbr *v,
> unsigned int thread_id)
>  	uint64_t old_bmap, new_bmap;
> 
>  	if (v == NULL || thread_id >= v->max_threads) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid input parameter\n", __func__);
> +		RCU_LOG(ERR, "Invalid input parameter");
>  		rte_errno = EINVAL;
> 
>  		return 1;
> @@ -137,8 +136,7 @@ rte_rcu_qsbr_thread_unregister(struct rte_rcu_qsbr
> *v, unsigned int thread_id)
>  	uint64_t old_bmap, new_bmap;
> 
>  	if (v == NULL || thread_id >= v->max_threads) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid input parameter\n", __func__);
> +		RCU_LOG(ERR, "Invalid input parameter");
>  		rte_errno = EINVAL;
> 
>  		return 1;
> @@ -211,8 +209,7 @@ rte_rcu_qsbr_dump(FILE *f, struct rte_rcu_qsbr *v)
>  	uint32_t i, t, id;
> 
>  	if (v == NULL || f == NULL) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid input parameter\n", __func__);
> +		RCU_LOG(ERR, "Invalid input parameter");
>  		rte_errno = EINVAL;
> 
>  		return 1;
> @@ -282,8 +279,7 @@ rte_rcu_qsbr_dq_create(const struct
> rte_rcu_qsbr_dq_parameters *params)
>  		params->v == NULL || params->name == NULL ||
>  		params->size == 0 || params->esize == 0 ||
>  		(params->esize % 4 != 0)) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid input parameter\n", __func__);
> +		RCU_LOG(ERR, "Invalid input parameter");
>  		rte_errno = EINVAL;
> 
>  		return NULL;
> @@ -293,9 +289,10 @@ rte_rcu_qsbr_dq_create(const struct
> rte_rcu_qsbr_dq_parameters *params)
>  	 */
>  	if ((params->trigger_reclaim_limit <= params->size) &&
>  	    (params->max_reclaim_size == 0)) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid input parameter, size = %u,
> trigger_reclaim_limit = %u, max_reclaim_size = %u\n",
> -			__func__, params->size, params-
> >trigger_reclaim_limit,
> +		RCU_LOG(ERR,
> +			"Invalid input parameter, size = %u,
> trigger_reclaim_limit = %u, "
> +			"max_reclaim_size = %u",
> +			params->size, params->trigger_reclaim_limit,
>  			params->max_reclaim_size);
>  		rte_errno = EINVAL;
> 
> @@ -328,8 +325,7 @@ rte_rcu_qsbr_dq_create(const struct
> rte_rcu_qsbr_dq_parameters *params)
>  			__RTE_QSBR_TOKEN_SIZE + params->esize,
>  			qs_fifo_size, SOCKET_ID_ANY, flags);
>  	if (dq->r == NULL) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): defer queue create failed\n", __func__);
> +		RCU_LOG(ERR, "defer queue create failed");
>  		rte_free(dq);
>  		return NULL;
>  	}
> @@ -354,8 +350,7 @@ int rte_rcu_qsbr_dq_enqueue(struct rte_rcu_qsbr_dq
> *dq, void *e)
>  	uint32_t cur_size;
> 
>  	if (dq == NULL || e == NULL) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid input parameter\n", __func__);
> +		RCU_LOG(ERR, "Invalid input parameter");
>  		rte_errno = EINVAL;
> 
>  		return 1;
> @@ -372,8 +367,7 @@ int rte_rcu_qsbr_dq_enqueue(struct rte_rcu_qsbr_dq
> *dq, void *e)
>  	 */
>  	cur_size = rte_ring_count(dq->r);
>  	if (cur_size > dq->trigger_reclaim_limit) {
> -		rte_log(RTE_LOG_INFO, rte_rcu_log_type,
> -			"%s(): Triggering reclamation\n", __func__);
> +		RCU_LOG(INFO, "Triggering reclamation");
>  		rte_rcu_qsbr_dq_reclaim(dq, dq->max_reclaim_size,
>  						NULL, NULL, NULL);
>  	}
> @@ -391,23 +385,18 @@ int rte_rcu_qsbr_dq_enqueue(struct
> rte_rcu_qsbr_dq *dq, void *e)
>  	 * Enqueue uses the configured flags when the DQ was created.
>  	 */
>  	if (rte_ring_enqueue_elem(dq->r, data, dq->esize) != 0) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Enqueue failed\n", __func__);
> +		RCU_LOG(ERR, "Enqueue failed");
>  		/* Note that the token generated above is not used.
>  		 * Other than wasting tokens, it should not cause any
>  		 * other issues.
>  		 */
> -		rte_log(RTE_LOG_INFO, rte_rcu_log_type,
> -			"%s(): Skipped enqueuing token = %" PRIu64 "\n",
> -			__func__, dq_elem->token);
> +		RCU_LOG(INFO, "Skipped enqueuing token = %" PRIu64,
> dq_elem->token);
> 
>  		rte_errno = ENOSPC;
>  		return 1;
>  	}
> 
> -	rte_log(RTE_LOG_INFO, rte_rcu_log_type,
> -		"%s(): Enqueued token = %" PRIu64 "\n",
> -		__func__, dq_elem->token);
> +	RCU_LOG(INFO, "Enqueued token = %" PRIu64, dq_elem->token);
> 
>  	return 0;
>  }
> @@ -422,8 +411,7 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq *dq,
> unsigned int n,
>  	__rte_rcu_qsbr_dq_elem_t *dq_elem;
> 
>  	if (dq == NULL || n == 0) {
> -		rte_log(RTE_LOG_ERR, rte_rcu_log_type,
> -			"%s(): Invalid input parameter\n", __func__);
> +		RCU_LOG(ERR, "Invalid input parameter");
>  		rte_errno = EINVAL;
> 
>  		return 1;
> @@ -445,17 +433,14 @@ rte_rcu_qsbr_dq_reclaim(struct rte_rcu_qsbr_dq
> *dq, unsigned int n,
>  		}
>  		rte_ring_dequeue_elem_finish(dq->r, 1);
> 
> -		rte_log(RTE_LOG_INFO, rte_rcu_log_type,
> -			"%s(): Reclaimed token = %" PRIu64 "\n",
> -			__func__, dq_elem->token);
> +		RCU_LOG(INFO, "Reclaimed token = %" PRIu64, dq_elem-
> >token);
> 
>  		dq->free_fn(dq->p, dq_elem->elem, 1);
> 
>  		cnt++;
>  	}
> 
> -	rte_log(RTE_LOG_INFO, rte_rcu_log_type,
> -		"%s(): Reclaimed %u resources\n", __func__, cnt);
> +	RCU_LOG(INFO, "Reclaimed %u resources", cnt);
> 
>  	if (freed != NULL)
>  		*freed = cnt;
> @@ -472,8 +457,7 @@ rte_rcu_qsbr_dq_delete(struct rte_rcu_qsbr_dq *dq)
>  	unsigned int pending;
> 
>  	if (dq == NULL) {
> -		rte_log(RTE_LOG_DEBUG, rte_rcu_log_type,
> -			"%s(): Invalid input parameter\n", __func__);
> +		RCU_LOG(DEBUG, "Invalid input parameter");
> 
>  		return 0;
>  	}
> diff --git a/lib/rcu/rte_rcu_qsbr.h b/lib/rcu/rte_rcu_qsbr.h index
> 6b908e7ee0..0dca8310c0 100644
> --- a/lib/rcu/rte_rcu_qsbr.h
> +++ b/lib/rcu/rte_rcu_qsbr.h
> @@ -36,6 +36,7 @@ extern "C" {
>  #include <rte_ring.h>
> 
>  extern int rte_rcu_log_type;
> +#define RTE_LOGTYPE_RCU rte_rcu_log_type
> 
>  #if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
>  #define __RTE_RCU_DP_LOG(level, fmt, args...) \
> --
> 2.43.0