Simplifying devfs: minor == unit

[Kostik Belousov]

On Tue, May 27, 2008 at 06:57:53PM +0200, Ed Schouten wrote:
> * Kostik Belousov <kostikbel at gmail.com> wrote:
> > > - 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}.
> > The patch was committed ~ a week ago.
> 
> Great. Looks like I wasn't paying attention back then.
> 
> > > - 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 think we cannot live without clones regardless of devfs_cdevpriv.
> > The model assumed for the pty, snp and probably several other devices
> > actually requires new cdev instead of the priv data.
> 
> The pty driver does not use the clone_* interface. It only uses the
> eventhandler, which should indeed be left intact. The snp driver does
> use the clone_* interface, but not in a way that can't be done using the
> eventhandler, validating the device name and calling make_dev()
> directly.
Ok.

> 
> Please take a look at src/usr.sbin/watch/watch.c:open_snp(). We might as
> well turn snp(4) into a single /dev/snp, where the kernel space driver
> uses per-descriptor data to distinguish the instances. This provides
> some advantages:
> 
> - No more silly open()-loops.
> 
> - A system administrator can change the permissions on /dev/snp, which
>   automatically sets a system wide policy, instead on one of the device
>   nodes.
> 
> - We don't fill up the system with a lot of unused nodes.
> 
> 	for i in `seq 1000`
> 	do
> 		ls /dev/bpf$i > /dev/null
> 	done
Please, do not overuse the cdevpriv data (I am not speaking about snp/watch
ATM, each case requires careful decision). Using cdevpriv disables some
features that may be provided by the clones, i.e. actual cdevs. For example,
you cannot have several independent opens operate on the same instance.

> 
> > > I was thinking about discussing this patch with my mentor + committing
> > > it somewhere in the nearby future. Any comments?
> > 
> > Making minor == unit number looks to be not a bad idea, please, look at
> > the saga of the tty_pty.c revs. 1.153, 156, 1.157. Making the devices use
> > si_drv0 directly probably is not so good since we remove the indirection
> > layer that is already present and allows for some (minor) freedom in the
> > devfs/kern_conf implementation.
> 
> But why isn't this done for si_drv1 and si_drv2 then? My idea is to turn
> si_drv0 in an integer field that can be freely used. There is reason to
> force a policy on this field.

I would argue that si_drv{1,2} are the warts. I do not want to point
finger at the drivers, but I remember there is a representative subset
of them using si_drv in the racy way when simultaneous open/close is
performed.

I would much prefer to have some KPI there instead of the direct access
into cdev members.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20080527/0d570ab1/attachment.pgp