Re: [PATCH v5 1/1] net/mana: add device reset support

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 29 May 2026 07:26:48 -0700
Wei Hu <weh@linux.microsoft.com> wrote:

> +#define MANA_OPS_1_LOCK(_func)						\
> +static int								\
> +_func##_lock(struct rte_eth_dev *dev)					\
> +{									\
> +	struct mana_priv *priv = dev->data->dev_private;		\
> +	int ret;							\
> +	if (!pthread_mutex_trylock(&priv->reset_ops_lock)) {		\
> +		if (rte_atomic_load_explicit(&priv->dev_state,		\
> +		    rte_memory_order_acquire) !=			\
> +		    MANA_DEV_ACTIVE) {					\
> +			pthread_mutex_unlock(&priv->reset_ops_lock);	\
> +			return -EBUSY;					\
> +		}							\
> +		ret = _func(dev);					\
> +		pthread_mutex_unlock(&priv->reset_ops_lock);		\
> +	} else {							\
> +		ret = -EBUSY;						\
> +	}								\
> +	return ret;							\
> +}
> +
> +MANA_OPS_1_LOCK(mana_dev_configure)
> +
> +MANA_OPS_1_LOCK(mana_dev_start)

I strongly dislike wrapping locking in macros.
Macros make code harder to analyze and hide things.

Also, using pthread_mutex here needs seems like a bigger hammer than needed.
It looks like reset is just an atomic flag and as long as it was in shared
memory simple atomic operators would suffice.

In Linux there are many ways to express the same thing
but in general if most drivers follow the same patterns it helps
when there is an issue to be able to fix it globally.

Also any code which turns off thread safety analysis is a red flag
for me. It needs strong justification.

Overall this reset logic looks more complex than other drivers.