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

[Simon Horman <horms@kernel.org>]

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?