tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Remove ASSERT_SLEEPABLE() in mii_phy_auto()



> Date: Mon, 23 Dec 2024 16:32:26 +0900
> From: Masanobu SAITOH <msaitoh%execsw.org@localhost>
> 
> 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!


Home | Main Index | Thread Index | Old Index