Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 13 May 2008 22:04:56 +0300
From:      Andriy Gapon <avg@icyb.net.ua>
To:        Kostik Belousov <kostikbel@gmail.com>
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: devctl (alike?) for devfs
Message-ID:  <4829E658.3000605@icyb.net.ua>
In-Reply-To: <20080511214833.GB18958@deviant.kiev.zoral.com.ua>
References:  <480E4269.2090604@icyb.net.ua> <480FBAB9.1000904@icyb.net.ua> <48103F36.6060707@icyb.net.ua> <200804240811.26183.jhb@freebsd.org> <4810FD1E.70602@icyb.net.ua> <20080425095009.GD18958@deviant.kiev.zoral.com.ua> <4811E6BC.4060306@icyb.net.ua> <20080425143646.GF18958@deviant.kiev.zoral.com.ua> <48275C0C.2040601@icyb.net.ua> <20080511214833.GB18958@deviant.kiev.zoral.com.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------080908040200020205000508
Content-Type: text/plain; charset=KOI8-U; format=flowed
Content-Transfer-Encoding: 7bit

on 12/05/2008 00:48 Kostik Belousov said the following:
> No, we do not have a leak, but we have somewhat non-obvious behaviour.
> 
> The cdev structure is freed only after the last reference to cdev is
> gone. Typical holder of the reference is the devfs vnode. In the normal
> usage, the vnode is present until both the device is destroyed _and_
> devfs_populate_loop() run is performed. This function actually reclaim
> the vnodes for destroyed devices, that causes last reference to cdev to
> be dropped and memory freed.
> 
> The populate loop is called syncronously from the upper levels. The
> easiest method to trigger it is to do ls /dev, since it is called from
> the devfs_lookupx().

Thank you for the explanation! ls did trigger DESTROY notifications.
But this arbitrary delay between device entry going away and 
notification about about it is a little bit "uncool".

So I re-wrote the patch to post notifications about deleted devices with 
si_refcount > 0 in a fashion similar to how si_refcount=0 devices are 
freed. I put those cdev-s onto a special list (re-/ab-using si_siblings 
LIST link field) and bump their si_refcount to prevent them from getting 
destroyed before the notification is sent (it would need si_name).
Then, in dev_unlock_and_free() I send notifications and call dev_rel on 
the devices.

I am not entirely satisfied with the code:
1. I don't like the way I move LIST elements from one head to the other, 
this should be a macro in queue.h
2. I am not sure about the names I picked
3. I slightly don't like a fact that parent-child destroy notifications 
are sent in reverse order because of my simplistic LIST usage.

-- 
Andriy Gapon

--------------080908040200020205000508
Content-Type: text/plain;
 name="devfs-notify.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="devfs-notify.diff"

diff --git a/sys/kern/kern_conf.c b/sys/kern/kern_conf.c
index 1db25f8..0245253 100644
--- a/sys/kern/kern_conf.c
+++ b/sys/kern/kern_conf.c
@@ -30,6 +30,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/param.h>
 #include <sys/kernel.h>
 #include <sys/systm.h>
+#include <sys/bus.h>
 #include <sys/bio.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
@@ -99,6 +100,9 @@ dev_unlock_and_free(void)
 	mtx_unlock(&devmtx);
 
 	while ((cdp = TAILQ_FIRST(&cdp_free)) != NULL) {
+		if (!cold)
+			devctl_notify("DEVFS", cdp->cdp_c.si_name, "DESTROY", NULL);
+
 		TAILQ_REMOVE(&cdp_free, cdp, cdp_list);
 		devfs_free(&cdp->cdp_c);
 	}
@@ -172,8 +176,12 @@ dev_rel(struct cdev *dev)
 		flag = 1;
 	}
 	dev_unlock();
-	if (flag)
+	if (flag) {
+		if (!cold)
+			devctl_notify("DEVFS", dev->si_name, "DESTROY", NULL);
+
 		devfs_free(dev);
+	}
 }
 
 struct cdevsw *
@@ -706,6 +714,10 @@ make_dev_credv(int flags, struct cdevsw *devsw, int minornr,
 	devfs_create(dev);
 	clean_unrhdrl(devfs_inos);
 	dev_unlock_and_free();
+
+	if (!cold)
+		devctl_notify("DEVFS", dev->si_name, "CREATE", NULL);
+
 	return (dev);
 }
 
@@ -794,6 +806,10 @@ make_dev_alias(struct cdev *pdev, const char *fmt, ...)
 	clean_unrhdrl(devfs_inos);
 	dev_unlock();
 	dev_depends(pdev, dev);
+
+	if (!cold)
+		devctl_notify("DEVFS", dev->si_name, "CREATE", NULL);
+
 	return (dev);
 }
 

--------------080908040200020205000508--



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