From: Jakub Kicinski <kuba@kernel.org>
To: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
Cc: netdev@vger.kernel.org, andrew@lunn.ch,
Julian.FRIEDRICH@frequentis.com, f.fainelli@gmail.com,
olteanv@gmail.com, davem@davemloft.net, edumazet@google.com,
pabeni@redhat.com, linux-kernel@vger.kernel.org,
upstream+netdev@sigma-star.at
Subject: Re: [PATCH] net: dsa: mv88e6xxx: properly shutdown PPU re-enable timer on destroy
Date: Sun, 3 Nov 2024 11:35:39 -0800 [thread overview]
Message-ID: <20241103113539.7b44e4f3@kernel.org> (raw)
In-Reply-To: <20241029124332.51008-1-david.oberhollenzer@sigma-star.at>
On Tue, 29 Oct 2024 13:42:45 +0100 David Oberhollenzer wrote:
> The mv88e6xxx has an internal PPU that polls PHY state. If we want to
> access the internal PHYs, we need to disable it. Because enable/disable
> of the PPU is a slow operation, a 10ms timer is used to re-enable it,
> canceled with every access, so bulk operations effectively only disable
> it once and re-enable it some 10ms after the last access.
>
> If a PHY is accessed and then the mv88e6xxx module is removed before
> the 10ms are up, the PPU re-enable ends up accessing a dangling pointer.
>
> This is easily triggered by deferred probing during boot-up. MDIO bus
> and PHY registration may succeed, but switch registration fails later
> on, because the CPU port depends on a very slow device. In this case,
> probe() fails, but the MDIO subsystem may already have accessed bus
> or the PHYs, arming timer.
>
> This is fixed as follows:
> - If probe fails after mv88e6xxx_phy_init(), make sure we also call
> mv88e6xxx_phy_destroy() before returning
> - In mv88e6xxx_phy_destroy(), grab the ppu_mutex to make sure the work
> function either has already exited, or (should it run) cannot do
> anything, fails to grab the mutex and returns.
> - In addition to destroying the timer, also destroy the work item, in
> case the timer has already fired.
> - Do all of this synchronously, to make sure timer & work item are
> destroyed and none of the callbacks are running.
Looks good, AFAICT. Could you repost with a Fixes tag added?
To make the job of the stable team easier?
prev parent reply other threads:[~2024-11-03 19:35 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-29 12:42 [PATCH] net: dsa: mv88e6xxx: properly shutdown PPU re-enable timer on destroy David Oberhollenzer
2024-11-03 19:35 ` Jakub Kicinski [this message]
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=20241103113539.7b44e4f3@kernel.org \
--to=kuba@kernel.org \
--cc=Julian.FRIEDRICH@frequentis.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=david.oberhollenzer@sigma-star.at \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=upstream+netdev@sigma-star.at \
/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.