On Mon, May 16, 2022 at 10:04:05AM +0300, Dan Carpenter wrote:
> Hello Roi Dayan,
>
> The patch 7dc84de98bab: "net/mlx5: E-Switch, Protect changing mode
> while adding rules" from Sep 16, 2020, leads to the following Smatch
> static checker warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c:2000 mlx5_esw_unlock()
> warn: inconsistent returns '&esw->mode_lock'.
>
> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> 1996 void mlx5_esw_unlock(struct mlx5_eswitch *esw)
> 1997 {
> 1998 if (!mlx5_esw_allowed(esw))
> 1999 return;
>
> Smatch is complaining because how will the caller know if we dropped
> the lock or not. I thought, "Hm. I guess the lock function has a
> similar check? Although, how does that work that mlx5_esw_allowed()
> means that it doesn't need locking?"
>
> But then when I looked at the lock function, mlx5_esw_try_lock(), and it
> does *NOT* have a similar check. This probably works because it's
> checked in different layers and this is just a duplicative (layering
> violation) check which is ugly but harmless.
Your analysis is correct and I agree with you, the check should be removed.
However the "problematic" commit is ec2fa47d7b98 ("net/mlx5: Lag, use lag lock"),
where mlx5_esw_lock() was removed.
Thanks
>
> --> 2000 up_write(&esw->mode_lock);
> 2001 }
>
> regards,
> dan carpenter
