From owner-freebsd-arch@FreeBSD.ORG Tue May 27 13:07:34 2008 Return-Path: Delivered-To: arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 31F451065672; Tue, 27 May 2008 13:07:34 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: from palm.hoeg.nl (mx0.hoeg.nl [IPv6:2001:610:652::211]) by mx1.freebsd.org (Postfix) with ESMTP id 29AB38FC1F; Tue, 27 May 2008 13:07:33 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: by palm.hoeg.nl (Postfix, from userid 1000) id B78301CC1A; Tue, 27 May 2008 15:06:15 +0200 (CEST) Date: Tue, 27 May 2008 15:06:15 +0200 From: Ed Schouten To: arch@FreeBSD.org Message-ID: <20080527130615.GJ64397@hoeg.nl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="08ATZu8fEq0x2T3M" Content-Disposition: inline User-Agent: Mutt/1.5.18 (2008-05-17) Cc: kib@FreeBSD.org Subject: Simplifying devfs: minor == unit X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 May 2008 13:07:34 -0000 --08ATZu8fEq0x2T3M Content-Type: multipart/mixed; boundary="liqSWPDvh3eyfZ9k" Content-Disposition: inline --liqSWPDvh3eyfZ9k Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello everyone, Right before I left to BSDCan I was looking at the devfs code. When I started hacking the TTY code, I discovered minor/unit numbers are still actively used within the FreeBSD kernel, even though they are never exposed to userspace. Devfs automatically generates an inode number for each device. Right now, st_rdev is always equal to st_ino, which still guarantees device numbers are unique throughout the system. In an experimental branch in Perforce, I decided to see what would happen if I would completely remove minor numbers from device drivers. This means make_dev()'s minor argument is removed, but also the minor(), unit2minor(), etc. functions. Drivers could use si_drv0 directly, just like si_drv1 and si_drv2 are used right now. This doesn't seem to be possible because of the design of the device cloner (not the eventhandler, just the clone_* routines), which preallocates an unnamed device with a specific unit numer, which can later be passed to make_dev(). This is why I want to do this in little steps right now. I was thinking about doing the following: - si_drv0 currently contains the minor number. We could alter the minor2unit(), etc. routines to make minor numbers equal to unit numbers. This means most routines will now become a no-op. See attachment. - When that hits the tree, we could decide to run a big regexp on the source code to make drivers use si_drv0 directly. - I've seen most drivers only use the device cloner, because they need descriptor local storage. It turns out more drivers need this than I initially thought. kib@ has a patch for this, so I hope this gets committed one of these {days,weeks,months}. - After we've got file descriptor local storage, I think we can live without the cloner. This means we could consider removing the minor number argument from make_dev(), removing the unique unit number restriction we currently have inside devfs, which causes many drivers to use number pools for no obvious reason. I was thinking about discussing this patch with my mentor + committing it somewhere in the nearby future. Any comments? --=20 Ed Schouten WWW: http://80386.nl/ --liqSWPDvh3eyfZ9k Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="devfs-unit-is-minor.diff" Content-Transfer-Encoding: quoted-printable --- sys/compat/svr4/svr4_ttold.c +++ sys/compat/svr4/svr4_ttold.c @@ -36,7 +36,6 @@ #include #include #include -#include #include =20 #include --- sys/dev/ieee488/upd7210.c +++ sys/dev/ieee488/upd7210.c @@ -277,7 +277,7 @@ struct cdev *dev; =20 if (units =3D=3D NULL) - units =3D new_unrhdr(0, minor2unit(MAXMINOR), NULL); + units =3D new_unrhdr(0, INT_MAX, NULL); u->unit =3D alloc_unr(units); mtx_init(&u->mutex, "gpib", NULL, MTX_DEF); u->cdev =3D make_dev(&gpib_l_cdevsw, u->unit, --- sys/dev/led/led.c +++ sys/dev/led/led.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -298,7 +299,7 @@ led_drvinit(void *unused) { =20 - led_unit =3D new_unrhdr(0, minor2unit(MAXMINOR), NULL); + led_unit =3D new_unrhdr(0, INT_MAX, NULL); mtx_init(&led_mtx, "LED mtx", NULL, MTX_DEF); sx_init(&led_sx, "LED sx"); callout_init(&led_ch, CALLOUT_MPSAFE); --- sys/dev/md/md.c +++ sys/dev/md/md.c @@ -64,6 +64,7 @@ #include #include #include +#include #include #include #include @@ -1234,7 +1235,7 @@ md_preloaded(ptr, len); sx_xunlock(&md_sx); } - status_dev =3D make_dev(&mdctl_cdevsw, MAXMINOR, UID_ROOT, GID_WHEEL, + status_dev =3D make_dev(&mdctl_cdevsw, INT_MAX, UID_ROOT, GID_WHEEL, 0600, MDCTL_NAME); g_topology_lock(); } --- sys/geom/geom_dev.c +++ sys/geom/geom_dev.c @@ -88,7 +88,7 @@ g_dev_init(struct g_class *mp) { =20 - unithdr =3D new_unrhdr(0, minor2unit(MAXMINOR), NULL); + unithdr =3D new_unrhdr(0, INT_MAX, NULL); } =20 void --- sys/kern/kern_conf.c +++ sys/kern/kern_conf.c @@ -500,7 +500,7 @@ { if (x =3D=3D NULL) return NODEV; - return(x->si_drv0 & MAXMINOR); + return (x->si_drv0); } =20 int @@ -509,23 +509,21 @@ =20 if (x =3D=3D NULL) return NODEV; - return (minor2unit(minor(x))); + return (x->si_drv0); } =20 u_int minor2unit(u_int _minor) { =20 - KASSERT((_minor & ~MAXMINOR) =3D=3D 0, ("Illegal minor %x", _minor)); - return ((_minor & 0xff) | ((_minor >> 8) & 0xffff00)); + return (_minor); } =20 int unit2minor(int unit) { =20 - KASSERT(unit <=3D 0xffffff, ("Invalid unit (%d) in unit2minor", unit)); - return ((unit & 0xff) | ((unit << 8) & ~0xffff)); + return (unit); } =20 static void @@ -581,16 +579,18 @@ return (si); } =20 +#define UMINORMASK 0xffff00ffU + int uminor(dev_t dev) { - return (dev & MAXMINOR); + return (dev & UMINORMASK); } =20 int umajor(dev_t dev) { - return ((dev & ~MAXMINOR) >> 8); + return ((dev & ~UMINORMASK) >> 8); } =20 static void @@ -690,9 +690,6 @@ struct cdev *dev; int i; =20 - KASSERT((minornr & ~MAXMINOR) =3D=3D 0, - ("Invalid minor (0x%x) in make_dev", minornr)); - dev =3D devfs_alloc(); dev_lock(); prep_cdevsw(devsw); --- sys/sys/conf.h +++ sys/sys/conf.h @@ -221,8 +221,6 @@ =20 #define NUMCDEVSW 256 =20 -#define MAXMINOR 0xffff00ffU - struct module; =20 struct devsw_module_data { --liqSWPDvh3eyfZ9k-- --08ATZu8fEq0x2T3M Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkg8B0cACgkQ52SDGA2eCwUkEQCfcVCSxUf5HfFMFXaGLMiDjagA 3B0An17ubeeZNfq8SaY1qLEaw1VbetBW =i2CN -----END PGP SIGNATURE----- --08ATZu8fEq0x2T3M--