From owner-freebsd-hackers@FreeBSD.ORG Tue May 13 19:05:04 2008 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E5DF71065686; Tue, 13 May 2008 19:05:04 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from hosted.kievnet.com (hosted.kievnet.com [193.138.144.10]) by mx1.freebsd.org (Postfix) with ESMTP id 81F088FC12; Tue, 13 May 2008 19:05:04 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from localhost ([127.0.0.1] helo=edge.pp.kiev.ua) by hosted.kievnet.com with esmtpa (Exim 4.62) (envelope-from ) id 1Jvzo5-0000Uv-QP; Tue, 13 May 2008 22:05:01 +0300 Message-ID: <4829E658.3000605@icyb.net.ua> Date: Tue, 13 May 2008 22:04:56 +0300 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.12 (X11/20080320) MIME-Version: 1.0 To: Kostik Belousov 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> In-Reply-To: <20080511214833.GB18958@deviant.kiev.zoral.com.ua> Content-Type: multipart/mixed; boundary="------------080908040200020205000508" Cc: freebsd-hackers@freebsd.org Subject: Re: devctl (alike?) for devfs X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 May 2008 19:05:05 -0000 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 #include #include +#include #include #include #include @@ -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--