* [PATCH 1/4] eventdev/timer: add periodic event timer support
@ 2022-08-03 16:25 Naga Harish K S V
2022-08-03 16:25 ` [PATCH 4/4] test/event: update periodic event timer tests Naga Harish K S V
2022-08-10 7:07 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
0 siblings, 2 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-03 16:25 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/eventdev/rte_event_timer_adapter.c | 86 ++++++++++++++++++--------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
3 files changed, 67 insertions(+), 27 deletions(-)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..f96cb98b5b 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_event_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,10 +203,11 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ ret = dev->dev_ops->timer_adapter_caps_get ?
+ dev->dev_ops->timer_adapter_caps_get(dev,
adapter->data->conf.flags,
&adapter->data->caps,
- &adapter->ops);
+ &adapter->ops) : 0;
if (ret < 0) {
rte_errno = -ret;
goto free_memzone;
@@ -348,10 +357,11 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ ret = dev->dev_ops->timer_adapter_caps_get ?
+ dev->dev_ops->timer_adapter_caps_get(dev,
adapter->data->conf.flags,
&adapter->data->caps,
- &adapter->ops);
+ &adapter->ops) : 0;
if (ret < 0) {
rte_errno = EINVAL;
return NULL;
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_event_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_event_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..4a2a1178da 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 4/4] test/event: update periodic event timer tests
2022-08-03 16:25 [PATCH 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
@ 2022-08-03 16:25 ` Naga Harish K S V
2022-08-10 7:07 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
1 sibling, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-03 16:25 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch updates the software timer adapter tests to
configure and use periodic event timers
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index d6170bb589..654c412836 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -386,11 +386,22 @@ timdev_setup_msec(void)
static int
timdev_setup_msec_periodic(void)
{
+ uint32_t caps = 0;
+ uint64_t max_tmo_ns;
+
uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ max_tmo_ns = 0;
+ else
+ max_tmo_ns = 180 * NSECPERSEC;
+
/* Periodic mode with 100 ms resolution */
- return _timdev_setup(0, NSECPERSEC / 10, flags);
+ return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
}
static int
@@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
/* Periodic mode with 1 sec resolution */
- return _timdev_setup(0, NSECPERSEC, flags);
+ return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
}
static int
@@ -561,12 +572,23 @@ test_timer_arm(void)
static inline int
test_timer_arm_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
}
@@ -649,12 +671,23 @@ test_timer_arm_burst(void)
static inline int
test_timer_arm_burst_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 1/4] eventdev/timer: add periodic event timer support
2022-08-03 16:25 [PATCH 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-08-03 16:25 ` [PATCH 4/4] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-08-10 7:07 ` Naga Harish K S V
2022-08-10 7:07 ` [PATCH v2 4/4] test/event: update periodic event timer tests Naga Harish K S V
` (2 more replies)
1 sibling, 3 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-10 7:07 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/eventdev/rte_event_timer_adapter.c | 86 ++++++++++++++++++--------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
3 files changed, 67 insertions(+), 27 deletions(-)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..0de88dfc0f 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_event_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,10 +203,11 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ ret = dev->dev_ops->timer_adapter_caps_get ?
+ dev->dev_ops->timer_adapter_caps_get(dev,
adapter->data->conf.flags,
&adapter->data->caps,
- &adapter->ops);
+ &adapter->ops) : 0;
if (ret < 0) {
rte_errno = -ret;
goto free_memzone;
@@ -348,10 +357,11 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ ret = dev->dev_ops->timer_adapter_caps_get ?
+ dev->dev_ops->timer_adapter_caps_get(dev,
adapter->data->conf.flags,
&adapter->data->caps,
- &adapter->ops);
+ &adapter->ops) : 0;
if (ret < 0) {
rte_errno = EINVAL;
return NULL;
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_event_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_event_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..4a2a1178da 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 4/4] test/event: update periodic event timer tests
2022-08-10 7:07 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
@ 2022-08-10 7:07 ` Naga Harish K S V
2022-08-10 19:55 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
2022-08-11 15:36 ` [PATCH v3 " Naga Harish K S V
2 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-10 7:07 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch updates the software timer adapter tests to
configure and use periodic event timers.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index d6170bb589..654c412836 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -386,11 +386,22 @@ timdev_setup_msec(void)
static int
timdev_setup_msec_periodic(void)
{
+ uint32_t caps = 0;
+ uint64_t max_tmo_ns;
+
uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ max_tmo_ns = 0;
+ else
+ max_tmo_ns = 180 * NSECPERSEC;
+
/* Periodic mode with 100 ms resolution */
- return _timdev_setup(0, NSECPERSEC / 10, flags);
+ return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
}
static int
@@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
/* Periodic mode with 1 sec resolution */
- return _timdev_setup(0, NSECPERSEC, flags);
+ return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
}
static int
@@ -561,12 +572,23 @@ test_timer_arm(void)
static inline int
test_timer_arm_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
}
@@ -649,12 +671,23 @@ test_timer_arm_burst(void)
static inline int
test_timer_arm_burst_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v2 1/4] eventdev/timer: add periodic event timer support
2022-08-10 7:07 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-08-10 7:07 ` [PATCH v2 4/4] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-08-10 19:55 ` Carrillo, Erik G
2022-08-11 15:43 ` Naga Harish K, S V
2022-08-11 15:36 ` [PATCH v3 " Naga Harish K S V
2 siblings, 1 reply; 41+ messages in thread
From: Carrillo, Erik G @ 2022-08-10 19:55 UTC (permalink / raw)
To: Naga Harish K, S V, jerinj; +Cc: pbhagavatula, sthotton, dev
Hi Harish,
> -----Original Message-----
> From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Sent: Wednesday, August 10, 2022 2:07 AM
> To: Carrillo, Erik G <erik.g.carrillo@intel.com>; jerinj@marvell.com
> Cc: pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> Subject: [PATCH v2 1/4] eventdev/timer: add periodic event timer support
>
> This patch adds support to configure and use periodic event timers in
> software timer adapter.
>
> The structure ``rte_event_timer_adapter_stats`` is extended by adding a
> new field, ``evtim_drop_count``. This stat represents the number of times an
> event_timer expiry event is dropped by the event timer adapter.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> ---
> lib/eventdev/rte_event_timer_adapter.c | 86 ++++++++++++++++++-------
> - lib/eventdev/rte_event_timer_adapter.h | 2 +
> lib/eventdev/rte_eventdev.c | 6 +-
> 3 files changed, 67 insertions(+), 27 deletions(-)
>
> diff --git a/lib/eventdev/rte_event_timer_adapter.c
> b/lib/eventdev/rte_event_timer_adapter.c
> index e0d978d641..0de88dfc0f 100644
> --- a/lib/eventdev/rte_event_timer_adapter.c
> +++ b/lib/eventdev/rte_event_timer_adapter.c
> @@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops
> swtim_ops; #define EVTIM_SVC_LOG_DBG(...) (void)0 #endif
>
> +static inline enum rte_timer_type
> +get_event_timer_type(const struct rte_event_timer_adapter *adapter) {
Let's call this function "get_timer_type" since it is selecting a type for an rte_timer.
> + return (adapter->data->conf.flags &
> + RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
> + PERIODICAL : SINGLE;
> +}
> +
> static int
> default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t
> *event_port_id,
> void *conf_arg)
> @@ -195,10 +203,11 @@ rte_event_timer_adapter_create_ext(
> adapter->data->conf = *conf; /* copy conf structure */
>
> /* Query eventdev PMD for timer adapter capabilities and ops */
> - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + ret = dev->dev_ops->timer_adapter_caps_get ?
> + dev->dev_ops-
> >timer_adapter_caps_get(dev,
> adapter->data->conf.flags,
> &adapter->data->caps,
> - &adapter->ops);
> + &adapter->ops) : 0;
> if (ret < 0) {
> rte_errno = -ret;
> goto free_memzone;
IMO, this hunk would read better as:
if (dev->dev_ops->timer_adapter_caps_get) {
ret = dev->dev_ops->timer_adapter_caps_get(dev,
adapter->data->conf.flags, &adapter->data->caps,
&adapter->ops);
if (ret < 0) {
rte_errno = -ret;
goto free_memzone;
}
}
> @@ -348,10 +357,11 @@ rte_event_timer_adapter_lookup(uint16_t
> adapter_id)
> dev = &rte_eventdevs[adapter->data->event_dev_id];
>
> /* Query eventdev PMD for timer adapter capabilities and ops */
> - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + ret = dev->dev_ops->timer_adapter_caps_get ?
> + dev->dev_ops->timer_adapter_caps_get(dev,
> adapter->data->conf.flags,
> &adapter->data->caps,
> - &adapter->ops);
> + &adapter->ops) : 0;
> if (ret < 0) {
> rte_errno = EINVAL;
> return NULL;
Same comment as above for this hunk...
Thanks,
Erik
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v3 1/4] eventdev/timer: add periodic event timer support
2022-08-10 7:07 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-08-10 7:07 ` [PATCH v2 4/4] test/event: update periodic event timer tests Naga Harish K S V
2022-08-10 19:55 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
@ 2022-08-11 15:36 ` Naga Harish K S V
2022-08-11 15:36 ` [PATCH v3 4/4] test/event: update periodic event timer tests Naga Harish K S V
` (2 more replies)
2 siblings, 3 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-11 15:36 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++---------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
3 files changed, 77 insertions(+), 37 deletions(-)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..d2480060c5 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = -ret;
- goto free_memzone;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = -ret;
+ goto free_memzone;
+ }
}
if (!(adapter->data->caps &
@@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = EINVAL;
- return NULL;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
}
/* If eventdev PMD did not provide ops, use default software
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..4a2a1178da 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v3 4/4] test/event: update periodic event timer tests
2022-08-11 15:36 ` [PATCH v3 " Naga Harish K S V
@ 2022-08-11 15:36 ` Naga Harish K S V
2022-08-11 19:22 ` [PATCH v3 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
2022-08-12 16:07 ` [PATCH v4 " Naga Harish K S V
2 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-11 15:36 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch updates the software timer adapter tests to
configure and use periodic event timers.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index d6170bb589..654c412836 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -386,11 +386,22 @@ timdev_setup_msec(void)
static int
timdev_setup_msec_periodic(void)
{
+ uint32_t caps = 0;
+ uint64_t max_tmo_ns;
+
uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ max_tmo_ns = 0;
+ else
+ max_tmo_ns = 180 * NSECPERSEC;
+
/* Periodic mode with 100 ms resolution */
- return _timdev_setup(0, NSECPERSEC / 10, flags);
+ return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
}
static int
@@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
/* Periodic mode with 1 sec resolution */
- return _timdev_setup(0, NSECPERSEC, flags);
+ return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
}
static int
@@ -561,12 +572,23 @@ test_timer_arm(void)
static inline int
test_timer_arm_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
}
@@ -649,12 +671,23 @@ test_timer_arm_burst(void)
static inline int
test_timer_arm_burst_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v2 1/4] eventdev/timer: add periodic event timer support
2022-08-10 19:55 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
@ 2022-08-11 15:43 ` Naga Harish K, S V
0 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K, S V @ 2022-08-11 15:43 UTC (permalink / raw)
To: Carrillo, Erik G, jerinj; +Cc: pbhagavatula, sthotton, dev
Hi Gabe,
> -----Original Message-----
> From: Carrillo, Erik G <erik.g.carrillo@intel.com>
> Sent: Thursday, August 11, 2022 1:25 AM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; jerinj@marvell.com
> Cc: pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> Subject: RE: [PATCH v2 1/4] eventdev/timer: add periodic event timer
> support
>
> Hi Harish,
>
> > -----Original Message-----
> > From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Sent: Wednesday, August 10, 2022 2:07 AM
> > To: Carrillo, Erik G <erik.g.carrillo@intel.com>; jerinj@marvell.com
> > Cc: pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> > Subject: [PATCH v2 1/4] eventdev/timer: add periodic event timer
> > support
> >
> > This patch adds support to configure and use periodic event timers in
> > software timer adapter.
> >
> > The structure ``rte_event_timer_adapter_stats`` is extended by adding
> > a new field, ``evtim_drop_count``. This stat represents the number of
> > times an event_timer expiry event is dropped by the event timer adapter.
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > ---
> > lib/eventdev/rte_event_timer_adapter.c | 86 ++++++++++++++++++-----
> --
> > - lib/eventdev/rte_event_timer_adapter.h | 2 +
> > lib/eventdev/rte_eventdev.c | 6 +-
> > 3 files changed, 67 insertions(+), 27 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_event_timer_adapter.c
> > b/lib/eventdev/rte_event_timer_adapter.c
> > index e0d978d641..0de88dfc0f 100644
> > --- a/lib/eventdev/rte_event_timer_adapter.c
> > +++ b/lib/eventdev/rte_event_timer_adapter.c
> > @@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops
> > swtim_ops; #define EVTIM_SVC_LOG_DBG(...) (void)0 #endif
> >
> > +static inline enum rte_timer_type
> > +get_event_timer_type(const struct rte_event_timer_adapter *adapter) {
>
> Let's call this function "get_timer_type" since it is selecting a type for an
> rte_timer.
>
Taken in v3 version of the patch
> > + return (adapter->data->conf.flags &
> > + RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
> > + PERIODICAL : SINGLE;
> > +}
> > +
> > static int
> > default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t
> > *event_port_id,
> > void *conf_arg)
> > @@ -195,10 +203,11 @@ rte_event_timer_adapter_create_ext(
> > adapter->data->conf = *conf; /* copy conf structure */
> >
> > /* Query eventdev PMD for timer adapter capabilities and ops */
> > - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> > + ret = dev->dev_ops->timer_adapter_caps_get ?
> > + dev->dev_ops-
> > >timer_adapter_caps_get(dev,
> > adapter->data->conf.flags,
> > &adapter->data->caps,
> > - &adapter->ops);
> > + &adapter->ops) : 0;
> > if (ret < 0) {
> > rte_errno = -ret;
> > goto free_memzone;
>
> IMO, this hunk would read better as:
>
> if (dev->dev_ops->timer_adapter_caps_get) {
> ret = dev->dev_ops->timer_adapter_caps_get(dev,
> adapter->data->conf.flags, &adapter->data->caps,
> &adapter->ops);
> if (ret < 0) {
> rte_errno = -ret;
> goto free_memzone;
> }
> }
>
Taken in v3 version of the patch
> > @@ -348,10 +357,11 @@ rte_event_timer_adapter_lookup(uint16_t
> > adapter_id)
> > dev = &rte_eventdevs[adapter->data->event_dev_id];
> >
> > /* Query eventdev PMD for timer adapter capabilities and ops */
> > - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> > + ret = dev->dev_ops->timer_adapter_caps_get ?
> > + dev->dev_ops->timer_adapter_caps_get(dev,
> > adapter->data->conf.flags,
> > &adapter->data->caps,
> > - &adapter->ops);
> > + &adapter->ops) : 0;
> > if (ret < 0) {
> > rte_errno = EINVAL;
> > return NULL;
>
> Same comment as above for this hunk...
>
> Thanks,
> Erik
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 1/4] eventdev/timer: add periodic event timer support
2022-08-11 15:36 ` [PATCH v3 " Naga Harish K S V
2022-08-11 15:36 ` [PATCH v3 4/4] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-08-11 19:22 ` Carrillo, Erik G
2022-08-12 16:10 ` Naga Harish K, S V
2022-08-12 16:07 ` [PATCH v4 " Naga Harish K S V
2 siblings, 1 reply; 41+ messages in thread
From: Carrillo, Erik G @ 2022-08-11 19:22 UTC (permalink / raw)
To: Naga Harish K, S V, jerinj; +Cc: pbhagavatula, sthotton, dev
Hi Harish,
> -----Original Message-----
> From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Sent: Thursday, August 11, 2022 10:37 AM
> To: Carrillo, Erik G <erik.g.carrillo@intel.com>; jerinj@marvell.com
> Cc: pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> Subject: [PATCH v3 1/4] eventdev/timer: add periodic event timer support
>
> This patch adds support to configure and use periodic event timers in
> software timer adapter.
>
> The structure ``rte_event_timer_adapter_stats`` is extended by adding a
> new field, ``evtim_drop_count``. This stat represents the number of times an
> event_timer expiry event is dropped by the event timer adapter.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> ---
<... snipped ...>
> diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
> index 1dc4f966be..4a2a1178da 100644
> --- a/lib/eventdev/rte_eventdev.c
> +++ b/lib/eventdev/rte_eventdev.c
> @@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id,
> uint32_t *caps)
>
> if (caps == NULL)
> return -EINVAL;
> - *caps = 0;
> +
> + if (dev->dev_ops->timer_adapter_caps_get == NULL)
> + *caps = RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC;
I think we should move the definition of RTE_EVENT_TIMER_ADAPTER_SW_CAP to this patch, and use that macro here as well. With that change, this looks good to me.
Thanks,
Erik
> + else
> + *caps = 0;
>
> return dev->dev_ops->timer_adapter_caps_get ?
> (*dev->dev_ops-
> >timer_adapter_caps_get)(dev,
> --
> 2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v4 1/4] eventdev/timer: add periodic event timer support
2022-08-11 15:36 ` [PATCH v3 " Naga Harish K S V
2022-08-11 15:36 ` [PATCH v3 4/4] test/event: update periodic event timer tests Naga Harish K S V
2022-08-11 19:22 ` [PATCH v3 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
@ 2022-08-12 16:07 ` Naga Harish K S V
2022-08-12 16:07 ` [PATCH v4 4/4] test/event: update periodic event timer tests Naga Harish K S V
` (3 more replies)
2 siblings, 4 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-12 16:07 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/eventdev/eventdev_pmd.h | 3 +
lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++---------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
4 files changed, 80 insertions(+), 37 deletions(-)
diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index 69402668d8..d9e71581b7 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -81,6 +81,9 @@ extern "C" {
* the ethdev to eventdev use a SW service function
*/
+#define RTE_EVENT_TIMER_ADAPTER_SW_CAP \
+ RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC
+
#define RTE_EVENTDEV_DETACHED (0)
#define RTE_EVENTDEV_ATTACHED (1)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..d2480060c5 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = -ret;
- goto free_memzone;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = -ret;
+ goto free_memzone;
+ }
}
if (!(adapter->data->caps &
@@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = EINVAL;
- return NULL;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
}
/* If eventdev PMD did not provide ops, use default software
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..59d8b49ef6 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v4 4/4] test/event: update periodic event timer tests
2022-08-12 16:07 ` [PATCH v4 " Naga Harish K S V
@ 2022-08-12 16:07 ` Naga Harish K S V
2022-08-18 13:13 ` Carrillo, Erik G
2022-08-18 13:06 ` [PATCH v4 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
` (2 subsequent siblings)
3 siblings, 1 reply; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-12 16:07 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch updates the software timer adapter tests to
configure and use periodic event timers.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index d6170bb589..654c412836 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -386,11 +386,22 @@ timdev_setup_msec(void)
static int
timdev_setup_msec_periodic(void)
{
+ uint32_t caps = 0;
+ uint64_t max_tmo_ns;
+
uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ max_tmo_ns = 0;
+ else
+ max_tmo_ns = 180 * NSECPERSEC;
+
/* Periodic mode with 100 ms resolution */
- return _timdev_setup(0, NSECPERSEC / 10, flags);
+ return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
}
static int
@@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
/* Periodic mode with 1 sec resolution */
- return _timdev_setup(0, NSECPERSEC, flags);
+ return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
}
static int
@@ -561,12 +572,23 @@ test_timer_arm(void)
static inline int
test_timer_arm_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
}
@@ -649,12 +671,23 @@ test_timer_arm_burst(void)
static inline int
test_timer_arm_burst_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 1/4] eventdev/timer: add periodic event timer support
2022-08-11 19:22 ` [PATCH v3 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
@ 2022-08-12 16:10 ` Naga Harish K, S V
0 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K, S V @ 2022-08-12 16:10 UTC (permalink / raw)
To: Carrillo, Erik G, jerinj; +Cc: pbhagavatula, sthotton, dev
Hi Gabe,
> -----Original Message-----
> From: Carrillo, Erik G <erik.g.carrillo@intel.com>
> Sent: Friday, August 12, 2022 12:52 AM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; jerinj@marvell.com
> Cc: pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> Subject: RE: [PATCH v3 1/4] eventdev/timer: add periodic event timer
> support
>
> Hi Harish,
>
> > -----Original Message-----
> > From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Sent: Thursday, August 11, 2022 10:37 AM
> > To: Carrillo, Erik G <erik.g.carrillo@intel.com>; jerinj@marvell.com
> > Cc: pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> > Subject: [PATCH v3 1/4] eventdev/timer: add periodic event timer
> > support
> >
> > This patch adds support to configure and use periodic event timers in
> > software timer adapter.
> >
> > The structure ``rte_event_timer_adapter_stats`` is extended by adding
> > a new field, ``evtim_drop_count``. This stat represents the number of
> > times an event_timer expiry event is dropped by the event timer adapter.
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > ---
>
> <... snipped ...>
>
> > diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
> > index 1dc4f966be..4a2a1178da 100644
> > --- a/lib/eventdev/rte_eventdev.c
> > +++ b/lib/eventdev/rte_eventdev.c
> > @@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t
> dev_id,
> > uint32_t *caps)
> >
> > if (caps == NULL)
> > return -EINVAL;
> > - *caps = 0;
> > +
> > + if (dev->dev_ops->timer_adapter_caps_get == NULL)
> > + *caps = RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC;
>
> I think we should move the definition of
> RTE_EVENT_TIMER_ADAPTER_SW_CAP to this patch, and use that macro
> here as well. With that change, this looks good to me.
>
Updated the same in v4 version of the patchset
> Thanks,
> Erik
>
> > + else
> > + *caps = 0;
> >
> > return dev->dev_ops->timer_adapter_caps_get ?
> > (*dev->dev_ops-
> > >timer_adapter_caps_get)(dev,
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v4 1/4] eventdev/timer: add periodic event timer support
2022-08-12 16:07 ` [PATCH v4 " Naga Harish K S V
2022-08-12 16:07 ` [PATCH v4 4/4] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-08-18 13:06 ` Carrillo, Erik G
2022-09-14 5:08 ` [PATCH v5 " Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
3 siblings, 0 replies; 41+ messages in thread
From: Carrillo, Erik G @ 2022-08-18 13:06 UTC (permalink / raw)
To: Naga Harish K, S V, jerinj; +Cc: pbhagavatula, sthotton, dev
> -----Original Message-----
> From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Sent: Friday, August 12, 2022 11:07 AM
> To: Carrillo, Erik G <erik.g.carrillo@intel.com>; jerinj@marvell.com
> Cc: pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> Subject: [PATCH v4 1/4] eventdev/timer: add periodic event timer support
>
> This patch adds support to configure and use periodic event timers in
> software timer adapter.
>
> The structure ``rte_event_timer_adapter_stats`` is extended by adding a
> new field, ``evtim_drop_count``. This stat represents the number of times an
> event_timer expiry event is dropped by the event timer adapter.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v4 4/4] test/event: update periodic event timer tests
2022-08-12 16:07 ` [PATCH v4 4/4] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-08-18 13:13 ` Carrillo, Erik G
0 siblings, 0 replies; 41+ messages in thread
From: Carrillo, Erik G @ 2022-08-18 13:13 UTC (permalink / raw)
To: Naga Harish K, S V, jerinj; +Cc: pbhagavatula, sthotton, dev
> -----Original Message-----
> From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Sent: Friday, August 12, 2022 11:07 AM
> To: Carrillo, Erik G <erik.g.carrillo@intel.com>; jerinj@marvell.com
> Cc: pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> Subject: [PATCH v4 4/4] test/event: update periodic event timer tests
>
> This patch updates the software timer adapter tests to configure and use
> periodic event timers.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 1/4] eventdev/timer: add periodic event timer support
2022-08-12 16:07 ` [PATCH v4 " Naga Harish K S V
2022-08-12 16:07 ` [PATCH v4 4/4] test/event: update periodic event timer tests Naga Harish K S V
2022-08-18 13:06 ` [PATCH v4 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
@ 2022-09-14 5:08 ` Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 2/4] timer: fix function to stop all timers Naga Harish K S V
` (2 more replies)
2022-09-14 5:15 ` [PATCH v5 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
3 siblings, 3 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 5:08 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
update the software eventdev pmd timer_adapter_caps_get
callback function to report the support of periodic
event timer capability
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
v5:
* squash first two patches and remove evtim_drop_count deprecation
notice
---
drivers/event/sw/sw_evdev.c | 2 +-
lib/eventdev/eventdev_pmd.h | 3 +
lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++---------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
5 files changed, 81 insertions(+), 38 deletions(-)
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index f93313b31b..6eddf8bd93 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -564,7 +564,7 @@ sw_timer_adapter_caps_get(const struct rte_eventdev *dev, uint64_t flags,
{
RTE_SET_USED(dev);
RTE_SET_USED(flags);
- *caps = 0;
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
/* Use default SW ops */
*ops = NULL;
diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index 69402668d8..d9e71581b7 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -81,6 +81,9 @@ extern "C" {
* the ethdev to eventdev use a SW service function
*/
+#define RTE_EVENT_TIMER_ADAPTER_SW_CAP \
+ RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC
+
#define RTE_EVENTDEV_DETACHED (0)
#define RTE_EVENTDEV_ATTACHED (1)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..d2480060c5 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = -ret;
- goto free_memzone;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = -ret;
+ goto free_memzone;
+ }
}
if (!(adapter->data->caps &
@@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = EINVAL;
- return NULL;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
}
/* If eventdev PMD did not provide ops, use default software
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..59d8b49ef6 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 2/4] timer: fix function to stop all timers
2022-09-14 5:08 ` [PATCH v5 " Naga Harish K S V
@ 2022-09-14 5:08 ` Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 3/4] test/event: update periodic event timer tests Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 4/4] doc: remove deprication notice Naga Harish K S V
2 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 5:08 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton, stable
There is a possibility of deadlock in this API,
as same spinlock is tried to be acquired in nested manner.
If the lcore that is stopping the timer is different from the lcore
that owns the timer, the timer list lock is acquired in timer_del(),
even if local_is_locked is true. Because the same lock was already
acquired in rte_timer_stop_all(), the thread will hang.
This patch removes the acquisition of nested lock.
Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a list")
Cc: stable@dpdk.org
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/timer/rte_timer.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/lib/timer/rte_timer.c b/lib/timer/rte_timer.c
index 9994813d0d..85d67573eb 100644
--- a/lib/timer/rte_timer.c
+++ b/lib/timer/rte_timer.c
@@ -580,7 +580,7 @@ rte_timer_reset_sync(struct rte_timer *tim, uint64_t ticks,
}
static int
-__rte_timer_stop(struct rte_timer *tim, int local_is_locked,
+__rte_timer_stop(struct rte_timer *tim,
struct rte_timer_data *timer_data)
{
union rte_timer_status prev_status, status;
@@ -602,7 +602,7 @@ __rte_timer_stop(struct rte_timer *tim, int local_is_locked,
/* remove it from list */
if (prev_status.state == RTE_TIMER_PENDING) {
- timer_del(tim, prev_status, local_is_locked, priv_timer);
+ timer_del(tim, prev_status, 0, priv_timer);
__TIMER_STAT_ADD(priv_timer, pending, -1);
}
@@ -631,7 +631,7 @@ rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim)
TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, timer_data, -EINVAL);
- return __rte_timer_stop(tim, 0, timer_data);
+ return __rte_timer_stop(tim, timer_data);
}
/* loop until rte_timer_stop() succeed */
@@ -987,21 +987,16 @@ rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
walk_lcore = walk_lcores[i];
priv_timer = &timer_data->priv_timer[walk_lcore];
- rte_spinlock_lock(&priv_timer->list_lock);
-
for (tim = priv_timer->pending_head.sl_next[0];
tim != NULL;
tim = next_tim) {
next_tim = tim->sl_next[0];
- /* Call timer_stop with lock held */
- __rte_timer_stop(tim, 1, timer_data);
+ __rte_timer_stop(tim, timer_data);
if (f)
f(tim, f_arg);
}
-
- rte_spinlock_unlock(&priv_timer->list_lock);
}
return 0;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 3/4] test/event: update periodic event timer tests
2022-09-14 5:08 ` [PATCH v5 " Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 2/4] timer: fix function to stop all timers Naga Harish K S V
@ 2022-09-14 5:08 ` Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 4/4] doc: remove deprication notice Naga Harish K S V
2 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 5:08 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch updates the software timer adapter tests to
configure and use periodic event timers.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index d6170bb589..654c412836 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -386,11 +386,22 @@ timdev_setup_msec(void)
static int
timdev_setup_msec_periodic(void)
{
+ uint32_t caps = 0;
+ uint64_t max_tmo_ns;
+
uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ max_tmo_ns = 0;
+ else
+ max_tmo_ns = 180 * NSECPERSEC;
+
/* Periodic mode with 100 ms resolution */
- return _timdev_setup(0, NSECPERSEC / 10, flags);
+ return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
}
static int
@@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
/* Periodic mode with 1 sec resolution */
- return _timdev_setup(0, NSECPERSEC, flags);
+ return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
}
static int
@@ -561,12 +572,23 @@ test_timer_arm(void)
static inline int
test_timer_arm_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
}
@@ -649,12 +671,23 @@ test_timer_arm_burst(void)
static inline int
test_timer_arm_burst_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 4/4] doc: remove deprication notice
2022-09-14 5:08 ` [PATCH v5 " Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 2/4] timer: fix function to stop all timers Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 3/4] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-09-14 5:08 ` Naga Harish K S V
2 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 5:08 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch removes event timer expiry drop stat deprication
notification.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
doc/guides/rel_notes/deprecation.rst | 7 -------
1 file changed, 7 deletions(-)
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index e7583cae4c..fd8ef4dff7 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -186,13 +186,6 @@ Deprecation Notices
Event will be one of the configuration fields,
together with additional vector parameters.
-* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
- extended by adding a new field ``evtim_drop_count``.
- This counter will represent the number of times an event_timer expiry event
- is dropped by the timer adapter.
- This field will be used to add periodic mode support
- to the software timer adapter in DPDK 22.11.
-
* eventdev: The function pointer declaration ``eventdev_stop_flush_t``
will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 1/4] eventdev/timer: add periodic event timer support
2022-08-12 16:07 ` [PATCH v4 " Naga Harish K S V
` (2 preceding siblings ...)
2022-09-14 5:08 ` [PATCH v5 " Naga Harish K S V
@ 2022-09-14 5:15 ` Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 2/4] timer: fix function to stop all timers Naga Harish K S V
` (3 more replies)
3 siblings, 4 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 5:15 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
update the software eventdev pmd timer_adapter_caps_get
callback function to report the support of periodic
event timer capability
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
v5:
* squash first two patches and remove evtim_drop_count deprecation
notice
---
drivers/event/sw/sw_evdev.c | 2 +-
lib/eventdev/eventdev_pmd.h | 3 +
lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++---------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
5 files changed, 81 insertions(+), 38 deletions(-)
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index f93313b31b..6eddf8bd93 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -564,7 +564,7 @@ sw_timer_adapter_caps_get(const struct rte_eventdev *dev, uint64_t flags,
{
RTE_SET_USED(dev);
RTE_SET_USED(flags);
- *caps = 0;
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
/* Use default SW ops */
*ops = NULL;
diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index 69402668d8..d9e71581b7 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -81,6 +81,9 @@ extern "C" {
* the ethdev to eventdev use a SW service function
*/
+#define RTE_EVENT_TIMER_ADAPTER_SW_CAP \
+ RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC
+
#define RTE_EVENTDEV_DETACHED (0)
#define RTE_EVENTDEV_ATTACHED (1)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..d2480060c5 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = -ret;
- goto free_memzone;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = -ret;
+ goto free_memzone;
+ }
}
if (!(adapter->data->caps &
@@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = EINVAL;
- return NULL;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
}
/* If eventdev PMD did not provide ops, use default software
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..59d8b49ef6 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 2/4] timer: fix function to stop all timers
2022-09-14 5:15 ` [PATCH v5 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
@ 2022-09-14 5:15 ` Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 3/4] test/event: update periodic event timer tests Naga Harish K S V
` (2 subsequent siblings)
3 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 5:15 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton, stable
There is a possibility of deadlock in this API,
as same spinlock is tried to be acquired in nested manner.
If the lcore that is stopping the timer is different from the lcore
that owns the timer, the timer list lock is acquired in timer_del(),
even if local_is_locked is true. Because the same lock was already
acquired in rte_timer_stop_all(), the thread will hang.
This patch removes the acquisition of nested lock.
Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a list")
Cc: stable@dpdk.org
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/timer/rte_timer.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/lib/timer/rte_timer.c b/lib/timer/rte_timer.c
index 9994813d0d..85d67573eb 100644
--- a/lib/timer/rte_timer.c
+++ b/lib/timer/rte_timer.c
@@ -580,7 +580,7 @@ rte_timer_reset_sync(struct rte_timer *tim, uint64_t ticks,
}
static int
-__rte_timer_stop(struct rte_timer *tim, int local_is_locked,
+__rte_timer_stop(struct rte_timer *tim,
struct rte_timer_data *timer_data)
{
union rte_timer_status prev_status, status;
@@ -602,7 +602,7 @@ __rte_timer_stop(struct rte_timer *tim, int local_is_locked,
/* remove it from list */
if (prev_status.state == RTE_TIMER_PENDING) {
- timer_del(tim, prev_status, local_is_locked, priv_timer);
+ timer_del(tim, prev_status, 0, priv_timer);
__TIMER_STAT_ADD(priv_timer, pending, -1);
}
@@ -631,7 +631,7 @@ rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim)
TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, timer_data, -EINVAL);
- return __rte_timer_stop(tim, 0, timer_data);
+ return __rte_timer_stop(tim, timer_data);
}
/* loop until rte_timer_stop() succeed */
@@ -987,21 +987,16 @@ rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
walk_lcore = walk_lcores[i];
priv_timer = &timer_data->priv_timer[walk_lcore];
- rte_spinlock_lock(&priv_timer->list_lock);
-
for (tim = priv_timer->pending_head.sl_next[0];
tim != NULL;
tim = next_tim) {
next_tim = tim->sl_next[0];
- /* Call timer_stop with lock held */
- __rte_timer_stop(tim, 1, timer_data);
+ __rte_timer_stop(tim, timer_data);
if (f)
f(tim, f_arg);
}
-
- rte_spinlock_unlock(&priv_timer->list_lock);
}
return 0;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 3/4] test/event: update periodic event timer tests
2022-09-14 5:15 ` [PATCH v5 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 2/4] timer: fix function to stop all timers Naga Harish K S V
@ 2022-09-14 5:15 ` Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 4/4] doc: remove deprecation notice Naga Harish K S V
2022-09-14 13:51 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Naga Harish K S V
3 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 5:15 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch updates the software timer adapter tests to
configure and use periodic event timers.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index d6170bb589..654c412836 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -386,11 +386,22 @@ timdev_setup_msec(void)
static int
timdev_setup_msec_periodic(void)
{
+ uint32_t caps = 0;
+ uint64_t max_tmo_ns;
+
uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ max_tmo_ns = 0;
+ else
+ max_tmo_ns = 180 * NSECPERSEC;
+
/* Periodic mode with 100 ms resolution */
- return _timdev_setup(0, NSECPERSEC / 10, flags);
+ return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
}
static int
@@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
/* Periodic mode with 1 sec resolution */
- return _timdev_setup(0, NSECPERSEC, flags);
+ return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
}
static int
@@ -561,12 +572,23 @@ test_timer_arm(void)
static inline int
test_timer_arm_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
}
@@ -649,12 +671,23 @@ test_timer_arm_burst(void)
static inline int
test_timer_arm_burst_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 4/4] doc: remove deprecation notice
2022-09-14 5:15 ` [PATCH v5 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 2/4] timer: fix function to stop all timers Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 3/4] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-09-14 5:15 ` Naga Harish K S V
2022-09-14 12:41 ` Jerin Jacob
2022-09-14 13:51 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Naga Harish K S V
3 siblings, 1 reply; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 5:15 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch removes event timer expiry drop stat deprecation
notification.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
doc/guides/rel_notes/deprecation.rst | 7 -------
1 file changed, 7 deletions(-)
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index e7583cae4c..fd8ef4dff7 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -186,13 +186,6 @@ Deprecation Notices
Event will be one of the configuration fields,
together with additional vector parameters.
-* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
- extended by adding a new field ``evtim_drop_count``.
- This counter will represent the number of times an event_timer expiry event
- is dropped by the timer adapter.
- This field will be used to add periodic mode support
- to the software timer adapter in DPDK 22.11.
-
* eventdev: The function pointer declaration ``eventdev_stop_flush_t``
will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5 4/4] doc: remove deprecation notice
2022-09-14 5:15 ` [PATCH v5 4/4] doc: remove deprecation notice Naga Harish K S V
@ 2022-09-14 12:41 ` Jerin Jacob
2022-09-14 13:54 ` Naga Harish K, S V
0 siblings, 1 reply; 41+ messages in thread
From: Jerin Jacob @ 2022-09-14 12:41 UTC (permalink / raw)
To: Naga Harish K S V; +Cc: jerinj, dev, erik.g.carrillo, pbhagavatula, sthotton
On Wed, Sep 14, 2022 at 10:46 AM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> This patch removes event timer expiry drop stat deprecation
> notification.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> ---
> doc/guides/rel_notes/deprecation.rst | 7 -------
Please squash this patch with 1/5.
Also, Please update "ABI Changes" in doc/guides/rel_notes/release_22_11.rst.
See https://patches.dpdk.org/project/dpdk/patch/dcd3cf0ec034632f97223bb9df389f9cedf9753c.1660116951.git.sthotton@marvell.com/
as example.
> 1 file changed, 7 deletions(-)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index e7583cae4c..fd8ef4dff7 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -186,13 +186,6 @@ Deprecation Notices
> Event will be one of the configuration fields,
> together with additional vector parameters.
>
> -* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
> - extended by adding a new field ``evtim_drop_count``.
> - This counter will represent the number of times an event_timer expiry event
> - is dropped by the timer adapter.
> - This field will be used to add periodic mode support
> - to the software timer adapter in DPDK 22.11.
> -
> * eventdev: The function pointer declaration ``eventdev_stop_flush_t``
> will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 1/3] eventdev/timer: add periodic event timer support
2022-09-14 5:15 ` [PATCH v5 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
` (2 preceding siblings ...)
2022-09-14 5:15 ` [PATCH v5 4/4] doc: remove deprecation notice Naga Harish K S V
@ 2022-09-14 13:51 ` Naga Harish K S V
2022-09-14 13:51 ` [PATCH v6 2/3] timer: fix function to stop all timers Naga Harish K S V
` (3 more replies)
3 siblings, 4 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 13:51 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
update the software eventdev pmd timer_adapter_caps_get
callback function to report the support of periodic
event timer capability
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
doc/guides/rel_notes/deprecation.rst | 7 --
doc/guides/rel_notes/release_22_11.rst | 2 +
drivers/event/sw/sw_evdev.c | 2 +-
lib/eventdev/eventdev_pmd.h | 3 +
lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++---------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
7 files changed, 83 insertions(+), 45 deletions(-)
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index e7583cae4c..fd8ef4dff7 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -186,13 +186,6 @@ Deprecation Notices
Event will be one of the configuration fields,
together with additional vector parameters.
-* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
- extended by adding a new field ``evtim_drop_count``.
- This counter will represent the number of times an event_timer expiry event
- is dropped by the timer adapter.
- This field will be used to add periodic mode support
- to the software timer adapter in DPDK 22.11.
-
* eventdev: The function pointer declaration ``eventdev_stop_flush_t``
will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
index c32c18ff49..976a74b054 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -94,6 +94,8 @@ API Changes
ABI Changes
-----------
+* eventdev: Added ``evtim_drop_count`` field to ``rte_event_timer_adapter_stats``
+ structure.
.. This section should contain ABI changes. Sample format:
* sample: Add a short 1-2 sentence description of the ABI change
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index c9efe35bf4..e866150379 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -564,7 +564,7 @@ sw_timer_adapter_caps_get(const struct rte_eventdev *dev, uint64_t flags,
{
RTE_SET_USED(dev);
RTE_SET_USED(flags);
- *caps = 0;
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
/* Use default SW ops */
*ops = NULL;
diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index f514a37575..085563ff52 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -81,6 +81,9 @@ extern "C" {
* the ethdev to eventdev use a SW service function
*/
+#define RTE_EVENT_TIMER_ADAPTER_SW_CAP \
+ RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC
+
#define RTE_EVENTDEV_DETACHED (0)
#define RTE_EVENTDEV_ATTACHED (1)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..d2480060c5 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = -ret;
- goto free_memzone;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = -ret;
+ goto free_memzone;
+ }
}
if (!(adapter->data->caps &
@@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = EINVAL;
- return NULL;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
}
/* If eventdev PMD did not provide ops, use default software
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..59d8b49ef6 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 2/3] timer: fix function to stop all timers
2022-09-14 13:51 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Naga Harish K S V
@ 2022-09-14 13:51 ` Naga Harish K S V
2022-09-14 13:51 ` [PATCH v6 3/3] test/event: update periodic event timer tests Naga Harish K S V
` (2 subsequent siblings)
3 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 13:51 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton, stable
There is a possibility of deadlock in this API,
as same spinlock is tried to be acquired in nested manner.
If the lcore that is stopping the timer is different from the lcore
that owns the timer, the timer list lock is acquired in timer_del(),
even if local_is_locked is true. Because the same lock was already
acquired in rte_timer_stop_all(), the thread will hang.
This patch removes the acquisition of nested lock.
Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a list")
Cc: stable@dpdk.org
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/timer/rte_timer.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/lib/timer/rte_timer.c b/lib/timer/rte_timer.c
index 9994813d0d..85d67573eb 100644
--- a/lib/timer/rte_timer.c
+++ b/lib/timer/rte_timer.c
@@ -580,7 +580,7 @@ rte_timer_reset_sync(struct rte_timer *tim, uint64_t ticks,
}
static int
-__rte_timer_stop(struct rte_timer *tim, int local_is_locked,
+__rte_timer_stop(struct rte_timer *tim,
struct rte_timer_data *timer_data)
{
union rte_timer_status prev_status, status;
@@ -602,7 +602,7 @@ __rte_timer_stop(struct rte_timer *tim, int local_is_locked,
/* remove it from list */
if (prev_status.state == RTE_TIMER_PENDING) {
- timer_del(tim, prev_status, local_is_locked, priv_timer);
+ timer_del(tim, prev_status, 0, priv_timer);
__TIMER_STAT_ADD(priv_timer, pending, -1);
}
@@ -631,7 +631,7 @@ rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim)
TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, timer_data, -EINVAL);
- return __rte_timer_stop(tim, 0, timer_data);
+ return __rte_timer_stop(tim, timer_data);
}
/* loop until rte_timer_stop() succeed */
@@ -987,21 +987,16 @@ rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
walk_lcore = walk_lcores[i];
priv_timer = &timer_data->priv_timer[walk_lcore];
- rte_spinlock_lock(&priv_timer->list_lock);
-
for (tim = priv_timer->pending_head.sl_next[0];
tim != NULL;
tim = next_tim) {
next_tim = tim->sl_next[0];
- /* Call timer_stop with lock held */
- __rte_timer_stop(tim, 1, timer_data);
+ __rte_timer_stop(tim, timer_data);
if (f)
f(tim, f_arg);
}
-
- rte_spinlock_unlock(&priv_timer->list_lock);
}
return 0;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v6 3/3] test/event: update periodic event timer tests
2022-09-14 13:51 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-09-14 13:51 ` [PATCH v6 2/3] timer: fix function to stop all timers Naga Harish K S V
@ 2022-09-14 13:51 ` Naga Harish K S V
2022-09-14 15:24 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Jerin Jacob
2022-09-14 15:33 ` [PATCH v7 " Naga Harish K S V
3 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 13:51 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch updates the software timer adapter tests to
configure and use periodic event timers.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index d6170bb589..654c412836 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -386,11 +386,22 @@ timdev_setup_msec(void)
static int
timdev_setup_msec_periodic(void)
{
+ uint32_t caps = 0;
+ uint64_t max_tmo_ns;
+
uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ max_tmo_ns = 0;
+ else
+ max_tmo_ns = 180 * NSECPERSEC;
+
/* Periodic mode with 100 ms resolution */
- return _timdev_setup(0, NSECPERSEC / 10, flags);
+ return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
}
static int
@@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
/* Periodic mode with 1 sec resolution */
- return _timdev_setup(0, NSECPERSEC, flags);
+ return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
}
static int
@@ -561,12 +572,23 @@ test_timer_arm(void)
static inline int
test_timer_arm_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
}
@@ -649,12 +671,23 @@ test_timer_arm_burst(void)
static inline int
test_timer_arm_burst_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v5 4/4] doc: remove deprecation notice
2022-09-14 12:41 ` Jerin Jacob
@ 2022-09-14 13:54 ` Naga Harish K, S V
0 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K, S V @ 2022-09-14 13:54 UTC (permalink / raw)
To: Jerin Jacob; +Cc: jerinj, dev, Carrillo, Erik G, pbhagavatula, sthotton
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, September 14, 2022 6:11 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; dev@dpdk.org; Carrillo, Erik G
> <erik.g.carrillo@intel.com>; pbhagavatula@marvell.com;
> sthotton@marvell.com
> Subject: Re: [PATCH v5 4/4] doc: remove deprecation notice
>
> On Wed, Sep 14, 2022 at 10:46 AM Naga Harish K S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > This patch removes event timer expiry drop stat deprecation
> > notification.
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > ---
> > doc/guides/rel_notes/deprecation.rst | 7 -------
>
> Please squash this patch with 1/5.
> Also, Please update "ABI Changes" in
> doc/guides/rel_notes/release_22_11.rst.
> See
> https://patches.dpdk.org/project/dpdk/patch/dcd3cf0ec034632f97223bb9df
> 389f9cedf9753c.1660116951.git.sthotton@marvell.com/
> as example.
>
>
V6 version is posted with requested changes.
> > 1 file changed, 7 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index e7583cae4c..fd8ef4dff7 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -186,13 +186,6 @@ Deprecation Notices
> > Event will be one of the configuration fields,
> > together with additional vector parameters.
> >
> > -* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
> > - extended by adding a new field ``evtim_drop_count``.
> > - This counter will represent the number of times an event_timer
> > expiry event
> > - is dropped by the timer adapter.
> > - This field will be used to add periodic mode support
> > - to the software timer adapter in DPDK 22.11.
> > -
> > * eventdev: The function pointer declaration ``eventdev_stop_flush_t``
> > will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
> >
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6 1/3] eventdev/timer: add periodic event timer support
2022-09-14 13:51 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-09-14 13:51 ` [PATCH v6 2/3] timer: fix function to stop all timers Naga Harish K S V
2022-09-14 13:51 ` [PATCH v6 3/3] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-09-14 15:24 ` Jerin Jacob
2022-09-14 16:43 ` Naga Harish K, S V
2022-09-14 15:33 ` [PATCH v7 " Naga Harish K S V
3 siblings, 1 reply; 41+ messages in thread
From: Jerin Jacob @ 2022-09-14 15:24 UTC (permalink / raw)
To: Naga Harish K S V; +Cc: jerinj, dev, erik.g.carrillo, pbhagavatula, sthotton
On Wed, Sep 14, 2022 at 7:21 PM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> This patch adds support to configure and use periodic event
> timers in software timer adapter.
>
> The structure ``rte_event_timer_adapter_stats`` is extended
> by adding a new field, ``evtim_drop_count``. This stat
> represents the number of times an event_timer expiry event
> is dropped by the event timer adapter.
>
> update the software eventdev pmd timer_adapter_caps_get
> callback function to report the support of periodic
> event timer capability
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Fix doc build issue
See: http://mails.dpdk.org/archives/test-report/2022-September/306650.html
> ---
> doc/guides/rel_notes/deprecation.rst | 7 --
> doc/guides/rel_notes/release_22_11.rst | 2 +
> drivers/event/sw/sw_evdev.c | 2 +-
> lib/eventdev/eventdev_pmd.h | 3 +
> lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++---------
> lib/eventdev/rte_event_timer_adapter.h | 2 +
> lib/eventdev/rte_eventdev.c | 6 +-
> 7 files changed, 83 insertions(+), 45 deletions(-)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index e7583cae4c..fd8ef4dff7 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -186,13 +186,6 @@ Deprecation Notices
> Event will be one of the configuration fields,
> together with additional vector parameters.
>
> -* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
> - extended by adding a new field ``evtim_drop_count``.
> - This counter will represent the number of times an event_timer expiry event
> - is dropped by the timer adapter.
> - This field will be used to add periodic mode support
> - to the software timer adapter in DPDK 22.11.
> -
> * eventdev: The function pointer declaration ``eventdev_stop_flush_t``
> will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
>
> diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
> index c32c18ff49..976a74b054 100644
> --- a/doc/guides/rel_notes/release_22_11.rst
> +++ b/doc/guides/rel_notes/release_22_11.rst
> @@ -94,6 +94,8 @@ API Changes
> ABI Changes
> -----------
>
> +* eventdev: Added ``evtim_drop_count`` field to ``rte_event_timer_adapter_stats``
> + structure.
> .. This section should contain ABI changes. Sample format:
>
> * sample: Add a short 1-2 sentence description of the ABI change
> diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> index c9efe35bf4..e866150379 100644
> --- a/drivers/event/sw/sw_evdev.c
> +++ b/drivers/event/sw/sw_evdev.c
> @@ -564,7 +564,7 @@ sw_timer_adapter_caps_get(const struct rte_eventdev *dev, uint64_t flags,
> {
> RTE_SET_USED(dev);
> RTE_SET_USED(flags);
> - *caps = 0;
> + *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
>
> /* Use default SW ops */
> *ops = NULL;
> diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
> index f514a37575..085563ff52 100644
> --- a/lib/eventdev/eventdev_pmd.h
> +++ b/lib/eventdev/eventdev_pmd.h
> @@ -81,6 +81,9 @@ extern "C" {
> * the ethdev to eventdev use a SW service function
> */
>
> +#define RTE_EVENT_TIMER_ADAPTER_SW_CAP \
> + RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC
> +
> #define RTE_EVENTDEV_DETACHED (0)
> #define RTE_EVENTDEV_ATTACHED (1)
>
> diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
> index e0d978d641..d2480060c5 100644
> --- a/lib/eventdev/rte_event_timer_adapter.c
> +++ b/lib/eventdev/rte_event_timer_adapter.c
> @@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
> #define EVTIM_SVC_LOG_DBG(...) (void)0
> #endif
>
> +static inline enum rte_timer_type
> +get_timer_type(const struct rte_event_timer_adapter *adapter)
> +{
> + return (adapter->data->conf.flags &
> + RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
> + PERIODICAL : SINGLE;
> +}
> +
> static int
> default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
> void *conf_arg)
> @@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
> adapter->data->conf = *conf; /* copy conf structure */
>
> /* Query eventdev PMD for timer adapter capabilities and ops */
> - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> - adapter->data->conf.flags,
> - &adapter->data->caps,
> - &adapter->ops);
> - if (ret < 0) {
> - rte_errno = -ret;
> - goto free_memzone;
> + if (dev->dev_ops->timer_adapter_caps_get) {
> + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + adapter->data->conf.flags,
> + &adapter->data->caps, &adapter->ops);
> + if (ret < 0) {
> + rte_errno = -ret;
> + goto free_memzone;
> + }
> }
>
> if (!(adapter->data->caps &
> @@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
> dev = &rte_eventdevs[adapter->data->event_dev_id];
>
> /* Query eventdev PMD for timer adapter capabilities and ops */
> - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> - adapter->data->conf.flags,
> - &adapter->data->caps,
> - &adapter->ops);
> - if (ret < 0) {
> - rte_errno = EINVAL;
> - return NULL;
> + if (dev->dev_ops->timer_adapter_caps_get) {
> + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + adapter->data->conf.flags,
> + &adapter->data->caps, &adapter->ops);
> + if (ret < 0) {
> + rte_errno = EINVAL;
> + return NULL;
> + }
> }
>
> /* If eventdev PMD did not provide ops, use default software
> @@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
> uint64_t opaque;
> int ret;
> int n_lcores;
> + enum rte_timer_type type;
>
> opaque = evtim->impl_opaque[1];
> adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
> sw = swtim_pmd_priv(adapter);
> + type = get_timer_type(adapter);
> +
> + if (unlikely(sw->in_use[lcore].v == 0)) {
> + sw->in_use[lcore].v = 1;
> + n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
> + __ATOMIC_RELAXED);
> + __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
> + __ATOMIC_RELAXED);
> + }
>
> ret = event_buffer_add(&sw->buffer, &evtim->ev);
> if (ret < 0) {
> - /* If event buffer is full, put timer back in list with
> - * immediate expiry value, so that we process it again on the
> - * next iteration.
> - */
> - ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
> - lcore, NULL, evtim);
> - if (ret < 0) {
> - EVTIM_LOG_DBG("event buffer full, failed to reset "
> - "timer with immediate expiry value");
> + if (type == SINGLE) {
> + /* If event buffer is full, put timer back in list with
> + * immediate expiry value, so that we process it again
> + * on the next iteration.
> + */
> + ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
> + SINGLE, lcore, NULL, evtim);
> + if (ret < 0) {
> + EVTIM_LOG_DBG("event buffer full, failed to "
> + "reset timer with immediate "
> + "expiry value");
> + } else {
> + sw->stats.evtim_retry_count++;
> + EVTIM_LOG_DBG("event buffer full, resetting "
> + "rte_timer with immediate "
> + "expiry value");
> + }
> } else {
> - sw->stats.evtim_retry_count++;
> - EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
> - "with immediate expiry value");
> + sw->stats.evtim_drop_count++;
> }
>
> - if (unlikely(sw->in_use[lcore].v == 0)) {
> - sw->in_use[lcore].v = 1;
> - n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
> - __ATOMIC_RELAXED);
> - __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
> - __ATOMIC_RELAXED);
> - }
> } else {
> EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
>
> @@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
> sw->n_expired_timers = 0;
> }
>
> - sw->expired_timers[sw->n_expired_timers++] = tim;
> + /* Don't free rte_timer for a periodic event timer until
> + * it is cancelled
> + */
> + if (type == SINGLE)
> + sw->expired_timers[sw->n_expired_timers++] = tim;
> sw->stats.evtim_exp_count++;
>
> - __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
> + if (type == SINGLE)
> + __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
> __ATOMIC_RELEASE);
> }
>
> @@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
> swtim_free_tim,
> sw);
>
> + ret = rte_timer_data_dealloc(sw->timer_data_id);
> + if (ret < 0) {
> + EVTIM_LOG_ERR("failed to deallocate timer data instance");
> + return ret;
> + }
> +
> ret = rte_service_component_unregister(sw->service_id);
> if (ret < 0) {
> EVTIM_LOG_ERR("failed to unregister service component");
> @@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
> /* Timer list for this lcore is not in use. */
> uint16_t exp_state = 0;
> enum rte_event_timer_state n_state;
> + enum rte_timer_type type = SINGLE;
>
> #ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> /* Check that the service is running. */
> @@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
> return 0;
> }
>
> + /* update timer type for periodic adapter */
> + type = get_timer_type(adapter);
> +
> for (i = 0; i < nb_evtims; i++) {
> n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
> if (n_state == RTE_EVENT_TIMER_ARMED) {
> @@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
>
> cycles = get_timeout_cycles(evtims[i], adapter);
> ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
> - SINGLE, lcore_id, NULL, evtims[i]);
> + type, lcore_id, NULL, evtims[i]);
> if (ret < 0) {
> /* tim was in RUNNING or CONFIG state */
> __atomic_store_n(&evtims[i]->state,
> diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
> index eab8e59a57..cd10db19e4 100644
> --- a/lib/eventdev/rte_event_timer_adapter.h
> +++ b/lib/eventdev/rte_event_timer_adapter.h
> @@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
> /**< Event timer retry count */
> uint64_t adapter_tick_count;
> /**< Tick count for the adapter, at its resolution */
> + uint64_t evtim_drop_count;
> + /**< event timer expiries dropped */
> };
>
> struct rte_event_timer_adapter;
> diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
> index 1dc4f966be..59d8b49ef6 100644
> --- a/lib/eventdev/rte_eventdev.c
> +++ b/lib/eventdev/rte_eventdev.c
> @@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
>
> if (caps == NULL)
> return -EINVAL;
> - *caps = 0;
> +
> + if (dev->dev_ops->timer_adapter_caps_get == NULL)
> + *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
> + else
> + *caps = 0;
>
> return dev->dev_ops->timer_adapter_caps_get ?
> (*dev->dev_ops->timer_adapter_caps_get)(dev,
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v7 1/3] eventdev/timer: add periodic event timer support
2022-09-14 13:51 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Naga Harish K S V
` (2 preceding siblings ...)
2022-09-14 15:24 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Jerin Jacob
@ 2022-09-14 15:33 ` Naga Harish K S V
2022-09-14 15:33 ` [PATCH v7 2/3] timer: fix function to stop all timers Naga Harish K S V
` (2 more replies)
3 siblings, 3 replies; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 15:33 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
update the software eventdev pmd timer_adapter_caps_get
callback function to report the support of periodic
event timer capability
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
doc/guides/rel_notes/deprecation.rst | 7 --
doc/guides/rel_notes/release_22_11.rst | 3 +
drivers/event/sw/sw_evdev.c | 2 +-
lib/eventdev/eventdev_pmd.h | 3 +
lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++---------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
7 files changed, 84 insertions(+), 45 deletions(-)
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index e7583cae4c..fd8ef4dff7 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -186,13 +186,6 @@ Deprecation Notices
Event will be one of the configuration fields,
together with additional vector parameters.
-* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
- extended by adding a new field ``evtim_drop_count``.
- This counter will represent the number of times an event_timer expiry event
- is dropped by the timer adapter.
- This field will be used to add periodic mode support
- to the software timer adapter in DPDK 22.11.
-
* eventdev: The function pointer declaration ``eventdev_stop_flush_t``
will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
index c32c18ff49..1bbdd7068a 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -94,6 +94,9 @@ API Changes
ABI Changes
-----------
+* eventdev: Added ``evtim_drop_count`` field to ``rte_event_timer_adapter_stats``
+ structure.
+
.. This section should contain ABI changes. Sample format:
* sample: Add a short 1-2 sentence description of the ABI change
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index c9efe35bf4..e866150379 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -564,7 +564,7 @@ sw_timer_adapter_caps_get(const struct rte_eventdev *dev, uint64_t flags,
{
RTE_SET_USED(dev);
RTE_SET_USED(flags);
- *caps = 0;
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
/* Use default SW ops */
*ops = NULL;
diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index f514a37575..085563ff52 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -81,6 +81,9 @@ extern "C" {
* the ethdev to eventdev use a SW service function
*/
+#define RTE_EVENT_TIMER_ADAPTER_SW_CAP \
+ RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC
+
#define RTE_EVENTDEV_DETACHED (0)
#define RTE_EVENTDEV_ATTACHED (1)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..d2480060c5 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = -ret;
- goto free_memzone;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = -ret;
+ goto free_memzone;
+ }
}
if (!(adapter->data->caps &
@@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
- adapter->data->conf.flags,
- &adapter->data->caps,
- &adapter->ops);
- if (ret < 0) {
- rte_errno = EINVAL;
- return NULL;
+ if (dev->dev_ops->timer_adapter_caps_get) {
+ ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ adapter->data->conf.flags,
+ &adapter->data->caps, &adapter->ops);
+ if (ret < 0) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
}
/* If eventdev PMD did not provide ops, use default software
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..59d8b49ef6 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v7 2/3] timer: fix function to stop all timers
2022-09-14 15:33 ` [PATCH v7 " Naga Harish K S V
@ 2022-09-14 15:33 ` Naga Harish K S V
2022-09-15 6:41 ` Jerin Jacob
2022-09-14 15:33 ` [PATCH v7 3/3] test/event: update periodic event timer tests Naga Harish K S V
2022-09-15 6:40 ` [PATCH v7 1/3] eventdev/timer: add periodic event timer support Jerin Jacob
2 siblings, 1 reply; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 15:33 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton, stable
There is a possibility of deadlock in this API,
as same spinlock is tried to be acquired in nested manner.
If the lcore that is stopping the timer is different from the lcore
that owns the timer, the timer list lock is acquired in timer_del(),
even if local_is_locked is true. Because the same lock was already
acquired in rte_timer_stop_all(), the thread will hang.
This patch removes the acquisition of nested lock.
Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a list")
Cc: stable@dpdk.org
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/timer/rte_timer.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/lib/timer/rte_timer.c b/lib/timer/rte_timer.c
index 9994813d0d..85d67573eb 100644
--- a/lib/timer/rte_timer.c
+++ b/lib/timer/rte_timer.c
@@ -580,7 +580,7 @@ rte_timer_reset_sync(struct rte_timer *tim, uint64_t ticks,
}
static int
-__rte_timer_stop(struct rte_timer *tim, int local_is_locked,
+__rte_timer_stop(struct rte_timer *tim,
struct rte_timer_data *timer_data)
{
union rte_timer_status prev_status, status;
@@ -602,7 +602,7 @@ __rte_timer_stop(struct rte_timer *tim, int local_is_locked,
/* remove it from list */
if (prev_status.state == RTE_TIMER_PENDING) {
- timer_del(tim, prev_status, local_is_locked, priv_timer);
+ timer_del(tim, prev_status, 0, priv_timer);
__TIMER_STAT_ADD(priv_timer, pending, -1);
}
@@ -631,7 +631,7 @@ rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim)
TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, timer_data, -EINVAL);
- return __rte_timer_stop(tim, 0, timer_data);
+ return __rte_timer_stop(tim, timer_data);
}
/* loop until rte_timer_stop() succeed */
@@ -987,21 +987,16 @@ rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
walk_lcore = walk_lcores[i];
priv_timer = &timer_data->priv_timer[walk_lcore];
- rte_spinlock_lock(&priv_timer->list_lock);
-
for (tim = priv_timer->pending_head.sl_next[0];
tim != NULL;
tim = next_tim) {
next_tim = tim->sl_next[0];
- /* Call timer_stop with lock held */
- __rte_timer_stop(tim, 1, timer_data);
+ __rte_timer_stop(tim, timer_data);
if (f)
f(tim, f_arg);
}
-
- rte_spinlock_unlock(&priv_timer->list_lock);
}
return 0;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v7 3/3] test/event: update periodic event timer tests
2022-09-14 15:33 ` [PATCH v7 " Naga Harish K S V
2022-09-14 15:33 ` [PATCH v7 2/3] timer: fix function to stop all timers Naga Harish K S V
@ 2022-09-14 15:33 ` Naga Harish K S V
2022-09-15 6:43 ` Jerin Jacob
2022-09-15 6:40 ` [PATCH v7 1/3] eventdev/timer: add periodic event timer support Jerin Jacob
2 siblings, 1 reply; 41+ messages in thread
From: Naga Harish K S V @ 2022-09-14 15:33 UTC (permalink / raw)
To: jerinj; +Cc: dev, erik.g.carrillo, pbhagavatula, sthotton
This patch updates the software timer adapter tests to
configure and use periodic event timers.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
index d6170bb589..654c412836 100644
--- a/app/test/test_event_timer_adapter.c
+++ b/app/test/test_event_timer_adapter.c
@@ -386,11 +386,22 @@ timdev_setup_msec(void)
static int
timdev_setup_msec_periodic(void)
{
+ uint32_t caps = 0;
+ uint64_t max_tmo_ns;
+
uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ max_tmo_ns = 0;
+ else
+ max_tmo_ns = 180 * NSECPERSEC;
+
/* Periodic mode with 100 ms resolution */
- return _timdev_setup(0, NSECPERSEC / 10, flags);
+ return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
}
static int
@@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
/* Periodic mode with 1 sec resolution */
- return _timdev_setup(0, NSECPERSEC, flags);
+ return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
}
static int
@@ -561,12 +572,23 @@ test_timer_arm(void)
static inline int
test_timer_arm_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
}
@@ -649,12 +671,23 @@ test_timer_arm_burst(void)
static inline int
test_timer_arm_burst_periodic(void)
{
+ uint32_t caps = 0;
+ uint32_t timeout_count = 0;
+
TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
"Failed to arm timers");
/* With a resolution of 100ms and wait time of 1sec,
* there will be 10 * MAX_TIMERS periodic timer triggers.
*/
- TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
+ TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
+ "failed to get adapter capabilities");
+
+ if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
+ timeout_count = 10;
+ else
+ timeout_count = 9;
+
+ TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
"Timer triggered count doesn't match arm count");
return TEST_SUCCESS;
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v6 1/3] eventdev/timer: add periodic event timer support
2022-09-14 15:24 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Jerin Jacob
@ 2022-09-14 16:43 ` Naga Harish K, S V
0 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K, S V @ 2022-09-14 16:43 UTC (permalink / raw)
To: Jerin Jacob; +Cc: jerinj, dev, Carrillo, Erik G, pbhagavatula, sthotton
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, September 14, 2022 8:54 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; dev@dpdk.org; Carrillo, Erik G
> <erik.g.carrillo@intel.com>; pbhagavatula@marvell.com;
> sthotton@marvell.com
> Subject: Re: [PATCH v6 1/3] eventdev/timer: add periodic event timer
> support
>
> On Wed, Sep 14, 2022 at 7:21 PM Naga Harish K S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > This patch adds support to configure and use periodic event timers in
> > software timer adapter.
> >
> > The structure ``rte_event_timer_adapter_stats`` is extended by adding
> > a new field, ``evtim_drop_count``. This stat represents the number of
> > times an event_timer expiry event is dropped by the event timer
> > adapter.
> >
> > update the software eventdev pmd timer_adapter_caps_get callback
> > function to report the support of periodic event timer capability
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
>
>
> Fix doc build issue
>
> See: http://mails.dpdk.org/archives/test-report/2022-
> September/306650.html
>
>
Fixed in V7 version of the patch set.
> > ---
> > doc/guides/rel_notes/deprecation.rst | 7 --
> > doc/guides/rel_notes/release_22_11.rst | 2 +
> > drivers/event/sw/sw_evdev.c | 2 +-
> > lib/eventdev/eventdev_pmd.h | 3 +
> > lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++-------
> --
> > lib/eventdev/rte_event_timer_adapter.h | 2 +
> > lib/eventdev/rte_eventdev.c | 6 +-
> > 7 files changed, 83 insertions(+), 45 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index e7583cae4c..fd8ef4dff7 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -186,13 +186,6 @@ Deprecation Notices
> > Event will be one of the configuration fields,
> > together with additional vector parameters.
> >
> > -* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
> > - extended by adding a new field ``evtim_drop_count``.
> > - This counter will represent the number of times an event_timer
> > expiry event
> > - is dropped by the timer adapter.
> > - This field will be used to add periodic mode support
> > - to the software timer adapter in DPDK 22.11.
> > -
> > * eventdev: The function pointer declaration ``eventdev_stop_flush_t``
> > will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
> >
> > diff --git a/doc/guides/rel_notes/release_22_11.rst
> > b/doc/guides/rel_notes/release_22_11.rst
> > index c32c18ff49..976a74b054 100644
> > --- a/doc/guides/rel_notes/release_22_11.rst
> > +++ b/doc/guides/rel_notes/release_22_11.rst
> > @@ -94,6 +94,8 @@ API Changes
> > ABI Changes
> > -----------
> >
> > +* eventdev: Added ``evtim_drop_count`` field to
> > +``rte_event_timer_adapter_stats``
> > + structure.
> > .. This section should contain ABI changes. Sample format:
> >
> > * sample: Add a short 1-2 sentence description of the ABI change
> > diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> > index c9efe35bf4..e866150379 100644
> > --- a/drivers/event/sw/sw_evdev.c
> > +++ b/drivers/event/sw/sw_evdev.c
> > @@ -564,7 +564,7 @@ sw_timer_adapter_caps_get(const struct
> > rte_eventdev *dev, uint64_t flags, {
> > RTE_SET_USED(dev);
> > RTE_SET_USED(flags);
> > - *caps = 0;
> > + *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
> >
> > /* Use default SW ops */
> > *ops = NULL;
> > diff --git a/lib/eventdev/eventdev_pmd.h
> b/lib/eventdev/eventdev_pmd.h
> > index f514a37575..085563ff52 100644
> > --- a/lib/eventdev/eventdev_pmd.h
> > +++ b/lib/eventdev/eventdev_pmd.h
> > @@ -81,6 +81,9 @@ extern "C" {
> > * the ethdev to eventdev use a SW service function
> > */
> >
> > +#define RTE_EVENT_TIMER_ADAPTER_SW_CAP \
> > + RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC
> > +
> > #define RTE_EVENTDEV_DETACHED (0)
> > #define RTE_EVENTDEV_ATTACHED (1)
> >
> > diff --git a/lib/eventdev/rte_event_timer_adapter.c
> > b/lib/eventdev/rte_event_timer_adapter.c
> > index e0d978d641..d2480060c5 100644
> > --- a/lib/eventdev/rte_event_timer_adapter.c
> > +++ b/lib/eventdev/rte_event_timer_adapter.c
> > @@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops
> > swtim_ops; #define EVTIM_SVC_LOG_DBG(...) (void)0 #endif
> >
> > +static inline enum rte_timer_type
> > +get_timer_type(const struct rte_event_timer_adapter *adapter) {
> > + return (adapter->data->conf.flags &
> > + RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
> > + PERIODICAL : SINGLE; }
> > +
> > static int
> > default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t
> *event_port_id,
> > void *conf_arg)
> > @@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
> > adapter->data->conf = *conf; /* copy conf structure */
> >
> > /* Query eventdev PMD for timer adapter capabilities and ops */
> > - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> > - adapter->data->conf.flags,
> > - &adapter->data->caps,
> > - &adapter->ops);
> > - if (ret < 0) {
> > - rte_errno = -ret;
> > - goto free_memzone;
> > + if (dev->dev_ops->timer_adapter_caps_get) {
> > + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> > + adapter->data->conf.flags,
> > + &adapter->data->caps, &adapter->ops);
> > + if (ret < 0) {
> > + rte_errno = -ret;
> > + goto free_memzone;
> > + }
> > }
> >
> > if (!(adapter->data->caps &
> > @@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t
> adapter_id)
> > dev = &rte_eventdevs[adapter->data->event_dev_id];
> >
> > /* Query eventdev PMD for timer adapter capabilities and ops */
> > - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> > - adapter->data->conf.flags,
> > - &adapter->data->caps,
> > - &adapter->ops);
> > - if (ret < 0) {
> > - rte_errno = EINVAL;
> > - return NULL;
> > + if (dev->dev_ops->timer_adapter_caps_get) {
> > + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> > + adapter->data->conf.flags,
> > + &adapter->data->caps, &adapter->ops);
> > + if (ret < 0) {
> > + rte_errno = EINVAL;
> > + return NULL;
> > + }
> > }
> >
> > /* If eventdev PMD did not provide ops, use default software
> > @@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
> > uint64_t opaque;
> > int ret;
> > int n_lcores;
> > + enum rte_timer_type type;
> >
> > opaque = evtim->impl_opaque[1];
> > adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
> > sw = swtim_pmd_priv(adapter);
> > + type = get_timer_type(adapter);
> > +
> > + if (unlikely(sw->in_use[lcore].v == 0)) {
> > + sw->in_use[lcore].v = 1;
> > + n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
> > + __ATOMIC_RELAXED);
> > + __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
> > + __ATOMIC_RELAXED);
> > + }
> >
> > ret = event_buffer_add(&sw->buffer, &evtim->ev);
> > if (ret < 0) {
> > - /* If event buffer is full, put timer back in list with
> > - * immediate expiry value, so that we process it again on the
> > - * next iteration.
> > - */
> > - ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
> > - lcore, NULL, evtim);
> > - if (ret < 0) {
> > - EVTIM_LOG_DBG("event buffer full, failed to reset "
> > - "timer with immediate expiry value");
> > + if (type == SINGLE) {
> > + /* If event buffer is full, put timer back in list with
> > + * immediate expiry value, so that we process it again
> > + * on the next iteration.
> > + */
> > + ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
> > + SINGLE, lcore, NULL, evtim);
> > + if (ret < 0) {
> > + EVTIM_LOG_DBG("event buffer full, failed to "
> > + "reset timer with immediate "
> > + "expiry value");
> > + } else {
> > + sw->stats.evtim_retry_count++;
> > + EVTIM_LOG_DBG("event buffer full, resetting "
> > + "rte_timer with immediate "
> > + "expiry value");
> > + }
> > } else {
> > - sw->stats.evtim_retry_count++;
> > - EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
> > - "with immediate expiry value");
> > + sw->stats.evtim_drop_count++;
> > }
> >
> > - if (unlikely(sw->in_use[lcore].v == 0)) {
> > - sw->in_use[lcore].v = 1;
> > - n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
> > - __ATOMIC_RELAXED);
> > - __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
> > - __ATOMIC_RELAXED);
> > - }
> > } else {
> > EVTIM_BUF_LOG_DBG("buffered an event timer expiry
> > event");
> >
> > @@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
> > sw->n_expired_timers = 0;
> > }
> >
> > - sw->expired_timers[sw->n_expired_timers++] = tim;
> > + /* Don't free rte_timer for a periodic event timer until
> > + * it is cancelled
> > + */
> > + if (type == SINGLE)
> > + sw->expired_timers[sw->n_expired_timers++] =
> > + tim;
> > sw->stats.evtim_exp_count++;
> >
> > - __atomic_store_n(&evtim->state,
> RTE_EVENT_TIMER_NOT_ARMED,
> > + if (type == SINGLE)
> > + __atomic_store_n(&evtim->state,
> > + RTE_EVENT_TIMER_NOT_ARMED,
> > __ATOMIC_RELEASE);
> > }
> >
> > @@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter
> *adapter)
> > swtim_free_tim,
> > sw);
> >
> > + ret = rte_timer_data_dealloc(sw->timer_data_id);
> > + if (ret < 0) {
> > + EVTIM_LOG_ERR("failed to deallocate timer data instance");
> > + return ret;
> > + }
> > +
> > ret = rte_service_component_unregister(sw->service_id);
> > if (ret < 0) {
> > EVTIM_LOG_ERR("failed to unregister service
> > component"); @@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct
> rte_event_timer_adapter *adapter,
> > /* Timer list for this lcore is not in use. */
> > uint16_t exp_state = 0;
> > enum rte_event_timer_state n_state;
> > + enum rte_timer_type type = SINGLE;
> >
> > #ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> > /* Check that the service is running. */ @@ -1092,6 +1123,9 @@
> > __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
> > return 0;
> > }
> >
> > + /* update timer type for periodic adapter */
> > + type = get_timer_type(adapter);
> > +
> > for (i = 0; i < nb_evtims; i++) {
> > n_state = __atomic_load_n(&evtims[i]->state,
> __ATOMIC_ACQUIRE);
> > if (n_state == RTE_EVENT_TIMER_ARMED) { @@ -1135,7
> > +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter
> > *adapter,
> >
> > cycles = get_timeout_cycles(evtims[i], adapter);
> > ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
> > - SINGLE, lcore_id, NULL, evtims[i]);
> > + type, lcore_id, NULL,
> > + evtims[i]);
> > if (ret < 0) {
> > /* tim was in RUNNING or CONFIG state */
> > __atomic_store_n(&evtims[i]->state,
> > diff --git a/lib/eventdev/rte_event_timer_adapter.h
> > b/lib/eventdev/rte_event_timer_adapter.h
> > index eab8e59a57..cd10db19e4 100644
> > --- a/lib/eventdev/rte_event_timer_adapter.h
> > +++ b/lib/eventdev/rte_event_timer_adapter.h
> > @@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
> > /**< Event timer retry count */
> > uint64_t adapter_tick_count;
> > /**< Tick count for the adapter, at its resolution */
> > + uint64_t evtim_drop_count;
> > + /**< event timer expiries dropped */
> > };
> >
> > struct rte_event_timer_adapter;
> > diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
> > index 1dc4f966be..59d8b49ef6 100644
> > --- a/lib/eventdev/rte_eventdev.c
> > +++ b/lib/eventdev/rte_eventdev.c
> > @@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t
> dev_id,
> > uint32_t *caps)
> >
> > if (caps == NULL)
> > return -EINVAL;
> > - *caps = 0;
> > +
> > + if (dev->dev_ops->timer_adapter_caps_get == NULL)
> > + *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
> > + else
> > + *caps = 0;
> >
> > return dev->dev_ops->timer_adapter_caps_get ?
> >
> > (*dev->dev_ops->timer_adapter_caps_get)(dev,
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 1/3] eventdev/timer: add periodic event timer support
2022-09-14 15:33 ` [PATCH v7 " Naga Harish K S V
2022-09-14 15:33 ` [PATCH v7 2/3] timer: fix function to stop all timers Naga Harish K S V
2022-09-14 15:33 ` [PATCH v7 3/3] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-09-15 6:40 ` Jerin Jacob
2 siblings, 0 replies; 41+ messages in thread
From: Jerin Jacob @ 2022-09-15 6:40 UTC (permalink / raw)
To: Naga Harish K S V; +Cc: jerinj, dev, erik.g.carrillo, pbhagavatula, sthotton
On Wed, Sep 14, 2022 at 9:03 PM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> This patch adds support to configure and use periodic event
> timers in software timer adapter.
>
> The structure ``rte_event_timer_adapter_stats`` is extended
> by adding a new field, ``evtim_drop_count``. This stat
> represents the number of times an event_timer expiry event
> is dropped by the event timer adapter.
>
> update the software eventdev pmd timer_adapter_caps_get
> callback function to report the support of periodic
> event timer capability
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Added missing Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
Updated the git commit as follows and applied to
dpdk-next-net-eventdev/for-main. Thanks
eventdev/timer: add periodic event timer support
Add support to configure and use periodic event timers in
software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
Updated the software eventdev pmd timer_adapter_caps_get
callback function to report the support of periodic
event timer capability.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---
> doc/guides/rel_notes/deprecation.rst | 7 --
> doc/guides/rel_notes/release_22_11.rst | 3 +
> drivers/event/sw/sw_evdev.c | 2 +-
> lib/eventdev/eventdev_pmd.h | 3 +
> lib/eventdev/rte_event_timer_adapter.c | 106 ++++++++++++++++---------
> lib/eventdev/rte_event_timer_adapter.h | 2 +
> lib/eventdev/rte_eventdev.c | 6 +-
> 7 files changed, 84 insertions(+), 45 deletions(-)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index e7583cae4c..fd8ef4dff7 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -186,13 +186,6 @@ Deprecation Notices
> Event will be one of the configuration fields,
> together with additional vector parameters.
>
> -* eventdev: The structure ``rte_event_timer_adapter_stats`` will be
> - extended by adding a new field ``evtim_drop_count``.
> - This counter will represent the number of times an event_timer expiry event
> - is dropped by the timer adapter.
> - This field will be used to add periodic mode support
> - to the software timer adapter in DPDK 22.11.
> -
> * eventdev: The function pointer declaration ``eventdev_stop_flush_t``
> will be renamed to ``rte_eventdev_stop_flush_t`` in DPDK 22.11.
>
> diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
> index c32c18ff49..1bbdd7068a 100644
> --- a/doc/guides/rel_notes/release_22_11.rst
> +++ b/doc/guides/rel_notes/release_22_11.rst
> @@ -94,6 +94,9 @@ API Changes
> ABI Changes
> -----------
>
> +* eventdev: Added ``evtim_drop_count`` field to ``rte_event_timer_adapter_stats``
> + structure.
> +
> .. This section should contain ABI changes. Sample format:
>
> * sample: Add a short 1-2 sentence description of the ABI change
> diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> index c9efe35bf4..e866150379 100644
> --- a/drivers/event/sw/sw_evdev.c
> +++ b/drivers/event/sw/sw_evdev.c
> @@ -564,7 +564,7 @@ sw_timer_adapter_caps_get(const struct rte_eventdev *dev, uint64_t flags,
> {
> RTE_SET_USED(dev);
> RTE_SET_USED(flags);
> - *caps = 0;
> + *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
>
> /* Use default SW ops */
> *ops = NULL;
> diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
> index f514a37575..085563ff52 100644
> --- a/lib/eventdev/eventdev_pmd.h
> +++ b/lib/eventdev/eventdev_pmd.h
> @@ -81,6 +81,9 @@ extern "C" {
> * the ethdev to eventdev use a SW service function
> */
>
> +#define RTE_EVENT_TIMER_ADAPTER_SW_CAP \
> + RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC
> +
> #define RTE_EVENTDEV_DETACHED (0)
> #define RTE_EVENTDEV_ATTACHED (1)
>
> diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
> index e0d978d641..d2480060c5 100644
> --- a/lib/eventdev/rte_event_timer_adapter.c
> +++ b/lib/eventdev/rte_event_timer_adapter.c
> @@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
> #define EVTIM_SVC_LOG_DBG(...) (void)0
> #endif
>
> +static inline enum rte_timer_type
> +get_timer_type(const struct rte_event_timer_adapter *adapter)
> +{
> + return (adapter->data->conf.flags &
> + RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
> + PERIODICAL : SINGLE;
> +}
> +
> static int
> default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
> void *conf_arg)
> @@ -195,13 +203,14 @@ rte_event_timer_adapter_create_ext(
> adapter->data->conf = *conf; /* copy conf structure */
>
> /* Query eventdev PMD for timer adapter capabilities and ops */
> - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> - adapter->data->conf.flags,
> - &adapter->data->caps,
> - &adapter->ops);
> - if (ret < 0) {
> - rte_errno = -ret;
> - goto free_memzone;
> + if (dev->dev_ops->timer_adapter_caps_get) {
> + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + adapter->data->conf.flags,
> + &adapter->data->caps, &adapter->ops);
> + if (ret < 0) {
> + rte_errno = -ret;
> + goto free_memzone;
> + }
> }
>
> if (!(adapter->data->caps &
> @@ -348,13 +357,14 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
> dev = &rte_eventdevs[adapter->data->event_dev_id];
>
> /* Query eventdev PMD for timer adapter capabilities and ops */
> - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> - adapter->data->conf.flags,
> - &adapter->data->caps,
> - &adapter->ops);
> - if (ret < 0) {
> - rte_errno = EINVAL;
> - return NULL;
> + if (dev->dev_ops->timer_adapter_caps_get) {
> + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + adapter->data->conf.flags,
> + &adapter->data->caps, &adapter->ops);
> + if (ret < 0) {
> + rte_errno = EINVAL;
> + return NULL;
> + }
> }
>
> /* If eventdev PMD did not provide ops, use default software
> @@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
> uint64_t opaque;
> int ret;
> int n_lcores;
> + enum rte_timer_type type;
>
> opaque = evtim->impl_opaque[1];
> adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
> sw = swtim_pmd_priv(adapter);
> + type = get_timer_type(adapter);
> +
> + if (unlikely(sw->in_use[lcore].v == 0)) {
> + sw->in_use[lcore].v = 1;
> + n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
> + __ATOMIC_RELAXED);
> + __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
> + __ATOMIC_RELAXED);
> + }
>
> ret = event_buffer_add(&sw->buffer, &evtim->ev);
> if (ret < 0) {
> - /* If event buffer is full, put timer back in list with
> - * immediate expiry value, so that we process it again on the
> - * next iteration.
> - */
> - ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
> - lcore, NULL, evtim);
> - if (ret < 0) {
> - EVTIM_LOG_DBG("event buffer full, failed to reset "
> - "timer with immediate expiry value");
> + if (type == SINGLE) {
> + /* If event buffer is full, put timer back in list with
> + * immediate expiry value, so that we process it again
> + * on the next iteration.
> + */
> + ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
> + SINGLE, lcore, NULL, evtim);
> + if (ret < 0) {
> + EVTIM_LOG_DBG("event buffer full, failed to "
> + "reset timer with immediate "
> + "expiry value");
> + } else {
> + sw->stats.evtim_retry_count++;
> + EVTIM_LOG_DBG("event buffer full, resetting "
> + "rte_timer with immediate "
> + "expiry value");
> + }
> } else {
> - sw->stats.evtim_retry_count++;
> - EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
> - "with immediate expiry value");
> + sw->stats.evtim_drop_count++;
> }
>
> - if (unlikely(sw->in_use[lcore].v == 0)) {
> - sw->in_use[lcore].v = 1;
> - n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
> - __ATOMIC_RELAXED);
> - __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
> - __ATOMIC_RELAXED);
> - }
> } else {
> EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
>
> @@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
> sw->n_expired_timers = 0;
> }
>
> - sw->expired_timers[sw->n_expired_timers++] = tim;
> + /* Don't free rte_timer for a periodic event timer until
> + * it is cancelled
> + */
> + if (type == SINGLE)
> + sw->expired_timers[sw->n_expired_timers++] = tim;
> sw->stats.evtim_exp_count++;
>
> - __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
> + if (type == SINGLE)
> + __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
> __ATOMIC_RELEASE);
> }
>
> @@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
> swtim_free_tim,
> sw);
>
> + ret = rte_timer_data_dealloc(sw->timer_data_id);
> + if (ret < 0) {
> + EVTIM_LOG_ERR("failed to deallocate timer data instance");
> + return ret;
> + }
> +
> ret = rte_service_component_unregister(sw->service_id);
> if (ret < 0) {
> EVTIM_LOG_ERR("failed to unregister service component");
> @@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
> /* Timer list for this lcore is not in use. */
> uint16_t exp_state = 0;
> enum rte_event_timer_state n_state;
> + enum rte_timer_type type = SINGLE;
>
> #ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> /* Check that the service is running. */
> @@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
> return 0;
> }
>
> + /* update timer type for periodic adapter */
> + type = get_timer_type(adapter);
> +
> for (i = 0; i < nb_evtims; i++) {
> n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
> if (n_state == RTE_EVENT_TIMER_ARMED) {
> @@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
>
> cycles = get_timeout_cycles(evtims[i], adapter);
> ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
> - SINGLE, lcore_id, NULL, evtims[i]);
> + type, lcore_id, NULL, evtims[i]);
> if (ret < 0) {
> /* tim was in RUNNING or CONFIG state */
> __atomic_store_n(&evtims[i]->state,
> diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
> index eab8e59a57..cd10db19e4 100644
> --- a/lib/eventdev/rte_event_timer_adapter.h
> +++ b/lib/eventdev/rte_event_timer_adapter.h
> @@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
> /**< Event timer retry count */
> uint64_t adapter_tick_count;
> /**< Tick count for the adapter, at its resolution */
> + uint64_t evtim_drop_count;
> + /**< event timer expiries dropped */
> };
>
> struct rte_event_timer_adapter;
> diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
> index 1dc4f966be..59d8b49ef6 100644
> --- a/lib/eventdev/rte_eventdev.c
> +++ b/lib/eventdev/rte_eventdev.c
> @@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
>
> if (caps == NULL)
> return -EINVAL;
> - *caps = 0;
> +
> + if (dev->dev_ops->timer_adapter_caps_get == NULL)
> + *caps = RTE_EVENT_TIMER_ADAPTER_SW_CAP;
> + else
> + *caps = 0;
>
> return dev->dev_ops->timer_adapter_caps_get ?
> (*dev->dev_ops->timer_adapter_caps_get)(dev,
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 2/3] timer: fix function to stop all timers
2022-09-14 15:33 ` [PATCH v7 2/3] timer: fix function to stop all timers Naga Harish K S V
@ 2022-09-15 6:41 ` Jerin Jacob
2022-09-16 4:40 ` Naga Harish K, S V
2022-09-26 5:21 ` Naga Harish K, S V
0 siblings, 2 replies; 41+ messages in thread
From: Jerin Jacob @ 2022-09-15 6:41 UTC (permalink / raw)
To: Naga Harish K S V, Thomas Monjalon
Cc: jerinj, dev, erik.g.carrillo, pbhagavatula, sthotton, stable
On Wed, Sep 14, 2022 at 9:03 PM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> There is a possibility of deadlock in this API,
> as same spinlock is tried to be acquired in nested manner.
>
> If the lcore that is stopping the timer is different from the lcore
> that owns the timer, the timer list lock is acquired in timer_del(),
> even if local_is_locked is true. Because the same lock was already
> acquired in rte_timer_stop_all(), the thread will hang.
>
> This patch removes the acquisition of nested lock.
>
> Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a list")
> Cc: stable@dpdk.org
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> ---
> lib/timer/rte_timer.c | 13 ++++---------
Since this change in lib/timer. Delegating this patch to @Thomas Monjalon
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 3/3] test/event: update periodic event timer tests
2022-09-14 15:33 ` [PATCH v7 3/3] test/event: update periodic event timer tests Naga Harish K S V
@ 2022-09-15 6:43 ` Jerin Jacob
0 siblings, 0 replies; 41+ messages in thread
From: Jerin Jacob @ 2022-09-15 6:43 UTC (permalink / raw)
To: Naga Harish K S V; +Cc: jerinj, dev, erik.g.carrillo, pbhagavatula, sthotton
On Wed, Sep 14, 2022 at 9:03 PM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> This patch updates the software timer adapter tests to
> configure and use periodic event timers.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> ---
> app/test/test_event_timer_adapter.c | 41 ++++++++++++++++++++++++++---
Applied 1/3 and 3/3 to dpdk-next-net-eventdev/for-main. Thanks
> 1 file changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/app/test/test_event_timer_adapter.c b/app/test/test_event_timer_adapter.c
> index d6170bb589..654c412836 100644
> --- a/app/test/test_event_timer_adapter.c
> +++ b/app/test/test_event_timer_adapter.c
> @@ -386,11 +386,22 @@ timdev_setup_msec(void)
> static int
> timdev_setup_msec_periodic(void)
> {
> + uint32_t caps = 0;
> + uint64_t max_tmo_ns;
> +
> uint64_t flags = RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES |
> RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
>
> + TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
> + "failed to get adapter capabilities");
> +
> + if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
> + max_tmo_ns = 0;
> + else
> + max_tmo_ns = 180 * NSECPERSEC;
> +
> /* Periodic mode with 100 ms resolution */
> - return _timdev_setup(0, NSECPERSEC / 10, flags);
> + return _timdev_setup(max_tmo_ns, NSECPERSEC / 10, flags);
> }
>
> static int
> @@ -409,7 +420,7 @@ timdev_setup_sec_periodic(void)
> RTE_EVENT_TIMER_ADAPTER_F_PERIODIC;
>
> /* Periodic mode with 1 sec resolution */
> - return _timdev_setup(0, NSECPERSEC, flags);
> + return _timdev_setup(180 * NSECPERSEC, NSECPERSEC, flags);
> }
>
> static int
> @@ -561,12 +572,23 @@ test_timer_arm(void)
> static inline int
> test_timer_arm_periodic(void)
> {
> + uint32_t caps = 0;
> + uint32_t timeout_count = 0;
> +
> TEST_ASSERT_SUCCESS(_arm_timers(1, MAX_TIMERS),
> "Failed to arm timers");
> /* With a resolution of 100ms and wait time of 1sec,
> * there will be 10 * MAX_TIMERS periodic timer triggers.
> */
> - TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
> + TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
> + "failed to get adapter capabilities");
> +
> + if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
> + timeout_count = 10;
> + else
> + timeout_count = 9;
> +
> + TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
> "Timer triggered count doesn't match arm count");
> return TEST_SUCCESS;
> }
> @@ -649,12 +671,23 @@ test_timer_arm_burst(void)
> static inline int
> test_timer_arm_burst_periodic(void)
> {
> + uint32_t caps = 0;
> + uint32_t timeout_count = 0;
> +
> TEST_ASSERT_SUCCESS(_arm_timers_burst(1, MAX_TIMERS),
> "Failed to arm timers");
> /* With a resolution of 100ms and wait time of 1sec,
> * there will be 10 * MAX_TIMERS periodic timer triggers.
> */
> - TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, 10 * MAX_TIMERS, 0),
> + TEST_ASSERT_SUCCESS(rte_event_timer_adapter_caps_get(evdev, &caps),
> + "failed to get adapter capabilities");
> +
> + if (caps & RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)
> + timeout_count = 10;
> + else
> + timeout_count = 9;
> +
> + TEST_ASSERT_SUCCESS(_wait_timer_triggers(1, timeout_count * MAX_TIMERS, 0),
> "Timer triggered count doesn't match arm count");
>
> return TEST_SUCCESS;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v7 2/3] timer: fix function to stop all timers
2022-09-15 6:41 ` Jerin Jacob
@ 2022-09-16 4:40 ` Naga Harish K, S V
2022-10-05 12:59 ` Thomas Monjalon
2022-09-26 5:21 ` Naga Harish K, S V
1 sibling, 1 reply; 41+ messages in thread
From: Naga Harish K, S V @ 2022-09-16 4:40 UTC (permalink / raw)
To: Jerin Jacob, Thomas Monjalon
Cc: jerinj, dev, Carrillo, Erik G, pbhagavatula, sthotton, stable
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, September 15, 2022 12:12 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: jerinj@marvell.com; dev@dpdk.org; Carrillo, Erik G
> <erik.g.carrillo@intel.com>; pbhagavatula@marvell.com;
> sthotton@marvell.com; stable@dpdk.org
> Subject: Re: [PATCH v7 2/3] timer: fix function to stop all timers
>
> On Wed, Sep 14, 2022 at 9:03 PM Naga Harish K S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > There is a possibility of deadlock in this API, as same spinlock is
> > tried to be acquired in nested manner.
> >
> > If the lcore that is stopping the timer is different from the lcore
> > that owns the timer, the timer list lock is acquired in timer_del(),
> > even if local_is_locked is true. Because the same lock was already
> > acquired in rte_timer_stop_all(), the thread will hang.
> >
> > This patch removes the acquisition of nested lock.
> >
> > Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a
> > list")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
Added missing ack
> > ---
> > lib/timer/rte_timer.c | 13 ++++---------
>
> Since this change in lib/timer. Delegating this patch to @Thomas Monjalon
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v7 2/3] timer: fix function to stop all timers
2022-09-15 6:41 ` Jerin Jacob
2022-09-16 4:40 ` Naga Harish K, S V
@ 2022-09-26 5:21 ` Naga Harish K, S V
1 sibling, 0 replies; 41+ messages in thread
From: Naga Harish K, S V @ 2022-09-26 5:21 UTC (permalink / raw)
To: Jerin Jacob, Thomas Monjalon
Cc: jerinj, dev, Carrillo, Erik G, pbhagavatula, sthotton, stable
Hi Thomas,
Did you get a chance to review this patch?
Without this patch, the periodic event timer tests for SW timer adapter hangs.
-Harish
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, September 15, 2022 12:12 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: jerinj@marvell.com; dev@dpdk.org; Carrillo, Erik G
> <erik.g.carrillo@intel.com>; pbhagavatula@marvell.com;
> sthotton@marvell.com; stable@dpdk.org
> Subject: Re: [PATCH v7 2/3] timer: fix function to stop all timers
>
> On Wed, Sep 14, 2022 at 9:03 PM Naga Harish K S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > There is a possibility of deadlock in this API, as same spinlock is
> > tried to be acquired in nested manner.
> >
> > If the lcore that is stopping the timer is different from the lcore
> > that owns the timer, the timer list lock is acquired in timer_del(),
> > even if local_is_locked is true. Because the same lock was already
> > acquired in rte_timer_stop_all(), the thread will hang.
> >
> > This patch removes the acquisition of nested lock.
> >
> > Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a
> > list")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > ---
> > lib/timer/rte_timer.c | 13 ++++---------
>
> Since this change in lib/timer. Delegating this patch to @Thomas Monjalon
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 2/3] timer: fix function to stop all timers
2022-09-16 4:40 ` Naga Harish K, S V
@ 2022-10-05 12:59 ` Thomas Monjalon
0 siblings, 0 replies; 41+ messages in thread
From: Thomas Monjalon @ 2022-10-05 12:59 UTC (permalink / raw)
To: Naga Harish K, S V
Cc: Jerin Jacob, stable, jerinj, dev, Carrillo, Erik G, pbhagavatula,
sthotton
> > On Wed, Sep 14, 2022 at 9:03 PM Naga Harish K S V
> > <s.v.naga.harish.k@intel.com> wrote:
> > >
> > > There is a possibility of deadlock in this API, as same spinlock is
> > > tried to be acquired in nested manner.
> > >
> > > If the lcore that is stopping the timer is different from the lcore
> > > that owns the timer, the timer list lock is acquired in timer_del(),
> > > even if local_is_locked is true. Because the same lock was already
> > > acquired in rte_timer_stop_all(), the thread will hang.
> > >
> > > This patch removes the acquisition of nested lock.
> > >
> > > Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a
> > > list")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
>
> Added missing ack
Applied, thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v2 1/4] eventdev/timer: add periodic event timer support
2022-09-13 12:07 ` Jerin Jacob
@ 2022-09-14 5:16 ` Naga Harish K, S V
0 siblings, 0 replies; 41+ messages in thread
From: Naga Harish K, S V @ 2022-09-14 5:16 UTC (permalink / raw)
To: Jerin Jacob; +Cc: Carrillo, Erik G, jerinj, pbhagavatula, sthotton, dev
Hi Jerin,
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, September 13, 2022 5:38 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: Carrillo, Erik G <erik.g.carrillo@intel.com>; jerinj@marvell.com;
> pbhagavatula@marvell.com; sthotton@marvell.com; dev@dpdk.org
> Subject: Re: [PATCH v2 1/4] eventdev/timer: add periodic event timer
> support
>
> On Wed, Aug 10, 2022 at 12:31 PM Naga Harish K S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > This patch adds support to configure and use periodic event timers in
> > software timer adapter.
> >
> > The structure ``rte_event_timer_adapter_stats`` is extended by adding
> > a new field, ``evtim_drop_count``. This stat represents the number of
> > times an event_timer expiry event is dropped by the event timer
> > adapter.
>
> 1) Please remove depreciation from
> doc/guides/rel_notes/deprecation.rst for above chage
> 2) Please squash "event/sw: report periodic event timer capability"
> with first path and update the comments.
>
> Rest looks good to me. Good to merge the next version.
New patchset (V5) is posted with requested changes.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 1/4] eventdev/timer: add periodic event timer support
2022-08-10 7:01 [PATCH v2 1/4] " Naga Harish K S V
@ 2022-09-13 12:07 ` Jerin Jacob
2022-09-14 5:16 ` Naga Harish K, S V
0 siblings, 1 reply; 41+ messages in thread
From: Jerin Jacob @ 2022-09-13 12:07 UTC (permalink / raw)
To: Naga Harish K S V; +Cc: erik.g.carrillo, jerinj, pbhagavatula, sthotton, dev
On Wed, Aug 10, 2022 at 12:31 PM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> This patch adds support to configure and use periodic event
> timers in software timer adapter.
>
> The structure ``rte_event_timer_adapter_stats`` is extended
> by adding a new field, ``evtim_drop_count``. This stat
> represents the number of times an event_timer expiry event
> is dropped by the event timer adapter.
1) Please remove depreciation from
doc/guides/rel_notes/deprecation.rst for above chage
2) Please squash "event/sw: report periodic event timer capability"
with first path and update the comments.
Rest looks good to me. Good to merge the next version.
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 1/4] eventdev/timer: add periodic event timer support
@ 2022-08-10 7:01 Naga Harish K S V
2022-09-13 12:07 ` Jerin Jacob
0 siblings, 1 reply; 41+ messages in thread
From: Naga Harish K S V @ 2022-08-10 7:01 UTC (permalink / raw)
To: erik.g.carrillo, jerinj; +Cc: pbhagavatula, sthotton, dev
This patch adds support to configure and use periodic event
timers in software timer adapter.
The structure ``rte_event_timer_adapter_stats`` is extended
by adding a new field, ``evtim_drop_count``. This stat
represents the number of times an event_timer expiry event
is dropped by the event timer adapter.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/eventdev/rte_event_timer_adapter.c | 86 ++++++++++++++++++--------
lib/eventdev/rte_event_timer_adapter.h | 2 +
lib/eventdev/rte_eventdev.c | 6 +-
3 files changed, 67 insertions(+), 27 deletions(-)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index e0d978d641..0de88dfc0f 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops swtim_ops;
#define EVTIM_SVC_LOG_DBG(...) (void)0
#endif
+static inline enum rte_timer_type
+get_event_timer_type(const struct rte_event_timer_adapter *adapter)
+{
+ return (adapter->data->conf.flags &
+ RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
+ PERIODICAL : SINGLE;
+}
+
static int
default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
void *conf_arg)
@@ -195,10 +203,11 @@ rte_event_timer_adapter_create_ext(
adapter->data->conf = *conf; /* copy conf structure */
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ ret = dev->dev_ops->timer_adapter_caps_get ?
+ dev->dev_ops->timer_adapter_caps_get(dev,
adapter->data->conf.flags,
&adapter->data->caps,
- &adapter->ops);
+ &adapter->ops) : 0;
if (ret < 0) {
rte_errno = -ret;
goto free_memzone;
@@ -348,10 +357,11 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
dev = &rte_eventdevs[adapter->data->event_dev_id];
/* Query eventdev PMD for timer adapter capabilities and ops */
- ret = dev->dev_ops->timer_adapter_caps_get(dev,
+ ret = dev->dev_ops->timer_adapter_caps_get ?
+ dev->dev_ops->timer_adapter_caps_get(dev,
adapter->data->conf.flags,
&adapter->data->caps,
- &adapter->ops);
+ &adapter->ops) : 0;
if (ret < 0) {
rte_errno = EINVAL;
return NULL;
@@ -612,35 +622,44 @@ swtim_callback(struct rte_timer *tim)
uint64_t opaque;
int ret;
int n_lcores;
+ enum rte_timer_type type;
opaque = evtim->impl_opaque[1];
adapter = (struct rte_event_timer_adapter *)(uintptr_t)opaque;
sw = swtim_pmd_priv(adapter);
+ type = get_event_timer_type(adapter);
+
+ if (unlikely(sw->in_use[lcore].v == 0)) {
+ sw->in_use[lcore].v = 1;
+ n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
+ __ATOMIC_RELAXED);
+ __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
+ __ATOMIC_RELAXED);
+ }
ret = event_buffer_add(&sw->buffer, &evtim->ev);
if (ret < 0) {
- /* If event buffer is full, put timer back in list with
- * immediate expiry value, so that we process it again on the
- * next iteration.
- */
- ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0, SINGLE,
- lcore, NULL, evtim);
- if (ret < 0) {
- EVTIM_LOG_DBG("event buffer full, failed to reset "
- "timer with immediate expiry value");
+ if (type == SINGLE) {
+ /* If event buffer is full, put timer back in list with
+ * immediate expiry value, so that we process it again
+ * on the next iteration.
+ */
+ ret = rte_timer_alt_reset(sw->timer_data_id, tim, 0,
+ SINGLE, lcore, NULL, evtim);
+ if (ret < 0) {
+ EVTIM_LOG_DBG("event buffer full, failed to "
+ "reset timer with immediate "
+ "expiry value");
+ } else {
+ sw->stats.evtim_retry_count++;
+ EVTIM_LOG_DBG("event buffer full, resetting "
+ "rte_timer with immediate "
+ "expiry value");
+ }
} else {
- sw->stats.evtim_retry_count++;
- EVTIM_LOG_DBG("event buffer full, resetting rte_timer "
- "with immediate expiry value");
+ sw->stats.evtim_drop_count++;
}
- if (unlikely(sw->in_use[lcore].v == 0)) {
- sw->in_use[lcore].v = 1;
- n_lcores = __atomic_fetch_add(&sw->n_poll_lcores, 1,
- __ATOMIC_RELAXED);
- __atomic_store_n(&sw->poll_lcores[n_lcores], lcore,
- __ATOMIC_RELAXED);
- }
} else {
EVTIM_BUF_LOG_DBG("buffered an event timer expiry event");
@@ -654,10 +673,15 @@ swtim_callback(struct rte_timer *tim)
sw->n_expired_timers = 0;
}
- sw->expired_timers[sw->n_expired_timers++] = tim;
+ /* Don't free rte_timer for a periodic event timer until
+ * it is cancelled
+ */
+ if (type == SINGLE)
+ sw->expired_timers[sw->n_expired_timers++] = tim;
sw->stats.evtim_exp_count++;
- __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
+ if (type == SINGLE)
+ __atomic_store_n(&evtim->state, RTE_EVENT_TIMER_NOT_ARMED,
__ATOMIC_RELEASE);
}
@@ -947,6 +971,12 @@ swtim_uninit(struct rte_event_timer_adapter *adapter)
swtim_free_tim,
sw);
+ ret = rte_timer_data_dealloc(sw->timer_data_id);
+ if (ret < 0) {
+ EVTIM_LOG_ERR("failed to deallocate timer data instance");
+ return ret;
+ }
+
ret = rte_service_component_unregister(sw->service_id);
if (ret < 0) {
EVTIM_LOG_ERR("failed to unregister service component");
@@ -1053,6 +1083,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
/* Timer list for this lcore is not in use. */
uint16_t exp_state = 0;
enum rte_event_timer_state n_state;
+ enum rte_timer_type type = SINGLE;
#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
/* Check that the service is running. */
@@ -1092,6 +1123,9 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
return 0;
}
+ /* update timer type for periodic adapter */
+ type = get_event_timer_type(adapter);
+
for (i = 0; i < nb_evtims; i++) {
n_state = __atomic_load_n(&evtims[i]->state, __ATOMIC_ACQUIRE);
if (n_state == RTE_EVENT_TIMER_ARMED) {
@@ -1135,7 +1169,7 @@ __swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
cycles = get_timeout_cycles(evtims[i], adapter);
ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
- SINGLE, lcore_id, NULL, evtims[i]);
+ type, lcore_id, NULL, evtims[i]);
if (ret < 0) {
/* tim was in RUNNING or CONFIG state */
__atomic_store_n(&evtims[i]->state,
diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
index eab8e59a57..cd10db19e4 100644
--- a/lib/eventdev/rte_event_timer_adapter.h
+++ b/lib/eventdev/rte_event_timer_adapter.h
@@ -193,6 +193,8 @@ struct rte_event_timer_adapter_stats {
/**< Event timer retry count */
uint64_t adapter_tick_count;
/**< Tick count for the adapter, at its resolution */
+ uint64_t evtim_drop_count;
+ /**< event timer expiries dropped */
};
struct rte_event_timer_adapter;
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 1dc4f966be..4a2a1178da 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -139,7 +139,11 @@ rte_event_timer_adapter_caps_get(uint8_t dev_id, uint32_t *caps)
if (caps == NULL)
return -EINVAL;
- *caps = 0;
+
+ if (dev->dev_ops->timer_adapter_caps_get == NULL)
+ *caps = RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC;
+ else
+ *caps = 0;
return dev->dev_ops->timer_adapter_caps_get ?
(*dev->dev_ops->timer_adapter_caps_get)(dev,
--
2.25.1
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2022-10-05 12:59 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03 16:25 [PATCH 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-08-03 16:25 ` [PATCH 4/4] test/event: update periodic event timer tests Naga Harish K S V
2022-08-10 7:07 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-08-10 7:07 ` [PATCH v2 4/4] test/event: update periodic event timer tests Naga Harish K S V
2022-08-10 19:55 ` [PATCH v2 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
2022-08-11 15:43 ` Naga Harish K, S V
2022-08-11 15:36 ` [PATCH v3 " Naga Harish K S V
2022-08-11 15:36 ` [PATCH v3 4/4] test/event: update periodic event timer tests Naga Harish K S V
2022-08-11 19:22 ` [PATCH v3 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
2022-08-12 16:10 ` Naga Harish K, S V
2022-08-12 16:07 ` [PATCH v4 " Naga Harish K S V
2022-08-12 16:07 ` [PATCH v4 4/4] test/event: update periodic event timer tests Naga Harish K S V
2022-08-18 13:13 ` Carrillo, Erik G
2022-08-18 13:06 ` [PATCH v4 1/4] eventdev/timer: add periodic event timer support Carrillo, Erik G
2022-09-14 5:08 ` [PATCH v5 " Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 2/4] timer: fix function to stop all timers Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 3/4] test/event: update periodic event timer tests Naga Harish K S V
2022-09-14 5:08 ` [PATCH v5 4/4] doc: remove deprication notice Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 1/4] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 2/4] timer: fix function to stop all timers Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 3/4] test/event: update periodic event timer tests Naga Harish K S V
2022-09-14 5:15 ` [PATCH v5 4/4] doc: remove deprecation notice Naga Harish K S V
2022-09-14 12:41 ` Jerin Jacob
2022-09-14 13:54 ` Naga Harish K, S V
2022-09-14 13:51 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Naga Harish K S V
2022-09-14 13:51 ` [PATCH v6 2/3] timer: fix function to stop all timers Naga Harish K S V
2022-09-14 13:51 ` [PATCH v6 3/3] test/event: update periodic event timer tests Naga Harish K S V
2022-09-14 15:24 ` [PATCH v6 1/3] eventdev/timer: add periodic event timer support Jerin Jacob
2022-09-14 16:43 ` Naga Harish K, S V
2022-09-14 15:33 ` [PATCH v7 " Naga Harish K S V
2022-09-14 15:33 ` [PATCH v7 2/3] timer: fix function to stop all timers Naga Harish K S V
2022-09-15 6:41 ` Jerin Jacob
2022-09-16 4:40 ` Naga Harish K, S V
2022-10-05 12:59 ` Thomas Monjalon
2022-09-26 5:21 ` Naga Harish K, S V
2022-09-14 15:33 ` [PATCH v7 3/3] test/event: update periodic event timer tests Naga Harish K S V
2022-09-15 6:43 ` Jerin Jacob
2022-09-15 6:40 ` [PATCH v7 1/3] eventdev/timer: add periodic event timer support Jerin Jacob
2022-08-10 7:01 [PATCH v2 1/4] " Naga Harish K S V
2022-09-13 12:07 ` Jerin Jacob
2022-09-14 5:16 ` Naga Harish K, S V
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).