From: Jakub Kicinski <kuba@kernel•org>
To: jiawenwu@trustnetic•com
Cc: Jakub Kicinski <kuba@kernel•org>,
netdev@vger•kernel.org, mengyuanlou@net-swift•com,
andrew+netdev@lunn•ch, davem@davemloft•net, edumazet@google•com,
pabeni@redhat•com, richardcochran@gmail•com,
linux@armlinux•org.uk, horms@kernel•org, kees@kernel•org,
larysa.zaremba@intel•com, leitao@debian•org, joe@dama•to,
jacob.e.keller@intel•com, fabio.baltieri@gmail•com
Subject: Re: [PATCH net-next v2 1/6] net: ngbe: implement libwx reset ops
Date: Sat, 2 May 2026 19:15:29 -0700 [thread overview]
Message-ID: <20260503021529.4127250-1-kuba@kernel.org> (raw)
In-Reply-To: <20260430082517.19612-2-jiawenwu@trustnetic.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: ngbe: implement libwx reset ops
This commit implements the wx->do_reset() operation for the ngbe driver,
allowing the library module to trigger a reset by bringing the interface
down and back up.
> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> index d8e3827a8b1f2..bd905e2675758 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
[ ... ]
> +static void ngbe_reinit_locked(struct wx *wx)
> +{
> + netif_trans_update(wx->netdev);
> +
> + mutex_lock(&wx->reset_lock);
> + set_bit(WX_STATE_RESETTING, wx->state);
> +
> + ngbe_down(wx);
> + ngbe_up(wx);
> +
> + clear_bit(WX_STATE_RESETTING, wx->state);
Does this introduce a regression by using WX_STATE_RESETTING as an ad-hoc
lock here?
While this pattern appears to exist elsewhere in the driver, adding it
to ngbe_reinit_locked() relies on a design where background readers might
check test_bit(WX_STATE_RESETTING, wx->state) without holding reset_lock.
Since the flag check and subsequent accesses are not atomic, a reader could
test the flag just before it is set and then proceed to access shared state
concurrently with the ngbe_down() and ngbe_up() teardown path.
Could this result in torn or corrupted reads, and would it be safer to use a
proper synchronization mechanism like RCU or a shared lock for readers?
> + mutex_unlock(&wx->reset_lock);
> +}
> +
> +void ngbe_do_reset(struct net_device *netdev)
[ ... ]
next prev parent reply other threads:[~2026-05-03 2:15 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-30 8:25 [PATCH net-next v2 0/6] net: wangxun: timeout and error Jiawen Wu
2026-04-30 8:25 ` [PATCH net-next v2 1/6] net: ngbe: implement libwx reset ops Jiawen Wu
2026-05-03 2:15 ` Jakub Kicinski [this message]
2026-05-06 9:05 ` Jiawen Wu
2026-04-30 8:25 ` [PATCH net-next v2 2/6] net: wangxun: add Tx timeout process Jiawen Wu
2026-05-03 2:15 ` Jakub Kicinski
2026-04-30 8:25 ` [PATCH net-next v2 3/6] net: wangxun: add reinit parameter to wx->do_reset callback Jiawen Wu
2026-04-30 8:25 ` [PATCH net-next v2 4/6] net: wangxun: extract the close_suspend sequence Jiawen Wu
2026-05-03 2:15 ` Jakub Kicinski
2026-04-30 8:25 ` [PATCH net-next v2 5/6] net: wangxun: clear stored DMA addresses after dma_free_coherent() Jiawen Wu
2026-05-03 2:15 ` Jakub Kicinski
2026-05-08 8:43 ` Jiawen Wu
2026-04-30 8:25 ` [PATCH net-next v2 6/6] net: wangxun: implement pci_error_handlers ops Jiawen Wu
2026-05-03 2:15 ` Jakub Kicinski
2026-05-09 8:29 ` 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=20260503021529.4127250-1-kuba@kernel.org \
--to=kuba@kernel$(echo .)org \
--cc=andrew+netdev@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=fabio.baltieri@gmail$(echo .)com \
--cc=horms@kernel$(echo .)org \
--cc=jacob.e.keller@intel$(echo .)com \
--cc=jiawenwu@trustnetic$(echo .)com \
--cc=joe@dama$(echo .)to \
--cc=kees@kernel$(echo .)org \
--cc=larysa.zaremba@intel$(echo .)com \
--cc=leitao@debian$(echo .)org \
--cc=linux@armlinux$(echo .)org.uk \
--cc=mengyuanlou@net-swift$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=richardcochran@gmail$(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