* [bug report] net/mlx5: E-Switch, Protect changing mode while adding rules
@ 2022-05-16 7:04 Dan Carpenter
2022-05-24 18:13 ` Leon Romanovsky
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-05-16 7:04 UTC (permalink / raw)
To: roid; +Cc: linux-rdma
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.
--> 2000 up_write(&esw->mode_lock);
2001 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] net/mlx5: E-Switch, Protect changing mode while adding rules
2022-05-16 7:04 [bug report] net/mlx5: E-Switch, Protect changing mode while adding rules Dan Carpenter
@ 2022-05-24 18:13 ` Leon Romanovsky
0 siblings, 0 replies; 2+ messages in thread
From: Leon Romanovsky @ 2022-05-24 18:13 UTC (permalink / raw)
To: Dan Carpenter; +Cc: roid, linux-rdma, Mark Bloch, Saeed Mahameed, linux-netdev
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-05-24 18:13 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-16 7:04 [bug report] net/mlx5: E-Switch, Protect changing mode while adding rules Dan Carpenter
2022-05-24 18:13 ` Leon Romanovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.