[v3,06/22] net/ena/base: destroy multiple "wait events"
Checks
Commit Message
Although the ENA DPDK PMD doesn't have to perform any actions for
destroying the wait event, some other platforms have to.
The macro "ENA_WAIT_EVENT_DESTROY" was renamed to
"ENA_WAIT_EVENTS_DESTROY" and also whole implementation responsible for
that was moved to a separate function for better readability.
Fixes: 3adcba9a8987 ("net/ena: update HAL to the newer version")
Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
drivers/net/ena/base/ena_com.c | 24 +++++++++++++++++-------
drivers/net/ena/base/ena_plat_dpdk.h | 4 ++--
2 files changed, 19 insertions(+), 9 deletions(-)
Comments
On 5/6/2021 3:25 PM, Michal Krawczyk wrote:
> Although the ENA DPDK PMD doesn't have to perform any actions for
> destroying the wait event, some other platforms have to.
>
> The macro "ENA_WAIT_EVENT_DESTROY" was renamed to
> "ENA_WAIT_EVENTS_DESTROY" and also whole implementation responsible for
> that was moved to a separate function for better readability.
>
> Fixes: 3adcba9a8987 ("net/ena: update HAL to the newer version")
>
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> ---
> drivers/net/ena/base/ena_com.c | 24 +++++++++++++++++-------
> drivers/net/ena/base/ena_plat_dpdk.h | 4 ++--
> 2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
> index 0cdeb1a2d9..d025c9cee1 100644
> --- a/drivers/net/ena/base/ena_com.c
> +++ b/drivers/net/ena/base/ena_com.c
> @@ -1671,6 +1671,22 @@ int ena_com_validate_version(struct ena_com_dev *ena_dev)
> return 0;
> }
>
> +static void
> +ena_com_free_ena_admin_queue_comp_ctx(struct ena_com_dev *ena_dev,
> + struct ena_com_admin_queue *admin_queue)
> +
> +{
> + if (!admin_queue->comp_ctx)
> + return;
> +
> + ENA_WAIT_EVENTS_DESTROY(admin_queue);
I can see both 'ENA_WAIT_EVENT_DESTROY' & 'ENA_WAIT_EVENTS_DESTROY' does nothing
but, according their definitions:
+#define ENA_WAIT_EVENT_DESTROY(admin_queue) ((void)(admin_queue))
+#define ENA_WAIT_EVENTS_DESTROY(waitqueue) ((void)(waitqueue)
If 'ENA_WAIT_EVENTS_DESTROY' also gets 'admin_queue' as parameter, does it make
sense to update the definition of it to get 'admin_queue', like:
#define ENA_WAIT_EVENTS_DESTROY(admin_queue) ((void)(admin_queue)
And I can see that is updated in next patches, so may be better to update here.
> + ENA_MEM_FREE(ena_dev->dmadev,
> + admin_queue->comp_ctx,
> + (admin_queue->q_depth * sizeof(struct ena_comp_ctx)));
> +
> + admin_queue->comp_ctx = NULL;
> +}
> +
> void ena_com_admin_destroy(struct ena_com_dev *ena_dev)
> {
> struct ena_com_admin_queue *admin_queue = &ena_dev->admin_queue;
> @@ -1679,14 +1695,8 @@ void ena_com_admin_destroy(struct ena_com_dev *ena_dev)
> struct ena_com_aenq *aenq = &ena_dev->aenq;
> u16 size;
>
> - if (admin_queue->comp_ctx) {
> - ENA_WAIT_EVENT_DESTROY(admin_queue->comp_ctx->wait_event);
> - ENA_MEM_FREE(ena_dev->dmadev,
> - admin_queue->comp_ctx,
> - (admin_queue->q_depth * sizeof(struct ena_comp_ctx)));
> - }
> + ena_com_free_ena_admin_queue_comp_ctx(ena_dev, admin_queue);
>
> - admin_queue->comp_ctx = NULL;
> size = ADMIN_SQ_SIZE(admin_queue->q_depth);
> if (sq->entries)
> ENA_MEM_FREE_COHERENT(ena_dev->dmadev, size, sq->entries,
> diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
> index 231d44e0e0..ad7b07b374 100644
> --- a/drivers/net/ena/base/ena_plat_dpdk.h
> +++ b/drivers/net/ena/base/ena_plat_dpdk.h
> @@ -166,7 +166,7 @@ extern int ena_logtype_com;
> #define ENA_WAIT_EVENT_SIGNAL(waitevent) pthread_cond_signal(&waitevent.cond)
> /* pthread condition doesn't need to be rearmed after usage */
> #define ENA_WAIT_EVENT_CLEAR(...)
> -#define ENA_WAIT_EVENT_DESTROY(waitqueue) ((void)(waitqueue))
> +#define ENA_WAIT_EVENT_DESTROY(admin_queue) ((void)(admin_queue))
>
> #define ena_wait_event_t ena_wait_queue_t
> #define ENA_MIGHT_SLEEP()
> @@ -298,7 +298,7 @@ extern rte_atomic32_t ena_alloc_cnt;
> #define ENA_TIME_EXPIRE(timeout) (timeout < rte_get_timer_cycles())
> #define ENA_GET_SYSTEM_TIMEOUT(timeout_us) \
> (timeout_us * rte_get_timer_hz() / 1000000 + rte_get_timer_cycles())
> -#define ENA_WAIT_EVENT_DESTROY(waitqueue) ((void)(waitqueue))
> +#define ENA_WAIT_EVENTS_DESTROY(waitqueue) ((void)(waitqueue))
>
> #ifndef READ_ONCE
> #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var))))
>
pt., 7 maj 2021 o 18:47 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 5/6/2021 3:25 PM, Michal Krawczyk wrote:
> > Although the ENA DPDK PMD doesn't have to perform any actions for
> > destroying the wait event, some other platforms have to.
> >
> > The macro "ENA_WAIT_EVENT_DESTROY" was renamed to
> > "ENA_WAIT_EVENTS_DESTROY" and also whole implementation responsible for
> > that was moved to a separate function for better readability.
> >
> > Fixes: 3adcba9a8987 ("net/ena: update HAL to the newer version")
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> > ---
> > drivers/net/ena/base/ena_com.c | 24 +++++++++++++++++-------
> > drivers/net/ena/base/ena_plat_dpdk.h | 4 ++--
> > 2 files changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
> > index 0cdeb1a2d9..d025c9cee1 100644
> > --- a/drivers/net/ena/base/ena_com.c
> > +++ b/drivers/net/ena/base/ena_com.c
> > @@ -1671,6 +1671,22 @@ int ena_com_validate_version(struct ena_com_dev *ena_dev)
> > return 0;
> > }
> >
> > +static void
> > +ena_com_free_ena_admin_queue_comp_ctx(struct ena_com_dev *ena_dev,
> > + struct ena_com_admin_queue *admin_queue)
> > +
> > +{
> > + if (!admin_queue->comp_ctx)
> > + return;
> > +
> > + ENA_WAIT_EVENTS_DESTROY(admin_queue);
>
> I can see both 'ENA_WAIT_EVENT_DESTROY' & 'ENA_WAIT_EVENTS_DESTROY' does nothing
> but, according their definitions:
> +#define ENA_WAIT_EVENT_DESTROY(admin_queue) ((void)(admin_queue))
> +#define ENA_WAIT_EVENTS_DESTROY(waitqueue) ((void)(waitqueue)
>
> If 'ENA_WAIT_EVENTS_DESTROY' also gets 'admin_queue' as parameter, does it make
> sense to update the definition of it to get 'admin_queue', like:
> #define ENA_WAIT_EVENTS_DESTROY(admin_queue) ((void)(admin_queue)
>
> And I can see that is updated in next patches, so may be better to update here.
>
I agree, the ENA_WAIT_EVENTS_DESTROY() should take as an argument
admin_queue. The ENA_WAIT_EVENT_DESTROY shouldn't be touched by this
patch. I will fix that in v4.
> > + ENA_MEM_FREE(ena_dev->dmadev,
> > + admin_queue->comp_ctx,
> > + (admin_queue->q_depth * sizeof(struct ena_comp_ctx)));
> > +
> > + admin_queue->comp_ctx = NULL;
> > +}
> > +
> > void ena_com_admin_destroy(struct ena_com_dev *ena_dev)
> > {
> > struct ena_com_admin_queue *admin_queue = &ena_dev->admin_queue;
> > @@ -1679,14 +1695,8 @@ void ena_com_admin_destroy(struct ena_com_dev *ena_dev)
> > struct ena_com_aenq *aenq = &ena_dev->aenq;
> > u16 size;
> >
> > - if (admin_queue->comp_ctx) {
> > - ENA_WAIT_EVENT_DESTROY(admin_queue->comp_ctx->wait_event);
> > - ENA_MEM_FREE(ena_dev->dmadev,
> > - admin_queue->comp_ctx,
> > - (admin_queue->q_depth * sizeof(struct ena_comp_ctx)));
> > - }
> > + ena_com_free_ena_admin_queue_comp_ctx(ena_dev, admin_queue);
> >
> > - admin_queue->comp_ctx = NULL;
> > size = ADMIN_SQ_SIZE(admin_queue->q_depth);
> > if (sq->entries)
> > ENA_MEM_FREE_COHERENT(ena_dev->dmadev, size, sq->entries,
> > diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
> > index 231d44e0e0..ad7b07b374 100644
> > --- a/drivers/net/ena/base/ena_plat_dpdk.h
> > +++ b/drivers/net/ena/base/ena_plat_dpdk.h
> > @@ -166,7 +166,7 @@ extern int ena_logtype_com;
> > #define ENA_WAIT_EVENT_SIGNAL(waitevent) pthread_cond_signal(&waitevent.cond)
> > /* pthread condition doesn't need to be rearmed after usage */
> > #define ENA_WAIT_EVENT_CLEAR(...)
> > -#define ENA_WAIT_EVENT_DESTROY(waitqueue) ((void)(waitqueue))
> > +#define ENA_WAIT_EVENT_DESTROY(admin_queue) ((void)(admin_queue))
> >
> > #define ena_wait_event_t ena_wait_queue_t
> > #define ENA_MIGHT_SLEEP()
> > @@ -298,7 +298,7 @@ extern rte_atomic32_t ena_alloc_cnt;
> > #define ENA_TIME_EXPIRE(timeout) (timeout < rte_get_timer_cycles())
> > #define ENA_GET_SYSTEM_TIMEOUT(timeout_us) \
> > (timeout_us * rte_get_timer_hz() / 1000000 + rte_get_timer_cycles())
> > -#define ENA_WAIT_EVENT_DESTROY(waitqueue) ((void)(waitqueue))
> > +#define ENA_WAIT_EVENTS_DESTROY(waitqueue) ((void)(waitqueue))
> >
> > #ifndef READ_ONCE
> > #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var))))
> >
>
@@ -1671,6 +1671,22 @@ int ena_com_validate_version(struct ena_com_dev *ena_dev)
return 0;
}
+static void
+ena_com_free_ena_admin_queue_comp_ctx(struct ena_com_dev *ena_dev,
+ struct ena_com_admin_queue *admin_queue)
+
+{
+ if (!admin_queue->comp_ctx)
+ return;
+
+ ENA_WAIT_EVENTS_DESTROY(admin_queue);
+ ENA_MEM_FREE(ena_dev->dmadev,
+ admin_queue->comp_ctx,
+ (admin_queue->q_depth * sizeof(struct ena_comp_ctx)));
+
+ admin_queue->comp_ctx = NULL;
+}
+
void ena_com_admin_destroy(struct ena_com_dev *ena_dev)
{
struct ena_com_admin_queue *admin_queue = &ena_dev->admin_queue;
@@ -1679,14 +1695,8 @@ void ena_com_admin_destroy(struct ena_com_dev *ena_dev)
struct ena_com_aenq *aenq = &ena_dev->aenq;
u16 size;
- if (admin_queue->comp_ctx) {
- ENA_WAIT_EVENT_DESTROY(admin_queue->comp_ctx->wait_event);
- ENA_MEM_FREE(ena_dev->dmadev,
- admin_queue->comp_ctx,
- (admin_queue->q_depth * sizeof(struct ena_comp_ctx)));
- }
+ ena_com_free_ena_admin_queue_comp_ctx(ena_dev, admin_queue);
- admin_queue->comp_ctx = NULL;
size = ADMIN_SQ_SIZE(admin_queue->q_depth);
if (sq->entries)
ENA_MEM_FREE_COHERENT(ena_dev->dmadev, size, sq->entries,
@@ -166,7 +166,7 @@ extern int ena_logtype_com;
#define ENA_WAIT_EVENT_SIGNAL(waitevent) pthread_cond_signal(&waitevent.cond)
/* pthread condition doesn't need to be rearmed after usage */
#define ENA_WAIT_EVENT_CLEAR(...)
-#define ENA_WAIT_EVENT_DESTROY(waitqueue) ((void)(waitqueue))
+#define ENA_WAIT_EVENT_DESTROY(admin_queue) ((void)(admin_queue))
#define ena_wait_event_t ena_wait_queue_t
#define ENA_MIGHT_SLEEP()
@@ -298,7 +298,7 @@ extern rte_atomic32_t ena_alloc_cnt;
#define ENA_TIME_EXPIRE(timeout) (timeout < rte_get_timer_cycles())
#define ENA_GET_SYSTEM_TIMEOUT(timeout_us) \
(timeout_us * rte_get_timer_hz() / 1000000 + rte_get_timer_cycles())
-#define ENA_WAIT_EVENT_DESTROY(waitqueue) ((void)(waitqueue))
+#define ENA_WAIT_EVENTS_DESTROY(waitqueue) ((void)(waitqueue))
#ifndef READ_ONCE
#define READ_ONCE(var) (*((volatile typeof(var) *)(&(var))))