Re: [V2 net 05/11] net/mlx5: Fix possible deadlock on mlx5e_tx_timeout_work

[Saeed Mahameed <saeed@kernel.org>]

On 08 Nov 11:19, Paolo Abeni wrote:
>On Mon, 2022-11-07 at 20:24 -0800, Jakub Kicinski wrote:
>> On Sat,  5 Nov 2022 00:10:22 -0700 Saeed Mahameed wrote:
>> > +	/* Once deactivated, new tx_timeout_work won't be initiated. */
>> > +	if (current_work() != &priv->tx_timeout_work)
>> > +		cancel_work_sync(&priv->tx_timeout_work);
>>
>> The work takes rtnl_lock, are there no callers of
>> mlx5e_switch_priv_channels() that are under rtnl_lock()?
>>
>> This patch is definitely going onto my "expecting Fixes"
>> bingo card :S
>
>I think Jakub is right and even mlx5e_close_locked() will deadlock on
>cancel_work_sync() if the work is scheduled but it has not yet acquired
>the rtnl lock.

Yes you are absolutely correct, you can see the deadlock just by looking at
the patch diff and applying common sense that mlx5e_switch_priv_channels()
is being called under rtnl.

>
>IIRC lockdep is not able to catch this kind of situation, so you can
>only observe the deadlock when reaching the critical scenario.
>
>I'm wild guessing than a possible solution would be restrict the
>state_lock scope in mlx5e_tx_timeout_work() around the state check,
>without additional cancel_work operations.
>

Thanks, i will drop the patch for now and send v3 without it.

>Thanks,
>
>Paolo
>