Re: [PATCH v3 5/9] net/ixgbe/base: fix lock checker errors

[David Marchand <david.marchand@redhat.com>]

On Fri, Mar 28, 2025 at 11:44 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Fri, Mar 28, 2025 at 09:20:16AM +0100, David Marchand wrote:
> > On Thu, Mar 27, 2025 at 3:53 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > When building on FreeBSD, errors are reported in the base code by the
> > > lock checker (-Wthread-safety). For example:
> > >
> > > ../drivers/net/intel/ixgbe/base/ixgbe_osdep.c:42:1: error: mutex 'lock->mutex' is still held at the end of function [-Werror,-Wthread-safety-analysis]
> > >    42 | }
> > >       | ^
> > >
> > > These errors are due to the checker not recognising the lock wrapper
> > > functions. We can avoid these errors by converting these functions into
> > > macros.
> > >
> > > Fixes: 30b19d1b5c43 ("net/ixgbe/base: add definitions for E610")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> > This is the best solution, given that FreeBSD pthread is instrumented
> > with clang thread safety annotations.
> >
> > As a sidenote, I don't see much value with the remaining
> > malloc/calloc/free wrappers in this osdep.c file.
> > I suspect this makes some other annotations non working.
> >
> Yes, I would tend to agree. Question is whether it is better to convert
> them to macros or just move them to the header file as static inlines. I'd
> tend towards the latter, because otherwise we'd need to use "," syntax to
> avoid potentially introducing other warnings for unused "hw" variable.
>
> Here are two option examples, WDYT of each?
>
> static inline void *
> ixgbe_malloc(struct ixgbe_hw __rte_unused *hw, size_t s) { return malloc(s); }
>
> or
>
> #define ixgbe_malloc(hw, s) ((void)hw, malloc(s))

I think we don't have much choice but to use macros, as attributes on
a symbol won't get inherited by an inline wrapper.


-- 
David Marchand