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
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);
}
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
