All of lore.kernel.org
 help / color / mirror / Atom feed
From: Furong Xu <0x1207@gmail.com>
To: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Jon Hunter <jonathanh@nvidia.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-stm32@st-md-mailman.stormreply.com,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	netdev@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>,
	Thierry Reding <treding@nvidia.com>
Subject: Re: [PATCH RFC net-next 2/5] net: stmmac: remove redundant racy tear-down in stmmac_dvr_remove()
Date: Fri, 28 Feb 2025 11:28:40 +0800	[thread overview]
Message-ID: <20250228112840.00003e24@gmail.com> (raw)
In-Reply-To: <E1tnfRj-0057SF-9t@rmk-PC.armlinux.org.uk>

On Thu, 27 Feb 2025 15:05:07 +0000
"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote:

> While the network device is registered, it is published to userspace,
> and thus userspace can change its state. This means calling
> functions such as stmmac_stop_all_dma() and stmmac_mac_set() are
> racy.
> 
> Moreover, unregister_netdev() will unpublish the network device, and
> then if appropriate call the .ndo_stop() method, which is
> stmmac_release(). This will first call phylink_stop() which will
> synchronously take the link down, resulting in stmmac_mac_link_down()
> and stmmac_mac_set(, false) being called.
> 
> stmmac_release() will also call stmmac_stop_all_dma().
> 
> Consequently, neither of these two functions need to called prior
> to unregister_netdev() as that will safely call paths that will
> result in this work being done if necessary.
> 
> Remove these redundant racy calls.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 84d8b1c9f6d4..9462d05c40c8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7757,8 +7757,6 @@ void stmmac_dvr_remove(struct device *dev)
>  
>  	pm_runtime_get_sync(dev);
>  
> -	stmmac_stop_all_dma(priv);
> -	stmmac_mac_set(priv, priv->ioaddr, false);
>  	unregister_netdev(ndev);
>  
>  #ifdef CONFIG_DEBUG_FS

We always build stmmac driver as built-in.
Tried to build stmmac driver as a module, but some complicated dependencies were
reported in our down-stream kernel :(
I can not test this patch, so:

Reviewed-by: Furong Xu <0x1207@gmail.com>


  parent reply	other threads:[~2025-02-28  3:30 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-27 15:00 [PATCH RFC 0/5] net: stmmac: fix setting RE and TE inappropriately Russell King (Oracle)
2025-02-27 15:05 ` [PATCH RFC net-next 1/5] net: stmmac: call phylink_start() and phylink_stop() in XDP functions Russell King (Oracle)
2025-02-27 22:27   ` Andrew Lunn
2025-02-28  0:02     ` Russell King (Oracle)
2025-02-28  7:31   ` Furong Xu
2025-02-28 13:14     ` Andrew Lunn
2025-02-28 14:38       ` Russell King (Oracle)
2025-02-27 15:05 ` [PATCH RFC net-next 2/5] net: stmmac: remove redundant racy tear-down in stmmac_dvr_remove() Russell King (Oracle)
2025-02-27 22:16   ` Andrew Lunn
2025-02-28  3:28   ` Furong Xu [this message]
2025-02-27 15:05 ` [PATCH RFC net-next 3/5] net: stmmac: remove unnecessary stmmac_mac_set() in stmmac_release() Russell King (Oracle)
2025-02-27 22:17   ` Andrew Lunn
2025-02-28  2:51   ` Furong Xu
2025-02-27 15:05 ` [PATCH RFC net-next 4/5] net: stmmac: remove _RE and _TE in (start|stop)_(tx|rx)() methods Russell King (Oracle)
2025-02-27 22:25   ` Andrew Lunn
2025-02-28  2:52   ` Furong Xu
2025-02-27 15:05 ` [PATCH RFC net-next 5/5] net: stmmac: leave enabling RE and TE to stmmac_mac_link_up() Russell King (Oracle)
2025-02-28  2:52   ` Furong Xu

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=20250228112840.00003e24@gmail.com \
    --to=0x1207@gmail.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=jonathanh@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=treding@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.