[v2,34/45] event/dlb2: use rte stdatomic API

Message ID 1711048652-7512-35-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series use stdatomic API |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Tyler Retzlaff March 21, 2024, 7:17 p.m. UTC
  Replace the use of gcc builtin __atomic_xxx intrinsics with
corresponding rte_atomic_xxx optional rte stdatomic API.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 drivers/event/dlb2/dlb2.c        | 34 +++++++++++++++++-----------------
 drivers/event/dlb2/dlb2_priv.h   | 10 +++++-----
 drivers/event/dlb2/dlb2_xstats.c |  2 +-
 3 files changed, 23 insertions(+), 23 deletions(-)
  

Comments

Mattias Rönnblom March 21, 2024, 9:03 p.m. UTC | #1
On 2024-03-21 20:17, Tyler Retzlaff wrote:
> Replace the use of gcc builtin __atomic_xxx intrinsics with
> corresponding rte_atomic_xxx optional rte stdatomic API.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>   drivers/event/dlb2/dlb2.c        | 34 +++++++++++++++++-----------------
>   drivers/event/dlb2/dlb2_priv.h   | 10 +++++-----
>   drivers/event/dlb2/dlb2_xstats.c |  2 +-
>   3 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/event/dlb2/dlb2.c b/drivers/event/dlb2/dlb2.c
> index 628ddef..0b91f03 100644
> --- a/drivers/event/dlb2/dlb2.c
> +++ b/drivers/event/dlb2/dlb2.c
> @@ -1005,7 +1005,7 @@ struct process_local_port_data
>   	}
>   
>   	dlb2->new_event_limit = config->nb_events_limit;
> -	__atomic_store_n(&dlb2->inflights, 0, __ATOMIC_SEQ_CST);
> +	rte_atomic_store_explicit(&dlb2->inflights, 0, rte_memory_order_seq_cst);
>   
>   	/* Save number of ports/queues for this event dev */
>   	dlb2->num_ports = config->nb_event_ports;
> @@ -2668,10 +2668,10 @@ static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
>   		batch_size = credits;
>   
>   	if (likely(credits &&
> -		   __atomic_compare_exchange_n(
> +		   rte_atomic_compare_exchange_strong_explicit(
>   			qm_port->credit_pool[type],
> -			&credits, credits - batch_size, false,
> -			__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)))
> +			&credits, credits - batch_size,
> +			rte_memory_order_seq_cst, rte_memory_order_seq_cst)))
>   		return batch_size;
>   	else
>   		return 0;
> @@ -2687,7 +2687,7 @@ static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
>   		/* Replenish credits, saving one quanta for enqueues */
>   		uint16_t val = ev_port->inflight_credits - quanta;
>   
> -		__atomic_fetch_sub(&dlb2->inflights, val, __ATOMIC_SEQ_CST);
> +		rte_atomic_fetch_sub_explicit(&dlb2->inflights, val, rte_memory_order_seq_cst);
>   		ev_port->inflight_credits -= val;
>   	}
>   }
> @@ -2696,8 +2696,8 @@ static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
>   dlb2_check_enqueue_sw_credits(struct dlb2_eventdev *dlb2,
>   			      struct dlb2_eventdev_port *ev_port)
>   {
> -	uint32_t sw_inflights = __atomic_load_n(&dlb2->inflights,
> -						__ATOMIC_SEQ_CST);
> +	uint32_t sw_inflights = rte_atomic_load_explicit(&dlb2->inflights,
> +						rte_memory_order_seq_cst);
>   	const int num = 1;
>   
>   	if (unlikely(ev_port->inflight_max < sw_inflights)) {
> @@ -2719,8 +2719,8 @@ static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
>   			return 1;
>   		}
>   
> -		__atomic_fetch_add(&dlb2->inflights, credit_update_quanta,
> -				   __ATOMIC_SEQ_CST);
> +		rte_atomic_fetch_add_explicit(&dlb2->inflights, credit_update_quanta,
> +				   rte_memory_order_seq_cst);
>   		ev_port->inflight_credits += (credit_update_quanta);
>   
>   		if (ev_port->inflight_credits < num) {
> @@ -3234,17 +3234,17 @@ static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
>   		if (qm_port->dlb2->version == DLB2_HW_V2) {
>   			qm_port->cached_ldb_credits += num;
>   			if (qm_port->cached_ldb_credits >= 2 * batch_size) {
> -				__atomic_fetch_add(
> +				rte_atomic_fetch_add_explicit(
>   					qm_port->credit_pool[DLB2_LDB_QUEUE],
> -					batch_size, __ATOMIC_SEQ_CST);
> +					batch_size, rte_memory_order_seq_cst);
>   				qm_port->cached_ldb_credits -= batch_size;
>   			}
>   		} else {
>   			qm_port->cached_credits += num;
>   			if (qm_port->cached_credits >= 2 * batch_size) {
> -				__atomic_fetch_add(
> +				rte_atomic_fetch_add_explicit(
>   				      qm_port->credit_pool[DLB2_COMBINED_POOL],
> -				      batch_size, __ATOMIC_SEQ_CST);
> +				      batch_size, rte_memory_order_seq_cst);
>   				qm_port->cached_credits -= batch_size;
>   			}
>   		}
> @@ -3252,17 +3252,17 @@ static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
>   		if (qm_port->dlb2->version == DLB2_HW_V2) {
>   			qm_port->cached_dir_credits += num;
>   			if (qm_port->cached_dir_credits >= 2 * batch_size) {
> -				__atomic_fetch_add(
> +				rte_atomic_fetch_add_explicit(
>   					qm_port->credit_pool[DLB2_DIR_QUEUE],
> -					batch_size, __ATOMIC_SEQ_CST);
> +					batch_size, rte_memory_order_seq_cst);
>   				qm_port->cached_dir_credits -= batch_size;
>   			}
>   		} else {
>   			qm_port->cached_credits += num;
>   			if (qm_port->cached_credits >= 2 * batch_size) {
> -				__atomic_fetch_add(
> +				rte_atomic_fetch_add_explicit(
>   				      qm_port->credit_pool[DLB2_COMBINED_POOL],
> -				      batch_size, __ATOMIC_SEQ_CST);
> +				      batch_size, rte_memory_order_seq_cst);
>   				qm_port->cached_credits -= batch_size;
>   			}
>   		}
> diff --git a/drivers/event/dlb2/dlb2_priv.h b/drivers/event/dlb2/dlb2_priv.h
> index 31a3bee..46883f2 100644
> --- a/drivers/event/dlb2/dlb2_priv.h
> +++ b/drivers/event/dlb2/dlb2_priv.h
> @@ -348,7 +348,7 @@ struct dlb2_port {
>   	uint32_t dequeue_depth;
>   	enum dlb2_token_pop_mode token_pop_mode;
>   	union dlb2_port_config cfg;
> -	uint32_t *credit_pool[DLB2_NUM_QUEUE_TYPES]; /* use __atomic builtins */
> +	RTE_ATOMIC(uint32_t) *credit_pool[DLB2_NUM_QUEUE_TYPES];
>   	union {
>   		struct {
>   			uint16_t cached_ldb_credits;
> @@ -586,7 +586,7 @@ struct dlb2_eventdev {
>   	uint32_t xstats_count_mode_dev;
>   	uint32_t xstats_count_mode_port;
>   	uint32_t xstats_count;
> -	uint32_t inflights; /* use __atomic builtins */
> +	RTE_ATOMIC(uint32_t) inflights; /* use __atomic builtins */
>   	uint32_t new_event_limit;
>   	int max_num_events_override;
>   	int num_dir_credits_override;
> @@ -624,14 +624,14 @@ struct dlb2_eventdev {
>   			uint16_t max_ldb_credits;
>   			uint16_t max_dir_credits;
>   			/* use __atomic builtins */ /* shared hw cred */

Delete the first of the above two comments.

> -			uint32_t ldb_credit_pool __rte_cache_aligned;
> +			RTE_ATOMIC(uint32_t) ldb_credit_pool __rte_cache_aligned;
>   			/* use __atomic builtins */ /* shared hw cred */

Same here.

> -			uint32_t dir_credit_pool __rte_cache_aligned;
> +			RTE_ATOMIC(uint32_t) dir_credit_pool __rte_cache_aligned;
>   		};
>   		struct {
>   			uint16_t max_credits;
>   			/* use __atomic builtins */ /* shared hw cred */
> -			uint32_t credit_pool __rte_cache_aligned;
> +			RTE_ATOMIC(uint32_t) credit_pool __rte_cache_aligned;
>   		};
>   	};
>   	uint32_t cos_ports[DLB2_COS_NUM_VALS]; /* total ldb ports in each class */
> diff --git a/drivers/event/dlb2/dlb2_xstats.c b/drivers/event/dlb2/dlb2_xstats.c
> index ff15271..22094f3 100644
> --- a/drivers/event/dlb2/dlb2_xstats.c
> +++ b/drivers/event/dlb2/dlb2_xstats.c
> @@ -173,7 +173,7 @@ struct dlb2_xstats_entry {
>   	case nb_events_limit:
>   		return dlb2->new_event_limit;
>   	case inflight_events:
> -		return __atomic_load_n(&dlb2->inflights, __ATOMIC_SEQ_CST);
> +		return rte_atomic_load_explicit(&dlb2->inflights, rte_memory_order_seq_cst);

This is more a question to the driver maintainer, but why does this load 
need to be CST? What stores need it to be ordered against. Even 
load-acquire seems overkill to me, but I may well be missing something.

>   	case ldb_pool_size:
>   		return dlb2->num_ldb_credits;
>   	case dir_pool_size:
  
Sevincer, Abdullah April 9, 2024, 7:31 p.m. UTC | #2
>+uint32_t *credit_pool[DLB2_NUM_QUEUE_TYPES]; /* use __atomic builtins */

Spell check complains here you can add a space after the '*'  if you don’t want that complaint. (e.g. uint32_t * credit_pool).


>   	case nb_events_limit:
>   		return dlb2->new_event_limit;
>   	case inflight_events:
> -		return __atomic_load_n(&dlb2->inflights, __ATOMIC_SEQ_CST);
> +		return rte_atomic_load_explicit(&dlb2->inflights, 
> +rte_memory_order_seq_cst);

>+This is more a question to the driver maintainer, but why does this load need to be CST? What stores need it to be ordered against. Even load-acquire seems overkill to me, but I may well be missing something.

I am not sure of this why previous maintainers went this way and I am looking into it . To me now it looks like the strict requirements can be changed. If so I will submit a patch later to change and address this.
  

Patch

diff --git a/drivers/event/dlb2/dlb2.c b/drivers/event/dlb2/dlb2.c
index 628ddef..0b91f03 100644
--- a/drivers/event/dlb2/dlb2.c
+++ b/drivers/event/dlb2/dlb2.c
@@ -1005,7 +1005,7 @@  struct process_local_port_data
 	}
 
 	dlb2->new_event_limit = config->nb_events_limit;
-	__atomic_store_n(&dlb2->inflights, 0, __ATOMIC_SEQ_CST);
+	rte_atomic_store_explicit(&dlb2->inflights, 0, rte_memory_order_seq_cst);
 
 	/* Save number of ports/queues for this event dev */
 	dlb2->num_ports = config->nb_event_ports;
@@ -2668,10 +2668,10 @@  static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
 		batch_size = credits;
 
 	if (likely(credits &&
-		   __atomic_compare_exchange_n(
+		   rte_atomic_compare_exchange_strong_explicit(
 			qm_port->credit_pool[type],
-			&credits, credits - batch_size, false,
-			__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)))
+			&credits, credits - batch_size,
+			rte_memory_order_seq_cst, rte_memory_order_seq_cst)))
 		return batch_size;
 	else
 		return 0;
@@ -2687,7 +2687,7 @@  static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
 		/* Replenish credits, saving one quanta for enqueues */
 		uint16_t val = ev_port->inflight_credits - quanta;
 
-		__atomic_fetch_sub(&dlb2->inflights, val, __ATOMIC_SEQ_CST);
+		rte_atomic_fetch_sub_explicit(&dlb2->inflights, val, rte_memory_order_seq_cst);
 		ev_port->inflight_credits -= val;
 	}
 }
@@ -2696,8 +2696,8 @@  static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
 dlb2_check_enqueue_sw_credits(struct dlb2_eventdev *dlb2,
 			      struct dlb2_eventdev_port *ev_port)
 {
-	uint32_t sw_inflights = __atomic_load_n(&dlb2->inflights,
-						__ATOMIC_SEQ_CST);
+	uint32_t sw_inflights = rte_atomic_load_explicit(&dlb2->inflights,
+						rte_memory_order_seq_cst);
 	const int num = 1;
 
 	if (unlikely(ev_port->inflight_max < sw_inflights)) {
@@ -2719,8 +2719,8 @@  static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
 			return 1;
 		}
 
-		__atomic_fetch_add(&dlb2->inflights, credit_update_quanta,
-				   __ATOMIC_SEQ_CST);
+		rte_atomic_fetch_add_explicit(&dlb2->inflights, credit_update_quanta,
+				   rte_memory_order_seq_cst);
 		ev_port->inflight_credits += (credit_update_quanta);
 
 		if (ev_port->inflight_credits < num) {
@@ -3234,17 +3234,17 @@  static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
 		if (qm_port->dlb2->version == DLB2_HW_V2) {
 			qm_port->cached_ldb_credits += num;
 			if (qm_port->cached_ldb_credits >= 2 * batch_size) {
-				__atomic_fetch_add(
+				rte_atomic_fetch_add_explicit(
 					qm_port->credit_pool[DLB2_LDB_QUEUE],
-					batch_size, __ATOMIC_SEQ_CST);
+					batch_size, rte_memory_order_seq_cst);
 				qm_port->cached_ldb_credits -= batch_size;
 			}
 		} else {
 			qm_port->cached_credits += num;
 			if (qm_port->cached_credits >= 2 * batch_size) {
-				__atomic_fetch_add(
+				rte_atomic_fetch_add_explicit(
 				      qm_port->credit_pool[DLB2_COMBINED_POOL],
-				      batch_size, __ATOMIC_SEQ_CST);
+				      batch_size, rte_memory_order_seq_cst);
 				qm_port->cached_credits -= batch_size;
 			}
 		}
@@ -3252,17 +3252,17 @@  static int dlb2_num_dir_queues_setup(struct dlb2_eventdev *dlb2)
 		if (qm_port->dlb2->version == DLB2_HW_V2) {
 			qm_port->cached_dir_credits += num;
 			if (qm_port->cached_dir_credits >= 2 * batch_size) {
-				__atomic_fetch_add(
+				rte_atomic_fetch_add_explicit(
 					qm_port->credit_pool[DLB2_DIR_QUEUE],
-					batch_size, __ATOMIC_SEQ_CST);
+					batch_size, rte_memory_order_seq_cst);
 				qm_port->cached_dir_credits -= batch_size;
 			}
 		} else {
 			qm_port->cached_credits += num;
 			if (qm_port->cached_credits >= 2 * batch_size) {
-				__atomic_fetch_add(
+				rte_atomic_fetch_add_explicit(
 				      qm_port->credit_pool[DLB2_COMBINED_POOL],
-				      batch_size, __ATOMIC_SEQ_CST);
+				      batch_size, rte_memory_order_seq_cst);
 				qm_port->cached_credits -= batch_size;
 			}
 		}
diff --git a/drivers/event/dlb2/dlb2_priv.h b/drivers/event/dlb2/dlb2_priv.h
index 31a3bee..46883f2 100644
--- a/drivers/event/dlb2/dlb2_priv.h
+++ b/drivers/event/dlb2/dlb2_priv.h
@@ -348,7 +348,7 @@  struct dlb2_port {
 	uint32_t dequeue_depth;
 	enum dlb2_token_pop_mode token_pop_mode;
 	union dlb2_port_config cfg;
-	uint32_t *credit_pool[DLB2_NUM_QUEUE_TYPES]; /* use __atomic builtins */
+	RTE_ATOMIC(uint32_t) *credit_pool[DLB2_NUM_QUEUE_TYPES];
 	union {
 		struct {
 			uint16_t cached_ldb_credits;
@@ -586,7 +586,7 @@  struct dlb2_eventdev {
 	uint32_t xstats_count_mode_dev;
 	uint32_t xstats_count_mode_port;
 	uint32_t xstats_count;
-	uint32_t inflights; /* use __atomic builtins */
+	RTE_ATOMIC(uint32_t) inflights; /* use __atomic builtins */
 	uint32_t new_event_limit;
 	int max_num_events_override;
 	int num_dir_credits_override;
@@ -624,14 +624,14 @@  struct dlb2_eventdev {
 			uint16_t max_ldb_credits;
 			uint16_t max_dir_credits;
 			/* use __atomic builtins */ /* shared hw cred */
-			uint32_t ldb_credit_pool __rte_cache_aligned;
+			RTE_ATOMIC(uint32_t) ldb_credit_pool __rte_cache_aligned;
 			/* use __atomic builtins */ /* shared hw cred */
-			uint32_t dir_credit_pool __rte_cache_aligned;
+			RTE_ATOMIC(uint32_t) dir_credit_pool __rte_cache_aligned;
 		};
 		struct {
 			uint16_t max_credits;
 			/* use __atomic builtins */ /* shared hw cred */
-			uint32_t credit_pool __rte_cache_aligned;
+			RTE_ATOMIC(uint32_t) credit_pool __rte_cache_aligned;
 		};
 	};
 	uint32_t cos_ports[DLB2_COS_NUM_VALS]; /* total ldb ports in each class */
diff --git a/drivers/event/dlb2/dlb2_xstats.c b/drivers/event/dlb2/dlb2_xstats.c
index ff15271..22094f3 100644
--- a/drivers/event/dlb2/dlb2_xstats.c
+++ b/drivers/event/dlb2/dlb2_xstats.c
@@ -173,7 +173,7 @@  struct dlb2_xstats_entry {
 	case nb_events_limit:
 		return dlb2->new_event_limit;
 	case inflight_events:
-		return __atomic_load_n(&dlb2->inflights, __ATOMIC_SEQ_CST);
+		return rte_atomic_load_explicit(&dlb2->inflights, rte_memory_order_seq_cst);
 	case ldb_pool_size:
 		return dlb2->num_ldb_credits;
 	case dir_pool_size: