All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] event/cnxk: fix burst timer arm routine
@ 2023-02-02  8:10 pbhagavatula
  2023-02-02  8:10 ` [PATCH 2/2] event/cnxk: update timer arm burst parameters pbhagavatula
  2023-02-07  9:27 ` [PATCH 1/2] event/cnxk: fix burst timer arm routine Jerin Jacob
  0 siblings, 2 replies; 3+ messages in thread
From: pbhagavatula @ 2023-02-02  8:10 UTC (permalink / raw)
  To: jerinj, Pavan Nikhilesh, Shijith Thotton; +Cc: dev

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Fix timer burst arm routine writing improper updates to
the bucket.

Fixes: 5f644e1bd14c ("event/cnxk: add timer arm timeout burst")

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 drivers/event/cnxk/cnxk_tim_worker.h | 81 ++++++++++++++--------------
 1 file changed, 42 insertions(+), 39 deletions(-)

diff --git a/drivers/event/cnxk/cnxk_tim_worker.h b/drivers/event/cnxk/cnxk_tim_worker.h
index eda84c6f31..6be31f6f9d 100644
--- a/drivers/event/cnxk/cnxk_tim_worker.h
+++ b/drivers/event/cnxk/cnxk_tim_worker.h
@@ -270,7 +270,8 @@ cnxk_tim_add_entry_sp(struct cnxk_tim_ring *const tim_ring,
 			} while (hbt_state & BIT_ULL(33));
 #endif
 
-			if (!(hbt_state & BIT_ULL(34))) {
+			if (!(hbt_state & BIT_ULL(34)) ||
+			    !(hbt_state & GENMASK(31, 0))) {
 				cnxk_tim_bkt_dec_lock(bkt);
 				goto __retry;
 			}
@@ -352,7 +353,8 @@ cnxk_tim_add_entry_mp(struct cnxk_tim_ring *const tim_ring,
 			} while (hbt_state & BIT_ULL(33));
 #endif
 
-			if (!(hbt_state & BIT_ULL(34))) {
+			if (!(hbt_state & BIT_ULL(34)) ||
+			    !(hbt_state & GENMASK(31, 0))) {
 				cnxk_tim_bkt_dec_lock(bkt);
 				goto __retry;
 			}
@@ -449,10 +451,10 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
 	struct cnxk_tim_ent *chunk = NULL;
 	struct cnxk_tim_bkt *mirr_bkt;
 	struct cnxk_tim_bkt *bkt;
-	uint16_t chunk_remainder;
+	int16_t chunk_remainder;
 	uint16_t index = 0;
 	uint64_t lock_sema;
-	int16_t rem, crem;
+	int16_t rem;
 	uint8_t lock_cnt;
 
 __retry:
@@ -460,31 +462,6 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
 
 	/* Only one thread beyond this. */
 	lock_sema = cnxk_tim_bkt_inc_lock(bkt);
-	lock_cnt = (uint8_t)((lock_sema >> TIM_BUCKET_W1_S_LOCK) &
-			     TIM_BUCKET_W1_M_LOCK);
-
-	if (lock_cnt) {
-		cnxk_tim_bkt_dec_lock(bkt);
-#ifdef RTE_ARCH_ARM64
-		asm volatile(PLT_CPU_FEATURE_PREAMBLE
-			     "		ldxrb %w[lock_cnt], [%[lock]]	\n"
-			     "		tst %w[lock_cnt], 255		\n"
-			     "		beq dne%=			\n"
-			     "		sevl				\n"
-			     "rty%=:	wfe				\n"
-			     "		ldxrb %w[lock_cnt], [%[lock]]	\n"
-			     "		tst %w[lock_cnt], 255		\n"
-			     "		bne rty%=			\n"
-			     "dne%=:					\n"
-			     : [lock_cnt] "=&r"(lock_cnt)
-			     : [lock] "r"(&bkt->lock)
-			     : "memory");
-#else
-		while (__atomic_load_n(&bkt->lock, __ATOMIC_RELAXED))
-			;
-#endif
-		goto __retry;
-	}
 
 	/* Bucket related checks. */
 	if (unlikely(cnxk_tim_bkt_get_hbt(lock_sema))) {
@@ -509,21 +486,46 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
 			} while (hbt_state & BIT_ULL(33));
 #endif
 
-			if (!(hbt_state & BIT_ULL(34))) {
+			if (!(hbt_state & BIT_ULL(34)) ||
+			    !(hbt_state & GENMASK(31, 0))) {
 				cnxk_tim_bkt_dec_lock(bkt);
 				goto __retry;
 			}
 		}
 	}
 
+	lock_cnt = (uint8_t)((lock_sema >> TIM_BUCKET_W1_S_LOCK) &
+			     TIM_BUCKET_W1_M_LOCK);
+	if (lock_cnt) {
+		cnxk_tim_bkt_dec_lock(bkt);
+#ifdef RTE_ARCH_ARM64
+		asm volatile(PLT_CPU_FEATURE_PREAMBLE
+			     "		ldxrb %w[lock_cnt], [%[lock]]	\n"
+			     "		tst %w[lock_cnt], 255		\n"
+			     "		beq dne%=			\n"
+			     "		sevl				\n"
+			     "rty%=:	wfe				\n"
+			     "		ldxrb %w[lock_cnt], [%[lock]]	\n"
+			     "		tst %w[lock_cnt], 255		\n"
+			     "		bne rty%=			\n"
+			     "dne%=:					\n"
+			     : [lock_cnt] "=&r"(lock_cnt)
+			     : [lock] "r"(&bkt->lock)
+			     : "memory");
+#else
+		while (__atomic_load_n(&bkt->lock, __ATOMIC_RELAXED))
+			;
+#endif
+		goto __retry;
+	}
+
 	chunk_remainder = cnxk_tim_bkt_fetch_rem(lock_sema);
 	rem = chunk_remainder - nb_timers;
 	if (rem < 0) {
-		crem = tim_ring->nb_chunk_slots - chunk_remainder;
-		if (chunk_remainder && crem) {
+		if (chunk_remainder > 0) {
 			chunk = ((struct cnxk_tim_ent *)
 					 mirr_bkt->current_chunk) +
-				crem;
+				tim_ring->nb_chunk_slots - chunk_remainder;
 
 			index = cnxk_tim_cpy_wrk(index, chunk_remainder, chunk,
 						 tim, ents, bkt);
@@ -537,18 +539,19 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
 			chunk = cnxk_tim_insert_chunk(bkt, mirr_bkt, tim_ring);
 
 		if (unlikely(chunk == NULL)) {
-			cnxk_tim_bkt_dec_lock(bkt);
+			cnxk_tim_bkt_dec_lock_relaxed(bkt);
 			rte_errno = ENOMEM;
 			tim[index]->state = RTE_EVENT_TIMER_ERROR;
-			return crem;
+			return index;
 		}
 		*(uint64_t *)(chunk + tim_ring->nb_chunk_slots) = 0;
 		mirr_bkt->current_chunk = (uintptr_t)chunk;
-		cnxk_tim_cpy_wrk(index, nb_timers, chunk, tim, ents, bkt);
+		index = cnxk_tim_cpy_wrk(index, nb_timers, chunk, tim, ents,
+					 bkt) -
+			index;
 
-		rem = nb_timers - chunk_remainder;
-		cnxk_tim_bkt_set_rem(bkt, tim_ring->nb_chunk_slots - rem);
-		cnxk_tim_bkt_add_nent(bkt, rem);
+		cnxk_tim_bkt_set_rem(bkt, tim_ring->nb_chunk_slots - index);
+		cnxk_tim_bkt_add_nent(bkt, index);
 	} else {
 		chunk = (struct cnxk_tim_ent *)mirr_bkt->current_chunk;
 		chunk += (tim_ring->nb_chunk_slots - chunk_remainder);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] event/cnxk: update timer arm burst parameters
  2023-02-02  8:10 [PATCH 1/2] event/cnxk: fix burst timer arm routine pbhagavatula
@ 2023-02-02  8:10 ` pbhagavatula
  2023-02-07  9:27 ` [PATCH 1/2] event/cnxk: fix burst timer arm routine Jerin Jacob
  1 sibling, 0 replies; 3+ messages in thread
From: pbhagavatula @ 2023-02-02  8:10 UTC (permalink / raw)
  To: jerinj, Pavan Nikhilesh, Shijith Thotton; +Cc: dev

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Increase the timer arm burst size to 16 and chunk size
for optimum performance.
Use fixed size chunk pool cache to avoid high alloc cycles.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 drivers/event/cnxk/cnxk_tim_evdev.c  |  7 ++-----
 drivers/event/cnxk/cnxk_tim_evdev.h  |  5 ++---
 drivers/event/cnxk/cnxk_tim_worker.h | 12 +++++++++---
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/event/cnxk/cnxk_tim_evdev.c b/drivers/event/cnxk/cnxk_tim_evdev.c
index 5dd79cbd47..6ff3ca72f7 100644
--- a/drivers/event/cnxk/cnxk_tim_evdev.c
+++ b/drivers/event/cnxk/cnxk_tim_evdev.c
@@ -14,12 +14,11 @@ static int
 cnxk_tim_chnk_pool_create(struct cnxk_tim_ring *tim_ring,
 			  struct rte_event_timer_adapter_conf *rcfg)
 {
-	unsigned int cache_sz = (tim_ring->nb_chunks / 1.5);
 	unsigned int mp_flags = 0;
+	unsigned int cache_sz;
 	char pool_name[25];
 	int rc;
 
-	cache_sz /= rte_lcore_count();
 	/* Create chunk pool. */
 	if (rcfg->flags & RTE_EVENT_TIMER_ADAPTER_F_SP_PUT) {
 		mp_flags = RTE_MEMPOOL_F_SP_PUT | RTE_MEMPOOL_F_SC_GET;
@@ -30,9 +29,7 @@ cnxk_tim_chnk_pool_create(struct cnxk_tim_ring *tim_ring,
 	snprintf(pool_name, sizeof(pool_name), "cnxk_tim_chunk_pool%d",
 		 tim_ring->ring_id);
 
-	if (cache_sz > CNXK_TIM_MAX_POOL_CACHE_SZ)
-		cache_sz = CNXK_TIM_MAX_POOL_CACHE_SZ;
-	cache_sz = cache_sz != 0 ? cache_sz : 2;
+	cache_sz = CNXK_TIM_MAX_POOL_CACHE_SZ;
 	tim_ring->nb_chunks += (cache_sz * rte_lcore_count());
 	if (!tim_ring->disable_npa) {
 		tim_ring->chunk_pool = rte_mempool_create_empty(
diff --git a/drivers/event/cnxk/cnxk_tim_evdev.h b/drivers/event/cnxk/cnxk_tim_evdev.h
index 0c192346c7..8c69d15c80 100644
--- a/drivers/event/cnxk/cnxk_tim_evdev.h
+++ b/drivers/event/cnxk/cnxk_tim_evdev.h
@@ -24,10 +24,9 @@
 
 #define CNXK_TIM_EVDEV_NAME	    cnxk_tim_eventdev
 #define CNXK_TIM_MAX_BUCKETS	    (0xFFFFF)
-#define CNXK_TIM_RING_DEF_CHUNK_SZ  (256)
+#define CNXK_TIM_RING_DEF_CHUNK_SZ  (1024)
 #define CNXK_TIM_CHUNK_ALIGNMENT    (16)
-#define CNXK_TIM_MAX_BURST	    \
-			(RTE_CACHE_LINE_SIZE / CNXK_TIM_CHUNK_ALIGNMENT)
+#define CNXK_TIM_MAX_BURST	    (16)
 #define CNXK_TIM_NB_CHUNK_SLOTS(sz) (((sz) / CNXK_TIM_CHUNK_ALIGNMENT) - 1)
 #define CNXK_TIM_MIN_CHUNK_SLOTS    (0x1)
 #define CNXK_TIM_MAX_CHUNK_SLOTS    (0x1FFE)
diff --git a/drivers/event/cnxk/cnxk_tim_worker.h b/drivers/event/cnxk/cnxk_tim_worker.h
index 6be31f6f9d..87ac91f387 100644
--- a/drivers/event/cnxk/cnxk_tim_worker.h
+++ b/drivers/event/cnxk/cnxk_tim_worker.h
@@ -106,11 +106,17 @@ cnxk_tim_bkt_inc_nent(struct cnxk_tim_bkt *bktp)
 }
 
 static inline void
-cnxk_tim_bkt_add_nent(struct cnxk_tim_bkt *bktp, uint32_t v)
+cnxk_tim_bkt_add_nent_relaxed(struct cnxk_tim_bkt *bktp, uint32_t v)
 {
 	__atomic_add_fetch(&bktp->nb_entry, v, __ATOMIC_RELAXED);
 }
 
+static inline void
+cnxk_tim_bkt_add_nent(struct cnxk_tim_bkt *bktp, uint32_t v)
+{
+	__atomic_add_fetch(&bktp->nb_entry, v, __ATOMIC_RELEASE);
+}
+
 static inline uint64_t
 cnxk_tim_bkt_clr_nent(struct cnxk_tim_bkt *bktp)
 {
@@ -530,7 +536,7 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
 			index = cnxk_tim_cpy_wrk(index, chunk_remainder, chunk,
 						 tim, ents, bkt);
 			cnxk_tim_bkt_sub_rem(bkt, chunk_remainder);
-			cnxk_tim_bkt_add_nent(bkt, chunk_remainder);
+			cnxk_tim_bkt_add_nent_relaxed(bkt, chunk_remainder);
 		}
 
 		if (flags & CNXK_TIM_ENA_FB)
@@ -561,7 +567,7 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
 		cnxk_tim_bkt_add_nent(bkt, nb_timers);
 	}
 
-	cnxk_tim_bkt_dec_lock(bkt);
+	cnxk_tim_bkt_dec_lock_relaxed(bkt);
 
 	return nb_timers;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] event/cnxk: fix burst timer arm routine
  2023-02-02  8:10 [PATCH 1/2] event/cnxk: fix burst timer arm routine pbhagavatula
  2023-02-02  8:10 ` [PATCH 2/2] event/cnxk: update timer arm burst parameters pbhagavatula
@ 2023-02-07  9:27 ` Jerin Jacob
  1 sibling, 0 replies; 3+ messages in thread
From: Jerin Jacob @ 2023-02-07  9:27 UTC (permalink / raw)
  To: pbhagavatula; +Cc: jerinj, Shijith Thotton, dev

On Thu, Feb 2, 2023 at 1:40 PM <pbhagavatula@marvell.com> wrote:
>
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Fix timer burst arm routine writing improper updates to
> the bucket.
>
> Fixes: 5f644e1bd14c ("event/cnxk: add timer arm timeout burst")
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---


Series applied to dpdk-next-net-eventdev/for-main. Thanks

>  drivers/event/cnxk/cnxk_tim_worker.h | 81 ++++++++++++++--------------
>  1 file changed, 42 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/event/cnxk/cnxk_tim_worker.h b/drivers/event/cnxk/cnxk_tim_worker.h
> index eda84c6f31..6be31f6f9d 100644
> --- a/drivers/event/cnxk/cnxk_tim_worker.h
> +++ b/drivers/event/cnxk/cnxk_tim_worker.h
> @@ -270,7 +270,8 @@ cnxk_tim_add_entry_sp(struct cnxk_tim_ring *const tim_ring,
>                         } while (hbt_state & BIT_ULL(33));
>  #endif
>
> -                       if (!(hbt_state & BIT_ULL(34))) {
> +                       if (!(hbt_state & BIT_ULL(34)) ||
> +                           !(hbt_state & GENMASK(31, 0))) {
>                                 cnxk_tim_bkt_dec_lock(bkt);
>                                 goto __retry;
>                         }
> @@ -352,7 +353,8 @@ cnxk_tim_add_entry_mp(struct cnxk_tim_ring *const tim_ring,
>                         } while (hbt_state & BIT_ULL(33));
>  #endif
>
> -                       if (!(hbt_state & BIT_ULL(34))) {
> +                       if (!(hbt_state & BIT_ULL(34)) ||
> +                           !(hbt_state & GENMASK(31, 0))) {
>                                 cnxk_tim_bkt_dec_lock(bkt);
>                                 goto __retry;
>                         }
> @@ -449,10 +451,10 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
>         struct cnxk_tim_ent *chunk = NULL;
>         struct cnxk_tim_bkt *mirr_bkt;
>         struct cnxk_tim_bkt *bkt;
> -       uint16_t chunk_remainder;
> +       int16_t chunk_remainder;
>         uint16_t index = 0;
>         uint64_t lock_sema;
> -       int16_t rem, crem;
> +       int16_t rem;
>         uint8_t lock_cnt;
>
>  __retry:
> @@ -460,31 +462,6 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
>
>         /* Only one thread beyond this. */
>         lock_sema = cnxk_tim_bkt_inc_lock(bkt);
> -       lock_cnt = (uint8_t)((lock_sema >> TIM_BUCKET_W1_S_LOCK) &
> -                            TIM_BUCKET_W1_M_LOCK);
> -
> -       if (lock_cnt) {
> -               cnxk_tim_bkt_dec_lock(bkt);
> -#ifdef RTE_ARCH_ARM64
> -               asm volatile(PLT_CPU_FEATURE_PREAMBLE
> -                            "          ldxrb %w[lock_cnt], [%[lock]]   \n"
> -                            "          tst %w[lock_cnt], 255           \n"
> -                            "          beq dne%=                       \n"
> -                            "          sevl                            \n"
> -                            "rty%=:    wfe                             \n"
> -                            "          ldxrb %w[lock_cnt], [%[lock]]   \n"
> -                            "          tst %w[lock_cnt], 255           \n"
> -                            "          bne rty%=                       \n"
> -                            "dne%=:                                    \n"
> -                            : [lock_cnt] "=&r"(lock_cnt)
> -                            : [lock] "r"(&bkt->lock)
> -                            : "memory");
> -#else
> -               while (__atomic_load_n(&bkt->lock, __ATOMIC_RELAXED))
> -                       ;
> -#endif
> -               goto __retry;
> -       }
>
>         /* Bucket related checks. */
>         if (unlikely(cnxk_tim_bkt_get_hbt(lock_sema))) {
> @@ -509,21 +486,46 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
>                         } while (hbt_state & BIT_ULL(33));
>  #endif
>
> -                       if (!(hbt_state & BIT_ULL(34))) {
> +                       if (!(hbt_state & BIT_ULL(34)) ||
> +                           !(hbt_state & GENMASK(31, 0))) {
>                                 cnxk_tim_bkt_dec_lock(bkt);
>                                 goto __retry;
>                         }
>                 }
>         }
>
> +       lock_cnt = (uint8_t)((lock_sema >> TIM_BUCKET_W1_S_LOCK) &
> +                            TIM_BUCKET_W1_M_LOCK);
> +       if (lock_cnt) {
> +               cnxk_tim_bkt_dec_lock(bkt);
> +#ifdef RTE_ARCH_ARM64
> +               asm volatile(PLT_CPU_FEATURE_PREAMBLE
> +                            "          ldxrb %w[lock_cnt], [%[lock]]   \n"
> +                            "          tst %w[lock_cnt], 255           \n"
> +                            "          beq dne%=                       \n"
> +                            "          sevl                            \n"
> +                            "rty%=:    wfe                             \n"
> +                            "          ldxrb %w[lock_cnt], [%[lock]]   \n"
> +                            "          tst %w[lock_cnt], 255           \n"
> +                            "          bne rty%=                       \n"
> +                            "dne%=:                                    \n"
> +                            : [lock_cnt] "=&r"(lock_cnt)
> +                            : [lock] "r"(&bkt->lock)
> +                            : "memory");
> +#else
> +               while (__atomic_load_n(&bkt->lock, __ATOMIC_RELAXED))
> +                       ;
> +#endif
> +               goto __retry;
> +       }
> +
>         chunk_remainder = cnxk_tim_bkt_fetch_rem(lock_sema);
>         rem = chunk_remainder - nb_timers;
>         if (rem < 0) {
> -               crem = tim_ring->nb_chunk_slots - chunk_remainder;
> -               if (chunk_remainder && crem) {
> +               if (chunk_remainder > 0) {
>                         chunk = ((struct cnxk_tim_ent *)
>                                          mirr_bkt->current_chunk) +
> -                               crem;
> +                               tim_ring->nb_chunk_slots - chunk_remainder;
>
>                         index = cnxk_tim_cpy_wrk(index, chunk_remainder, chunk,
>                                                  tim, ents, bkt);
> @@ -537,18 +539,19 @@ cnxk_tim_add_entry_brst(struct cnxk_tim_ring *const tim_ring,
>                         chunk = cnxk_tim_insert_chunk(bkt, mirr_bkt, tim_ring);
>
>                 if (unlikely(chunk == NULL)) {
> -                       cnxk_tim_bkt_dec_lock(bkt);
> +                       cnxk_tim_bkt_dec_lock_relaxed(bkt);
>                         rte_errno = ENOMEM;
>                         tim[index]->state = RTE_EVENT_TIMER_ERROR;
> -                       return crem;
> +                       return index;
>                 }
>                 *(uint64_t *)(chunk + tim_ring->nb_chunk_slots) = 0;
>                 mirr_bkt->current_chunk = (uintptr_t)chunk;
> -               cnxk_tim_cpy_wrk(index, nb_timers, chunk, tim, ents, bkt);
> +               index = cnxk_tim_cpy_wrk(index, nb_timers, chunk, tim, ents,
> +                                        bkt) -
> +                       index;
>
> -               rem = nb_timers - chunk_remainder;
> -               cnxk_tim_bkt_set_rem(bkt, tim_ring->nb_chunk_slots - rem);
> -               cnxk_tim_bkt_add_nent(bkt, rem);
> +               cnxk_tim_bkt_set_rem(bkt, tim_ring->nb_chunk_slots - index);
> +               cnxk_tim_bkt_add_nent(bkt, index);
>         } else {
>                 chunk = (struct cnxk_tim_ent *)mirr_bkt->current_chunk;
>                 chunk += (tim_ring->nb_chunk_slots - chunk_remainder);
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-02-07  9:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02  8:10 [PATCH 1/2] event/cnxk: fix burst timer arm routine pbhagavatula
2023-02-02  8:10 ` [PATCH 2/2] event/cnxk: update timer arm burst parameters pbhagavatula
2023-02-07  9:27 ` [PATCH 1/2] event/cnxk: fix burst timer arm routine Jerin Jacob

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.