Re: Remove ASSERT_SLEEPABLE() in mii_phy_auto()

Taylor R Campbell <[email protected]>
Newsgroups gmane.os.netbsd.devel.kernel
Message-ID <[email protected]>
> Date: Mon, 23 Dec 2024 16:32:26 +0900
> From: Masanobu SAITOH <[email protected]>
> 
> A driver uses a spin mutex for mii/ifmedia lock.
> 
> When I modified the MDIO access to be sleepable using with
> workqueue and kpause(), the following assert() fired:
> 
> diff --git a/sys/dev/mii/mii_physubr.c b/sys/dev/mii/mii_physubr.c
> index b469387c051b..ec7d7130b2be 100644
> --- a/sys/dev/mii/mii_physubr.c
> +++ b/sys/dev/mii/mii_physubr.c
> @@ -271,7 +271,7 @@ mii_phy_auto(struct mii_softc *sc)
>          * delays all the time while the system is running!
>          */
>         if (sc->mii_flags & MIIF_AUTOTSLEEP) {
> -               ASSERT_SLEEPABLE();
> +//             ASSERT_SLEEPABLE();
>                 sc->mii_flags |= MIIF_DOINGAUTO;
>                 kpause("miiaut", false, hz >> 1, mii->mii_media.ifm_lock);
>                 mii_phy_auto_timeout_locked(sc);
> 
> I think it's OK to remove the above ASSERT_SLEEPABLE().
> 
>  Is it OK to commit?

Yes, that looks fine.  Maybe we should have
ASSERT_SLEEPABLE_WITHOUT_LOCK(lock) or something but in this case the
assertion is so close to the kpause that should trip an assertion
internally anyway that I don't think the ASSERT_SLEEPABLE is important
here.

...There _is_ an assertion that will trip somewhere inside kpause
after it releases the lock, right?  If not we should have one!
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.