From owner-svn-src-stable@freebsd.org Sun Oct 20 22:01:36 2019 Return-Path: Delivered-To: svn-src-stable@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 012BF16C201; Sun, 20 Oct 2019 22:01:36 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46xDHz5wP2z4QKS; Sun, 20 Oct 2019 22:01:35 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id AF1AF1A597; Sun, 20 Oct 2019 22:01:35 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x9KM1Zk2057116; Sun, 20 Oct 2019 22:01:35 GMT (envelope-from kevans@FreeBSD.org) Received: (from kevans@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x9KM1Zpi056866; Sun, 20 Oct 2019 22:01:35 GMT (envelope-from kevans@FreeBSD.org) Message-Id: <201910202201.x9KM1Zpi056866@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kevans set sender to kevans@FreeBSD.org using -f From: Kyle Evans Date: Sun, 20 Oct 2019 22:01:35 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r353783 - in stable: 11/sys/kern 11/sys/sys 12/sys/kern 12/sys/sys X-SVN-Group: stable-11 X-SVN-Commit-Author: kevans X-SVN-Commit-Paths: in stable: 11/sys/kern 11/sys/sys 12/sys/kern 12/sys/sys X-SVN-Commit-Revision: 353783 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 20 Oct 2019 22:01:36 -0000 Author: kevans Date: Sun Oct 20 22:01:35 2019 New Revision: 353783 URL: https://svnweb.freebsd.org/changeset/base/353783 Log: MFC r353128-r353129: fully initialize cloned devices w/ make_dev_args r353128: kern_conf: fully initialize cloned devices with make_dev_args, too Attempting to initialize si_drv{1,2} with mda_si_drv{1,2} does not work if you are operating on cloned devices. clone_create must be called prior to the make_dev* family to create/return the device on the clonelist as needed. This device is later returned early in newdev(), prior to si_drv{0,1,2} initialization. This patch simply breaks out of the loop if we've found a device and finishes init. r353129: Remove the remnants of SI_CHEAPCLONE SI_CHEAPCLONE was introduced in r66067 for use with cloned bpfs. It was later also used in tty, tun, tap at points. The rough timeline for being removed in each of these is as follows: - r181690: bpf switched to use cdevpriv API by ed@ - r181905: ed@ rewrote the TTY later to be mpsafe - r204464: kib@ removes it from tun/tap, declaring it unused I've not yet been able to dig up any other consumers in the intervening 9 years. It is no longer set on any devices in the tree and leaves an interesting situation in make_dev_sv where we're ok with the device already being set SI_NAMED. Modified: stable/11/sys/kern/kern_conf.c stable/11/sys/sys/conf.h Directory Properties: stable/11/ (props changed) Changes in other areas also in this revision: Modified: stable/12/sys/kern/kern_conf.c stable/12/sys/sys/conf.h Directory Properties: stable/12/ (props changed) Modified: stable/11/sys/kern/kern_conf.c ============================================================================== --- stable/11/sys/kern/kern_conf.c Sun Oct 20 21:06:25 2019 (r353782) +++ stable/11/sys/kern/kern_conf.c Sun Oct 20 22:01:35 2019 (r353783) @@ -164,12 +164,6 @@ dev_rel(struct cdev *dev) dev->si_refcount--; KASSERT(dev->si_refcount >= 0, ("dev_rel(%s) gave negative count", devtoname(dev))); -#if 0 - if (dev->si_usecount == 0 && - (dev->si_flags & SI_CHEAPCLONE) && (dev->si_flags & SI_NAMED)) - ; - else -#endif if (dev->si_devsw == NULL && dev->si_refcount == 0) { LIST_REMOVE(dev, si_list); flag = 1; @@ -573,20 +567,41 @@ newdev(struct make_dev_args *args, struct cdev *si) mtx_assert(&devmtx, MA_OWNED); csw = args->mda_devsw; + si2 = NULL; if (csw->d_flags & D_NEEDMINOR) { /* We may want to return an existing device */ LIST_FOREACH(si2, &csw->d_devs, si_list) { if (dev2unit(si2) == args->mda_unit) { dev_free_devlocked(si); - return (si2); + si = si2; + break; } } + + /* + * If we're returning an existing device, we should make sure + * it isn't already initialized. This would have been caught + * in consumers anyways, but it's good to catch such a case + * early. We still need to complete initialization of the + * device, and we'll use whatever make_dev_args were passed in + * to do so. + */ + KASSERT(si2 == NULL || (si2->si_flags & SI_NAMED) == 0, + ("make_dev() by driver %s on pre-existing device (min=%x, name=%s)", + args->mda_devsw->d_name, dev2unit(si2), devtoname(si2))); } si->si_drv0 = args->mda_unit; - si->si_devsw = csw; si->si_drv1 = args->mda_si_drv1; si->si_drv2 = args->mda_si_drv2; - LIST_INSERT_HEAD(&csw->d_devs, si, si_list); + /* Only push to csw->d_devs if it's not a cloned device. */ + if (si2 == NULL) { + si->si_devsw = csw; + LIST_INSERT_HEAD(&csw->d_devs, si, si_list); + } else { + KASSERT(si->si_devsw == csw, + ("%s: inconsistent devsw between clone_create() and make_dev()", + __func__)); + } return (si); } @@ -796,17 +811,6 @@ make_dev_sv(struct make_dev_args *args1, struct cdev * dev_refl(dev); if ((args.mda_flags & MAKEDEV_ETERNAL) != 0) dev->si_flags |= SI_ETERNAL; - if (dev->si_flags & SI_CHEAPCLONE && - dev->si_flags & SI_NAMED) { - /* - * This is allowed as it removes races and generally - * simplifies cloning devices. - * XXX: still ?? - */ - dev_unlock_and_free(); - *dres = dev; - return (0); - } KASSERT(!(dev->si_flags & SI_NAMED), ("make_dev() by driver %s on pre-existing device (min=%x, name=%s)", args.mda_devsw->d_name, dev2unit(dev), devtoname(dev))); @@ -1541,7 +1545,6 @@ DB_SHOW_COMMAND(cdev, db_show_cdev) SI_FLAG(SI_ETERNAL); SI_FLAG(SI_ALIAS); SI_FLAG(SI_NAMED); - SI_FLAG(SI_CHEAPCLONE); SI_FLAG(SI_CHILD); SI_FLAG(SI_DUMPDEV); SI_FLAG(SI_CLONELIST); Modified: stable/11/sys/sys/conf.h ============================================================================== --- stable/11/sys/sys/conf.h Sun Oct 20 21:06:25 2019 (r353782) +++ stable/11/sys/sys/conf.h Sun Oct 20 22:01:35 2019 (r353783) @@ -57,7 +57,7 @@ struct cdev { #define SI_ETERNAL 0x0001 /* never destroyed */ #define SI_ALIAS 0x0002 /* carrier of alias name */ #define SI_NAMED 0x0004 /* make_dev{_alias} has been called */ -#define SI_CHEAPCLONE 0x0008 /* can be removed_dev'ed when vnode reclaims */ +#define SI_UNUSED1 0x0008 /* unused */ #define SI_CHILD 0x0010 /* child of another struct cdev **/ #define SI_DUMPDEV 0x0080 /* is kernel dumpdev */ #define SI_CLONELIST 0x0200 /* on a clone list */