Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 19 Jan 2015 17:36:53 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r277391 - in head/sys: fs/devfs kern
Message-ID:  <201501191736.t0JHarYf044488@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Mon Jan 19 17:36:52 2015
New Revision: 277391
URL: https://svnweb.freebsd.org/changeset/base/277391

Log:
  Stop enforcing additional reference on all cdevs, which was introduced
  in r277199.  Acquire the neccessary reference in delist_dev_locked()
  and inform destroy_devl() about it using CDP_UNREF_DTR flag.
  
  Fix some style nits, add asserts.
  
  Discussed with:	hselasky
  Tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Modified:
  head/sys/fs/devfs/devfs_devs.c
  head/sys/fs/devfs/devfs_int.h
  head/sys/kern/kern_conf.c

Modified: head/sys/fs/devfs/devfs_devs.c
==============================================================================
--- head/sys/fs/devfs/devfs_devs.c	Mon Jan 19 17:24:52 2015	(r277390)
+++ head/sys/fs/devfs/devfs_devs.c	Mon Jan 19 17:36:52 2015	(r277391)
@@ -137,12 +137,6 @@ devfs_alloc(int flags)
 	vfs_timestamp(&ts);
 	cdev->si_atime = cdev->si_mtime = cdev->si_ctime = ts;
 	cdev->si_cred = NULL;
-	/*
-	 * Avoid race with dev_rel() by setting the initial
-	 * reference count to 1. This last reference is taken
-	 * by the destroy_dev() function.
-	 */
-	cdev->si_refcount = 1;
 
 	return (cdev);
 }

Modified: head/sys/fs/devfs/devfs_int.h
==============================================================================
--- head/sys/fs/devfs/devfs_int.h	Mon Jan 19 17:24:52 2015	(r277390)
+++ head/sys/fs/devfs/devfs_int.h	Mon Jan 19 17:36:52 2015	(r277391)
@@ -56,6 +56,7 @@ struct cdev_priv {
 	u_int			cdp_flags;
 #define CDP_ACTIVE		(1 << 0)
 #define CDP_SCHED_DTR		(1 << 1)
+#define	CDP_UNREF_DTR		(1 << 2)
 
 	u_int			cdp_inuse;
 	u_int			cdp_maxdirent;

Modified: head/sys/kern/kern_conf.c
==============================================================================
--- head/sys/kern/kern_conf.c	Mon Jan 19 17:24:52 2015	(r277390)
+++ head/sys/kern/kern_conf.c	Mon Jan 19 17:36:52 2015	(r277391)
@@ -116,6 +116,8 @@ dev_free_devlocked(struct cdev *cdev)
 
 	mtx_assert(&devmtx, MA_OWNED);
 	cdp = cdev2priv(cdev);
+	KASSERT((cdp->cdp_flags & CDP_UNREF_DTR) == 0,
+	    ("destroy_dev() was not called after delist_dev(%p)", cdev));
 	TAILQ_INSERT_HEAD(&cdevp_free_list, cdp, cdp_list);
 }
 
@@ -1035,6 +1037,7 @@ destroy_devl(struct cdev *dev)
 {
 	struct cdevsw *csw;
 	struct cdev_privdata *p;
+	struct cdev_priv *cdp;
 
 	mtx_assert(&devmtx, MA_OWNED);
 	KASSERT(dev->si_flags & SI_NAMED,
@@ -1043,7 +1046,18 @@ destroy_devl(struct cdev *dev)
 	    ("WARNING: Driver mistake: destroy_dev on eternal %d\n",
 	     dev2unit(dev)));
 
-	devfs_destroy(dev);
+	cdp = cdev2priv(dev);
+	if ((cdp->cdp_flags & CDP_UNREF_DTR) == 0) {
+		/*
+		 * Avoid race with dev_rel(), e.g. from the populate
+		 * loop.  If CDP_UNREF_DTR flag is set, the reference
+		 * to be dropped at the end of destroy_devl() was
+		 * already taken by delist_dev_locked().
+		 */
+		dev_refl(dev);
+
+		devfs_destroy(dev);
+	}
 
 	/* Remove name marking */
 	dev->si_flags &= ~SI_NAMED;
@@ -1103,19 +1117,27 @@ destroy_devl(struct cdev *dev)
 		}
 	}
 	dev->si_flags &= ~SI_ALIAS;
-	dev->si_refcount--;	/* Avoid race with dev_rel() */
+	cdp->cdp_flags &= ~CDP_UNREF_DTR;
+	dev->si_refcount--;
 
-	if (dev->si_refcount > 0) {
+	if (dev->si_refcount > 0)
 		LIST_INSERT_HEAD(&dead_cdevsw.d_devs, dev, si_list);
-	} else {
+	else
 		dev_free_devlocked(dev);
-	}
 }
 
 static void
 delist_dev_locked(struct cdev *dev)
 {
+	struct cdev_priv *cdp;
 	struct cdev *child;
+
+	mtx_assert(&devmtx, MA_OWNED);
+	cdp = cdev2priv(dev);
+	if ((cdp->cdp_flags & CDP_UNREF_DTR) != 0)
+		return;
+	cdp->cdp_flags |= CDP_UNREF_DTR;
+	dev_refl(dev);
 	devfs_destroy(dev);
 	LIST_FOREACH(child, &dev->si_children, si_siblings)
 		delist_dev_locked(child);
@@ -1124,6 +1146,7 @@ delist_dev_locked(struct cdev *dev)
 void
 delist_dev(struct cdev *dev)
 {
+
 	dev_lock();
 	delist_dev_locked(dev);
 	dev_unlock();

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201501191736.t0JHarYf044488>