Re: [PATCH net-next v6 04/11] net: ngbe: implement libwx reset ops

[Jacob Keller <jacob.e.keller@intel.com>]

On 3/31/2026 12:19 AM, Jiawen Wu wrote:
>>> +static void ngbe_reinit_locked(struct wx *wx)
>>> +{
>>> +	int err = 0;
>>> +
>>> +	netif_trans_update(wx->netdev);
>>> +
>>> +	err = wx_set_state_reset(wx);
>>> +	if (err) {
>>> +		wx_err(wx, "wait device reset timeout\n");
>>> +		return;
>>> +	}
>>> +
>>> +	ngbe_down(wx);
>>> +	ngbe_up(wx);
>>> +
>>> +	clear_bit(WX_STATE_RESETTING, wx->state);
>>> +}
>>
>> Is it possible to use a standard kernel mutex here instead of a bit flag for
>> synchronization?
>>
>> Looking at the underlying implementation of wx_set_state_reset(), it relies
>> on an open-coded polling loop:
>>
>> static inline int wx_set_state_reset(struct wx *wx)
>> {
>>         u8 timeout = 50;
>>
>>         while (test_and_set_bit(WX_STATE_RESETTING, wx->state)) {
>>                 timeout--;
>>                 if (!timeout)
>>                         return -EBUSY;
>>
>>                 usleep_range(1000, 2000);
>>         }
>>
>>         return 0;
>> }
>>
>> Using a bit flag and a sleep-polling loop to guard a teardown and bringup
>> section acts as an ad-hoc lock. This approach bypasses standard kernel
>> synchronization guarantees and prevents lockdep from analyzing the lock
>> ordering.
>>
>> Since this context is allowed to sleep, could this section be protected by
>> a standard mutex instead?
> > I think using a state flag here would be better. Because other code
paths
> (like watchdog) need to check if a reset is in process without taking a lock.
> 
> 


Couldn't you use an atomic and a mutex together?

mutex_lock(wx->reset_lock);
set_bit(WX_STATE_RESETTING, wx->state);
...
Mutex_unlock(wx->reset_lock);

Paths that only need to know a reset has already started can just
test_bit. Paths which need to wait on reset can acquire the lock.

There shouldn't be any issue doing this that doesn't already exist with
the test_and_set loop, because you can already have a path doing "if
(test_bit(resetting)) exit" flow would still potentially begin executing
even if a reset already acquired the lock, it could have ordered the
test path first regardless, so you already aren't really guaranteed that
a reset won't start until your operation is done unless you hold the lock.

Using a mutex also gives you lockdep checking and is generally more sane
to reason about.

Thanks,
Jake