RE: [PATCH net-next v1 1/3] net: wangxun: introduce WX_STATE_DOWN to serialize device shutdown state

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

On Fri, May 22, 2026 3:34 PM, Simon Horman wrote:
> From: 'Simon Horman' <horms@kernel.org>
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> net: wangxun: introduce WX_STATE_DOWN to serialize device shutdown state
> 
> Replace various netif_running() checks with an explicit WX_STATE_DOWN
> state bit to track whether the device datapath and interrupt handling
> are operational.
> 
> > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> > index 746623fa59b40..69fe197376796 100644
> > --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> > +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> [ ... ]
> > @@ -3314,7 +3316,8 @@ EXPORT_SYMBOL(wx_set_ring);
> >
> >  void wx_service_event_schedule(struct wx *wx)
> >  {
> > -	if (!test_and_set_bit(WX_STATE_SERVICE_SCHED, wx->state))
> > +	if (!test_and_set_bit(WX_STATE_SERVICE_SCHED, wx->state) &&
> > +	    !test_bit(WX_STATE_DOWN, wx->state))
> >  		queue_work(system_power_efficient_wq, &wx->service_task);
> >  }
> 
> Can this leak the WX_STATE_SERVICE_SCHED bit when the device is down?
> 
> If WX_STATE_DOWN is set, test_and_set_bit() executes first and sets
> WX_STATE_SERVICE_SCHED, but the second condition evaluates to false.
> This skips queue_work(), meaning the service task is never queued.
> 
> Because the task does not execute, WX_STATE_SERVICE_SCHED is never cleared.
> When the device is brought back up, subsequent calls to
> wx_service_event_schedule() might always fail the test_and_set_bit() check,
> permanently disabling the service task.
> 
> Should the conditions be reversed to check WX_STATE_DOWN before setting the
> service schedule bit?
> 

I'll fix it in V2.