From: "Jiawen Wu" <jiawenwu@trustnetic•com>
To: "'Simon Horman'" <horms@kernel•org>
Cc: <netdev@vger•kernel.org>, <mengyuanlou@net-swift•com>,
<andrew+netdev@lunn•ch>, <davem@davemloft•net>,
<edumazet@google•com>, <kuba@kernel•org>, <pabeni@redhat•com>,
<kees@kernel•org>, <larysa.zaremba@intel•com>,
<jacob.e.keller@intel•com>, <netdev@vger•kernel.org>,
<mengyuanlou@net-swift•com>, <andrew+netdev@lunn•ch>,
<davem@davemloft•net>, <edumazet@google•com>, <kuba@kernel•org>,
<pabeni@redhat•com>, <kees@kernel•org>,
<larysa.zaremba@intel•com>, <jacob.e.keller@intel•com>
Subject: RE: [PATCH net-next v1 1/3] net: wangxun: introduce WX_STATE_DOWN to serialize device shutdown state
Date: Mon, 25 May 2026 09:44:03 +0800 [thread overview]
Message-ID: <03b201dcebe7$f79804a0$e6c80de0$@trustnetic.com> (raw)
In-Reply-To: <20260522073415.2032087-1-horms@kernel.org>
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.
next prev parent reply other threads:[~2026-05-25 1:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-19 8:00 [PATCH net-next v1 0/3] wangxun: improve service task synchronization Jiawen Wu
2026-05-19 8:00 ` [PATCH net-next v1 1/3] net: wangxun: introduce WX_STATE_DOWN to serialize device shutdown state Jiawen Wu
2026-05-22 7:34 ` Simon Horman
2026-05-25 1:44 ` Jiawen Wu [this message]
2026-05-19 8:00 ` [PATCH net-next v1 2/3] net: wangxun: avoid statistics updates during device teardown Jiawen Wu
2026-05-19 8:00 ` [PATCH net-next v1 3/3] net: txgbe: rework service event handling Jiawen Wu
2026-05-22 7:34 ` Simon Horman
2026-05-25 1:55 ` Jiawen Wu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='03b201dcebe7$f79804a0$e6c80de0$@trustnetic.com' \
--to=jiawenwu@trustnetic$(echo .)com \
--cc=andrew+netdev@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=horms@kernel$(echo .)org \
--cc=jacob.e.keller@intel$(echo .)com \
--cc=kees@kernel$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=larysa.zaremba@intel$(echo .)com \
--cc=mengyuanlou@net-swift$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox