[v2,12/20] raw/ifpga: inherit lock annotations
Checks
Commit Message
The checks in those helpers are useless:
- all (start/stop/reset/test) callers ensure that dev != NULL,
- dev->sd can't be NULL either as it would mean the application is calling
those helpers for a dev pointer that did not pass initialisation,
Once the checks are removed, the only thing that remains is calls to the
rte_spinlock API, so simply use macros and inherit annotations from the
lock API.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
drivers/raw/ifpga/afu_pmd_core.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
Comments
Hi,
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, February 24, 2023 11:12 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Xu, Rosen <rosen.xu@intel.com>; Zhang, Tianfei
> <tianfei.zhang@intel.com>
> Subject: [PATCH v2 12/20] raw/ifpga: inherit lock annotations
>
> The checks in those helpers are useless:
> - all (start/stop/reset/test) callers ensure that dev != NULL,
> - dev->sd can't be NULL either as it would mean the application is calling
> those helpers for a dev pointer that did not pass initialisation,
>
> Once the checks are removed, the only thing that remains is calls to the
> rte_spinlock API, so simply use macros and inherit annotations from the lock
> API.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> drivers/raw/ifpga/afu_pmd_core.c | 17 ++---------------
> 1 file changed, 2 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/raw/ifpga/afu_pmd_core.c
> b/drivers/raw/ifpga/afu_pmd_core.c
> index ddf7a34f33..3ab1f47ac1 100644
> --- a/drivers/raw/ifpga/afu_pmd_core.c
> +++ b/drivers/raw/ifpga/afu_pmd_core.c
> @@ -23,21 +23,8 @@ static struct rte_afu_uuid
> afu_pmd_uuid_map[AFU_RAWDEV_MAX_DRVS+1];
> TAILQ_HEAD(afu_drv_list, afu_rawdev_drv); static struct afu_drv_list
> afu_pmd_list = TAILQ_HEAD_INITIALIZER(afu_pmd_list);
>
> -static inline int afu_rawdev_trylock(struct afu_rawdev *dev) -{
> - if (!dev || !dev->sd)
> - return 0;
> -
> - return rte_spinlock_trylock(&dev->sd->lock);
> -}
> -
> -static inline void afu_rawdev_unlock(struct afu_rawdev *dev) -{
> - if (!dev || !dev->sd)
> - return;
> -
> - rte_spinlock_unlock(&dev->sd->lock);
> -}
> +#define afu_rawdev_trylock(dev) rte_spinlock_trylock(&dev->sd->lock)
> +#define afu_rawdev_unlock(dev) rte_spinlock_unlock(&dev->sd->lock)
>
> static int afu_rawdev_configure(const struct rte_rawdev *rawdev,
> rte_rawdev_obj_t config, size_t config_size)
> --
> 2.39.2
It looks good for me.
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
Hi,
> -----Original Message-----
> From: Xu, Rosen <rosen.xu@intel.com>
> Sent: Monday, February 27, 2023 14:29
> To: David Marchand <david.marchand@redhat.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; Zhang, Tianfei <tianfei.zhang@intel.com>; Huang,
> Wei <wei.huang@intel.com>
> Subject: RE: [PATCH v2 12/20] raw/ifpga: inherit lock annotations
>
> Hi,
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Friday, February 24, 2023 11:12 PM
> > To: dev@dpdk.org
> > Cc: thomas@monjalon.net; Xu, Rosen <rosen.xu@intel.com>; Zhang,
> > Tianfei <tianfei.zhang@intel.com>
> > Subject: [PATCH v2 12/20] raw/ifpga: inherit lock annotations
> >
> > The checks in those helpers are useless:
> > - all (start/stop/reset/test) callers ensure that dev != NULL,
> > - dev->sd can't be NULL either as it would mean the application is calling
> > those helpers for a dev pointer that did not pass initialisation,
> >
> > Once the checks are removed, the only thing that remains is calls to
> > the rte_spinlock API, so simply use macros and inherit annotations
> > from the lock API.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> > drivers/raw/ifpga/afu_pmd_core.c | 17 ++---------------
> > 1 file changed, 2 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/raw/ifpga/afu_pmd_core.c
> > b/drivers/raw/ifpga/afu_pmd_core.c
> > index ddf7a34f33..3ab1f47ac1 100644
> > --- a/drivers/raw/ifpga/afu_pmd_core.c
> > +++ b/drivers/raw/ifpga/afu_pmd_core.c
> > @@ -23,21 +23,8 @@ static struct rte_afu_uuid
> > afu_pmd_uuid_map[AFU_RAWDEV_MAX_DRVS+1];
> > TAILQ_HEAD(afu_drv_list, afu_rawdev_drv); static struct afu_drv_list
> > afu_pmd_list = TAILQ_HEAD_INITIALIZER(afu_pmd_list);
> >
> > -static inline int afu_rawdev_trylock(struct afu_rawdev *dev) -{
> > - if (!dev || !dev->sd)
> > - return 0;
> > -
> > - return rte_spinlock_trylock(&dev->sd->lock);
> > -}
> > -
> > -static inline void afu_rawdev_unlock(struct afu_rawdev *dev) -{
> > - if (!dev || !dev->sd)
> > - return;
> > -
> > - rte_spinlock_unlock(&dev->sd->lock);
> > -}
> > +#define afu_rawdev_trylock(dev) rte_spinlock_trylock(&dev->sd->lock)
> > +#define afu_rawdev_unlock(dev) rte_spinlock_unlock(&dev->sd->lock)
> >
> > static int afu_rawdev_configure(const struct rte_rawdev *rawdev,
> > rte_rawdev_obj_t config, size_t config_size)
> > --
> > 2.39.2
>
>
> It looks good for me.
> Reviewed-by: Rosen Xu <rosen.xu@intel.com>
Agree that pointer check is redundant, rte_spinlock API can be called directly.
Reviewed-by: Wei Huang <wei.huang@intel.com>
@@ -23,21 +23,8 @@ static struct rte_afu_uuid afu_pmd_uuid_map[AFU_RAWDEV_MAX_DRVS+1];
TAILQ_HEAD(afu_drv_list, afu_rawdev_drv);
static struct afu_drv_list afu_pmd_list = TAILQ_HEAD_INITIALIZER(afu_pmd_list);
-static inline int afu_rawdev_trylock(struct afu_rawdev *dev)
-{
- if (!dev || !dev->sd)
- return 0;
-
- return rte_spinlock_trylock(&dev->sd->lock);
-}
-
-static inline void afu_rawdev_unlock(struct afu_rawdev *dev)
-{
- if (!dev || !dev->sd)
- return;
-
- rte_spinlock_unlock(&dev->sd->lock);
-}
+#define afu_rawdev_trylock(dev) rte_spinlock_trylock(&dev->sd->lock)
+#define afu_rawdev_unlock(dev) rte_spinlock_unlock(&dev->sd->lock)
static int afu_rawdev_configure(const struct rte_rawdev *rawdev,
rte_rawdev_obj_t config, size_t config_size)