Date: Mon, 12 May 2008 00:48:33 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Andriy Gapon <avg@icyb.net.ua> Cc: freebsd-hackers@freebsd.org Subject: Re: devctl (alike?) for devfs Message-ID: <20080511214833.GB18958@deviant.kiev.zoral.com.ua> In-Reply-To: <48275C0C.2040601@icyb.net.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>
next in thread | previous in thread | raw e-mail | index | archive | help
--GURrebd5m7w1Nnt/
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sun, May 11, 2008 at 11:50:20PM +0300, Andriy Gapon wrote:
>=20
> Kostik, John, Warner,
>=20
> thank you for your guidance and suggestions.
> I am currently testing the patch attached and I am using a kernel with=20
> WITNESS and INVARIANTS enabled.
> Scope of my testing is plugging/unplugging of UMASS devices.
> I get CREATE notifications all right.
> I do not get any panics/complaints from the kernel, good.
> Unfortunately I do not get any DESTROY notifications either.
>=20
> Could you please look through the patch? Is there any control flow path=
=20
> that I missed or something even more obvious?
> I hope that we do not have any cdev leaks.
>=20
> I am testing the patch with RELENG_7 as of May 6.
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().
>=20
> --=20
> 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);
> =20
> while ((cdp =3D TAILQ_FIRST(&cdp_free)) !=3D 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 =3D 1;
> }
> dev_unlock();
> - if (flag)
> + if (flag) {
> + if (!cold)
> + devctl_notify("DEVFS", dev->si_name, "DESTROY", NULL);
> +
> devfs_free(dev);
> + }
> }
> =20
> 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);
> }
> =20
> @@ -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);
> }
> =20
--GURrebd5m7w1Nnt/
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)
iEYEARECAAYFAkgnabAACgkQC3+MBN1Mb4ihSgCfRQjZSXKDLinsfMuGXTEx+kbZ
JcgAoN3qatcTfVTT3ABTzjXn3TUm9Ijs
=FBo+
-----END PGP SIGNATURE-----
--GURrebd5m7w1Nnt/--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080511214833.GB18958>