Date: Mon, 5 Dec 2022 14:01:14 -0700 From: Warner Losh <imp@bsdimp.com> To: Cy Schubert <Cy.Schubert@cschubert.com> Cc: Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, dev-commits-src-main@freebsd.org Subject: Re: git: aa52c6bdd7b1 - main - newbus: Create a knob to disable devices that fail to attach. Message-ID: <CANCZdfpiSYx3UK8zWpjQ1O3iOYFHHKLJuThN8RYAaLXm_YcNnA@mail.gmail.com> In-Reply-To: <20221205195134.08074206@slippy.cwsent.com> References: <202212042333.2B4NX1JM089126@gitrepo.freebsd.org> <20221205195134.08074206@slippy.cwsent.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--00000000000013f21f05ef1b23ea Content-Type: text/plain; charset="UTF-8" I think I'll just revert the default change... but in a few hours when I'm back home. Warner On Mon, Dec 5, 2022, 12:51 PM Cy Schubert <Cy.Schubert@cschubert.com> wrote: > In message <202212042333.2B4NX1JM089126@gitrepo.freebsd.org>, Warner Losh > write > s: > > The branch main has been updated by imp: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=aa52c6bdd7b157b7c5d9612e4feac5c9 > > 4f376df6 > > > > commit aa52c6bdd7b157b7c5d9612e4feac5c94f376df6 > > Author: Warner Losh <imp@FreeBSD.org> > > AuthorDate: 2022-12-04 23:20:24 +0000 > > Commit: Warner Losh <imp@FreeBSD.org> > > CommitDate: 2022-12-04 23:20:24 +0000 > > > > newbus: Create a knob to disable devices that fail to attach. > > > > Normally, when a device fails to attach, we tear down the newbus > state > > for that device so that future driver loads can try again (maybe > with a > > different driver, or maybe with a re-loaded and fixed kld). > > > > Sometimes, however, it is desirable to have the device fail > > permanantly. We do this by calling device_disable() on a failed > > attached, as well as keeping the device in DS_ATTACHING forever. This > > prevents retries on that device. This is enabled via > > hw.bus.disable_failed_devices=1 in either a hint via the loader, or > at > > runtime with a sysctl setting. Setting from 1 -> 0 at runtime will > not > > affect previously disabled devices, however: they remain disabled. > > They can be re-enabled manually with devctl enable, however. > > > > Sponsored by: Netflix > > > > Reviewed by: gallatin, hselasky, jhb > > Differential Revision: https://reviews.freebsd.org/D37517 > > --- > > sys/kern/subr_bus.c | 33 +++++++++++++++++++++++++++------ > > 1 file changed, 27 insertions(+), 6 deletions(-) > > > > diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c > > index 739484ad849a..b9615b033007 100644 > > --- a/sys/kern/subr_bus.c > > +++ b/sys/kern/subr_bus.c > > @@ -69,6 +69,10 @@ SYSCTL_NODE(_hw, OID_AUTO, bus, CTLFLAG_RW | > CTLFLAG_MPSAF > > E, NULL, > > SYSCTL_ROOT_NODE(OID_AUTO, dev, CTLFLAG_RW | CTLFLAG_MPSAFE, NULL, > > NULL); > > > > +static bool disable_failed_devs = false; > > +SYSCTL_BOOL(_hw_bus, OID_AUTO, disable_failed_devices, CTLFLAG_RWTUN, > &disab > > le_failed_devs, > > + 0, "Do not retry attaching devices that return an error from > DEVICE_ATTA > > CH the first time"); > > + > > /* > > * Used to attach drivers to devclasses. > > */ > > @@ -2533,12 +2537,29 @@ device_attach(device_t dev) > > if ((error = DEVICE_ATTACH(dev)) != 0) { > > printf("device_attach: %s%d attach returned %d\n", > > dev->driver->name, dev->unit, error); > > - if (!(dev->flags & DF_FIXEDCLASS)) > > - devclass_delete_device(dev->devclass, dev); > > - (void)device_set_driver(dev, NULL); > > - device_sysctl_fini(dev); > > - KASSERT(dev->busy == 0, ("attach failed but busy")); > > - dev->state = DS_NOTPRESENT; > > + if (disable_failed_devs) { > > + /* > > + * When the user has asked to disable failed > devices, w > > e > > + * directly disable the device, but leave it in the > > + * attaching state. It will not try to > probe/attach the > > + * device further. This leaves the device numbering > > + * intact for other similar devices in the system. > It > > + * can be removed from this state with devctl. > > + */ > > + device_disable(dev); > > + } else { > > + /* > > + * Otherwise, when attach fails, tear down the > state > > + * around that so we can retry when, for example, > new > > + * drivers are loaded. > > + */ > > + if (!(dev->flags & DF_FIXEDCLASS)) > > + devclass_delete_device(dev->devclass, dev); > > + (void)device_set_driver(dev, NULL); > > + device_sysctl_fini(dev); > > + KASSERT(dev->busy == 0, ("attach failed but > busy")); > > + dev->state = DS_NOTPRESENT; > > + } > > return (error); > > } > > dev->flags |= DF_ATTACHED_ONCE; > > > > When switching a KVM console from a FreeBSD machine to another machine > attached to the KVM the FreeBSD machine will panic. Photo attached. > > This probably needs an UPDATING entry explaining that > hw.bus.disable_failed_devices sysctl/kenv must be zero (0) to avoid the > panic. > > Sorry for taking so long to report this. Had an appointment this morning. > > > Cheers, > Cy Schubert <Cy.Schubert@cschubert.com> > FreeBSD UNIX: <cy@FreeBSD.org> Web: https://FreeBSD.org > NTP: <cy@nwtime.org> Web: https://nwtime.org > > e^(i*pi)+1=0 > --00000000000013f21f05ef1b23ea Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"auto">I think I'll just revert the default change... but in= a few hours when I'm back home.<div dir=3D"auto"><br></div><div dir=3D= "auto">Warner=C2=A0</div></div><br><div class=3D"gmail_quote"><div dir=3D"l= tr" class=3D"gmail_attr">On Mon, Dec 5, 2022, 12:51 PM Cy Schubert <<a h= ref=3D"mailto:Cy.Schubert@cschubert.com">Cy.Schubert@cschubert.com</a>> = wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8e= x;border-left:1px #ccc solid;padding-left:1ex">In message <<a href=3D"ma= ilto:202212042333.2B4NX1JM089126@gitrepo.freebsd.org" target=3D"_blank" rel= =3D"noreferrer">202212042333.2B4NX1JM089126@gitrepo.freebsd.org</a>>, Wa= rner Losh <br> write<br> s:<br> > The branch main has been updated by imp:<br> ><br> > URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3Daa52c6bdd7b1= 57b7c5d9612e4feac5c9" rel=3D"noreferrer noreferrer" target=3D"_blank">https= ://cgit.FreeBSD.org/src/commit/?id=3Daa52c6bdd7b157b7c5d9612e4feac5c9</a><b= r> > 4f376df6<br> ><br> > commit aa52c6bdd7b157b7c5d9612e4feac5c94f376df6<br> > Author:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org><br> > AuthorDate: 2022-12-04 23:20:24 +0000<br> > Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org><br> > CommitDate: 2022-12-04 23:20:24 +0000<br> ><br> >=C2=A0 =C2=A0 =C2=A0newbus: Create a knob to disable devices that fail = to attach.<br> >=C2=A0 =C2=A0 =C2=A0<br> >=C2=A0 =C2=A0 =C2=A0Normally, when a device fails to attach, we tear do= wn the newbus state<br> >=C2=A0 =C2=A0 =C2=A0for that device so that future driver loads can try= again (maybe with a<br> >=C2=A0 =C2=A0 =C2=A0different driver, or maybe with a re-loaded and fix= ed kld).<br> >=C2=A0 =C2=A0 =C2=A0<br> >=C2=A0 =C2=A0 =C2=A0Sometimes, however, it is desirable to have the dev= ice fail<br> >=C2=A0 =C2=A0 =C2=A0permanantly. We do this by calling device_disable()= on a failed<br> >=C2=A0 =C2=A0 =C2=A0attached, as well as keeping the device in DS_ATTAC= HING forever. This<br> >=C2=A0 =C2=A0 =C2=A0prevents retries on that device. This is enabled vi= a<br> >=C2=A0 =C2=A0 =C2=A0hw.bus.disable_failed_devices=3D1 in either a hint = via the loader, or at<br> >=C2=A0 =C2=A0 =C2=A0runtime with a sysctl setting. Setting from 1 ->= 0 at runtime will not<br> >=C2=A0 =C2=A0 =C2=A0affect previously disabled devices, however: they r= emain disabled.<br> >=C2=A0 =C2=A0 =C2=A0They can be re-enabled manually with devctl enable,= however.<br> >=C2=A0 =C2=A0 =C2=A0<br> >=C2=A0 =C2=A0 =C2=A0Sponsored by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0Netflix<br> >=C2=A0 =C2=A0 =C2=A0<br> >=C2=A0 =C2=A0 =C2=A0Reviewed by:=C2=A0 =C2=A0 gallatin, hselasky, jhb<b= r> >=C2=A0 =C2=A0 =C2=A0Differential Revision:=C2=A0 <a href=3D"https://rev= iews.freebsd.org/D37517" rel=3D"noreferrer noreferrer" target=3D"_blank">ht= tps://reviews.freebsd.org/D37517</a><br> > ---<br> >=C2=A0 sys/kern/subr_bus.c | 33 +++++++++++++++++++++++++++------<br> >=C2=A0 1 file changed, 27 insertions(+), 6 deletions(-)<br> ><br> > diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c<br> > index 739484ad849a..b9615b033007 100644<br> > --- a/sys/kern/subr_bus.c<br> > +++ b/sys/kern/subr_bus.c<br> > @@ -69,6 +69,10 @@ SYSCTL_NODE(_hw, OID_AUTO, bus, CTLFLAG_RW | CTLFLA= G_MPSAF<br> > E, NULL,<br> >=C2=A0 SYSCTL_ROOT_NODE(OID_AUTO, dev, CTLFLAG_RW | CTLFLAG_MPSAFE, NUL= L,<br> >=C2=A0 =C2=A0 =C2=A0 NULL);<br> >=C2=A0 <br> > +static bool disable_failed_devs =3D false;<br> > +SYSCTL_BOOL(_hw_bus, OID_AUTO, disable_failed_devices, CTLFLAG_RWTUN,= &disab<br> > le_failed_devs,<br> > +=C2=A0 =C2=A0 0, "Do not retry attaching devices that return an = error from DEVICE_ATTA<br> > CH the first time");<br> > +<br> >=C2=A0 /*<br> >=C2=A0 =C2=A0* Used to attach drivers to devclasses.<br> >=C2=A0 =C2=A0*/<br> > @@ -2533,12 +2537,29 @@ device_attach(device_t dev)<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0if ((error =3D DEVICE_ATTACH(dev)) !=3D 0) {= <br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0printf("dev= ice_attach: %s%d attach returned %d\n",<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0de= v->driver->name, dev->unit, error);<br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!(dev->flags &= amp; DF_FIXEDCLASS))<br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0devclass_delete_device(dev->devclass, dev);<br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(void)device_set_driv= er(dev, NULL);<br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0device_sysctl_fini(de= v);<br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0KASSERT(dev->busy = =3D=3D 0, ("attach failed but busy"));<br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->state =3D DS_= NOTPRESENT;<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (disable_failed_de= vs) {<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0/*<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * When the user has asked to disable failed devices, w<br> > e<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * directly disable the device, but leave it in the<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * attaching state. It will not try to probe/attach the<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * device further. This leaves the device numbering<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * intact for other similar devices in the system. It<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * can be removed from this state with devctl.<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 */<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0device_disable(dev);<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0/*<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * Otherwise, when attach fails, tear down the state<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * around that so we can retry when, for example, new<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * drivers are loaded.<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 */<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0if (!(dev->flags & DF_FIXEDCLASS))<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0devclass_delete_device(dev->devclass,= dev);<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0(void)device_set_driver(dev, NULL);<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0device_sysctl_fini(dev);<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0KASSERT(dev->busy =3D=3D 0, ("attach failed but busy"))= ;<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0dev->state =3D DS_NOTPRESENT;<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return (error);<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0dev->flags |=3D DF_ATTACHED_ONCE;<br> ><br> <br> When switching a KVM console from a FreeBSD machine to another machine <br> attached to the KVM the FreeBSD machine will panic. Photo attached.<br> <br> This probably needs an UPDATING entry explaining that <br> hw.bus.disable_failed_devices sysctl/kenv must be zero (0) to avoid the <br= > panic.<br> <br> Sorry for taking so long to report this. Had an appointment this morning.<b= r> <br> <br> Cheers,<br> Cy Schubert <<a href=3D"mailto:Cy.Schubert@cschubert.com" target=3D"_bla= nk" rel=3D"noreferrer">Cy.Schubert@cschubert.com</a>><br> FreeBSD UNIX:=C2=A0 <cy@FreeBSD.org>=C2=A0 =C2=A0Web:=C2=A0 <a href= =3D"https://FreeBSD.org" rel=3D"noreferrer noreferrer" target=3D"_blank">ht= tps://FreeBSD.org</a><br> NTP:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0<<a href=3D"mailto:cy@nwtim= e.org" target=3D"_blank" rel=3D"noreferrer">cy@nwtime.org</a>>=C2=A0 =C2= =A0 Web:=C2=A0 <a href=3D"https://nwtime.org" rel=3D"noreferrer noreferrer"= target=3D"_blank">https://nwtime.org</a><br>; <br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 e^(i*pi)+1=3D0<br> </blockquote></div> --00000000000013f21f05ef1b23ea--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfpiSYx3UK8zWpjQ1O3iOYFHHKLJuThN8RYAaLXm_YcNnA>