From nobody Mon Dec 5 21:01:14 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4NQx7d00l8z4jYJH for ; Mon, 5 Dec 2022 21:11:57 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ej1-x629.google.com (mail-ej1-x629.google.com [IPv6:2a00:1450:4864:20::629]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4NQx7c512Xz3q08 for ; Mon, 5 Dec 2022 21:11:56 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ej1-x629.google.com with SMTP id vp12so1512593ejc.8 for ; Mon, 05 Dec 2022 13:11:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20210112.gappssmtp.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=XvAVTvue6uc6nLg1TWOw4I4rMoTIrvaoKspTfotAQpU=; b=BZ8ertc97vxZ2G/a3RKbLNvJHOxBwej5Uzq6Uv0W1om9k4qUaUtKy1f7su6UM1EDXm ymwPU+yqmKWgQuLvhLY58M3+uv+6ZYdJKTl6bQV/RVBMIrUyTs+j+9SOEuXz+DyKyafu EOQ3Ug06ZpHUBBPYnY1eaDuBN41DSFbFVvkbRt4xEBLIlYY2RernBJkh5drdD9O12I68 s2bFkk8u7kLez0Kn2dqmLmpI23PnIZD6t+S39HkIB7JXKNvmCKrlFazmEzU+k+39ipFS uer0eD5nS0LZpN5K/4tNW+4cWWu1CDhCy/eZanseI4TM/tR/AQXP1947LHd+BD/ijQXc Jtew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=XvAVTvue6uc6nLg1TWOw4I4rMoTIrvaoKspTfotAQpU=; b=jAIbfi+AnEoykNedDq12nTjQ/KpJc2Q9AcZQXOjKfxBfqHTLi8119XkmUsHVBW0PQM rJt2t1H0uEACOivtwHc7hMkHgSxzg635gMbtLbhF6qhHUPeZrB8J32qj/VZUJpPZOZj7 ea7NgoRBO+HiehlpKF3wkzsfLR8IFZ811tLYQ8F0lfcCvYmCIks/CCDAJMQ0Wc2WQ/rk VoiY1AFa+kCTg7pT6zpK7FXStiSVRsdZ8ZuulvYTe1zXkYMyUPfSe6vZSNSeC/4NXIIH bxFBxU5KrQSxd6i95CzYbg0ejXfjorDN/+0zO+F3N1AlNEyM6TPRIn812JcDswSyr13u t4+w== X-Gm-Message-State: ANoB5pnU2OTuxpaFvM2KPTYLIh0zfwvCUiKjTtRvmWCt5j8nOcmUHudV wbeZgKBYjrJP2OWkYCs59SpWYzE5UP0R9n1NcJtwjh8Qf9cjSA== X-Google-Smtp-Source: AA0mqf70i5+PQHed2pNC8IrCE5ePLf0y64B08x7Dj88iJ/ytnEz89E3SovcanvS/oiN49l84s6C82YVEc8rGdg5zG8U= X-Received: by 2002:a17:906:5908:b0:7c0:fa92:713 with SMTP id h8-20020a170906590800b007c0fa920713mr3920147ejq.634.1670274715210; Mon, 05 Dec 2022 13:11:55 -0800 (PST) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202212042333.2B4NX1JM089126@gitrepo.freebsd.org> <20221205195134.08074206@slippy.cwsent.com> In-Reply-To: <20221205195134.08074206@slippy.cwsent.com> From: Warner Losh Date: Mon, 5 Dec 2022 14:01:14 -0700 Message-ID: Subject: Re: git: aa52c6bdd7b1 - main - newbus: Create a knob to disable devices that fail to attach. To: Cy Schubert Cc: Warner Losh , src-committers , "" , dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="00000000000013f21f05ef1b23ea" X-Rspamd-Queue-Id: 4NQx7c512Xz3q08 X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N --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 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 > > AuthorDate: 2022-12-04 23:20:24 +0000 > > Commit: Warner Losh > > 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 > FreeBSD UNIX: Web: https://FreeBSD.org > NTP: Web: https://nwtime.org > > e^(i*pi)+1=0 > --00000000000013f21f05ef1b23ea Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
I think I'll just revert the default change... but in= a few hours when I'm back home.

Warner=C2=A0

On Mon, Dec 5, 2022, 12:51 PM Cy Schubert <Cy.Schubert@cschubert.com> = wrote:
In message <202212042333.2B4NX1JM089126@gitrepo.freebsd.org>, Wa= rner Losh
write
s:
> The branch main has been updated by imp:
>
> URL: https= ://cgit.FreeBSD.org/src/commit/?id=3Daa52c6bdd7b157b7c5d9612e4feac5c9 > 4f376df6
>
> commit aa52c6bdd7b157b7c5d9612e4feac5c94f376df6
> Author:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org>
> AuthorDate: 2022-12-04 23:20:24 +0000
> Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org>
> CommitDate: 2022-12-04 23:20:24 +0000
>
>=C2=A0 =C2=A0 =C2=A0newbus: Create a knob to disable devices that fail = to attach.
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0Normally, when a device fails to attach, we tear do= wn the newbus state
>=C2=A0 =C2=A0 =C2=A0for that device so that future driver loads can try= again (maybe with a
>=C2=A0 =C2=A0 =C2=A0different driver, or maybe with a re-loaded and fix= ed kld).
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0Sometimes, however, it is desirable to have the dev= ice fail
>=C2=A0 =C2=A0 =C2=A0permanantly. We do this by calling device_disable()= on a failed
>=C2=A0 =C2=A0 =C2=A0attached, as well as keeping the device in DS_ATTAC= HING forever. This
>=C2=A0 =C2=A0 =C2=A0prevents retries on that device. This is enabled vi= a
>=C2=A0 =C2=A0 =C2=A0hw.bus.disable_failed_devices=3D1 in either a hint = via the loader, or at
>=C2=A0 =C2=A0 =C2=A0runtime with a sysctl setting. Setting from 1 ->= 0 at runtime will not
>=C2=A0 =C2=A0 =C2=A0affect previously disabled devices, however: they r= emain disabled.
>=C2=A0 =C2=A0 =C2=A0They can be re-enabled manually with devctl enable,= however.
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0Sponsored by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0Netflix
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0Reviewed by:=C2=A0 =C2=A0 gallatin, hselasky, jhb >=C2=A0 =C2=A0 =C2=A0Differential Revision:=C2=A0 ht= tps://reviews.freebsd.org/D37517
> ---
>=C2=A0 sys/kern/subr_bus.c | 33 +++++++++++++++++++++++++++------
>=C2=A0 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 | CTLFLA= G_MPSAF
> E, NULL,
>=C2=A0 SYSCTL_ROOT_NODE(OID_AUTO, dev, CTLFLAG_RW | CTLFLAG_MPSAFE, NUL= L,
>=C2=A0 =C2=A0 =C2=A0 NULL);
>=C2=A0
> +static bool disable_failed_devs =3D false;
> +SYSCTL_BOOL(_hw_bus, OID_AUTO, disable_failed_devices, CTLFLAG_RWTUN,= &disab
> le_failed_devs,
> +=C2=A0 =C2=A0 0, "Do not retry attaching devices that return an = error from DEVICE_ATTA
> CH the first time");
> +
>=C2=A0 /*
>=C2=A0 =C2=A0* Used to attach drivers to devclasses.
>=C2=A0 =C2=A0*/
> @@ -2533,12 +2537,29 @@ device_attach(device_t dev)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if ((error =3D DEVICE_ATTACH(dev)) !=3D 0) {=
>=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",
>=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);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!(dev->flags &= amp; DF_FIXEDCLASS))
> -=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);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(void)device_set_driv= er(dev, NULL);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0device_sysctl_fini(de= v);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0KASSERT(dev->busy = =3D=3D 0, ("attach failed but busy"));
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->state =3D DS_= NOTPRESENT;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (disable_failed_de= vs) {
> +=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=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
> e
> +=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
> +=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
> +=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
> +=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
> +=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.
> +=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=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0device_disable(dev);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> +=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=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 * Otherwise, when attach fails, tear down the state
> +=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
> +=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.
> +=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=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0if (!(dev->flags & DF_FIXEDCLASS))
> +=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);
> +=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);
> +=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);
> +=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"))= ;
> +=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;
> +=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=A0return (error);<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0 =C2=A0 =C2=A0dev->flags |=3D 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:=C2=A0 <cy@FreeBSD.org>=C2=A0 =C2=A0Web:=C2=A0 ht= tps://FreeBSD.org
NTP:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0<cy@nwtime.org>=C2=A0 =C2= =A0 Web:=C2=A0 https://nwtime.org

=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
--00000000000013f21f05ef1b23ea--