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

["Jiawen Wu" <jiawenwu@trustnetic.com>]

> > +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.