Date: Tue, 27 May 2008 15:06:15 +0200 From: Ed Schouten <ed@80386.nl> To: arch@FreeBSD.org Cc: kib@FreeBSD.org Subject: Simplifying devfs: minor == unit Message-ID: <20080527130615.GJ64397@hoeg.nl>
next in thread | raw e-mail | index | archive | help
--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 <ed@80386.nl>
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 <sys/systm.h>
#include <sys/file.h>
#include <sys/filedesc.h>
-#include <sys/ioctl_compat.h>
#include <sys/termios.h>
=20
#include <compat/svr4/svr4.h>
--- 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 <sys/conf.h>
#include <sys/kernel.h>
#include <sys/systm.h>
+#include <sys/limits.h>
#include <sys/malloc.h>
#include <sys/ctype.h>
#include <sys/sbuf.h>
@@ -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 <sys/fcntl.h>
#include <sys/kernel.h>
#include <sys/kthread.h>
+#include <sys/limits.h>
#include <sys/linker.h>
#include <sys/lock.h>
#include <sys/malloc.h>
@@ -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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080527130615.GJ64397>