Am Donnerstag, 25. April 2002 17:11 schrieb Greg KH:
> On Thu, Apr 25, 2002 at 10:14:59AM +0200, Oliver Neukum wrote:
> > > Right now there isn't a "simple" way for userspace to realize what
> > > minor is assigned to each device, but I will work on that tomorrow,
> > > as it's late (hint, /sbin/hotplug will be involved :)
> >
> > This is no good. You come to a system of extreme brittleness this way.
> > There's no way to resynchronise. You need a way to query all
> > assigned minors at any time from any application from the kernel.
>
> Why is that a requirement?  I've heard this argument before quite
> frequently from the people I work with (old Dynix/PTX engineers which
> had such a system.)  If you have a working userspace deamon that keeps
> track of such things properly, you do not need to burden the kernel with
> such a database.  devfs doesn't have this "feature" :)

That means that you absolutely depend on that demon always working.
There's no fault tolerance at all. This means, for example that
any script failing due to oom or an io error opens a potential
security risk.

I really want to see a scheme that works securely. And I won't
even mention the issue of races. You'll find a very long thread
between me and Adam Richter about this in the archives.

Devfs goes the whole way.
Let's look at the basic function of major:minor combinations.
You use them to assign names and permissions to a device.
If minors become dynamic minors no longer have any function,
you are better of eliminating them.
Your system as is has an obvious race. A new device will
have old arbitrary permissions before the hotplug script can reset them.

> But I'll leave the userspace interaction comments for later, as other
> people are working on solutions that USB will tie into.  That work is
> not yet finished, so I'll hold off for now.
>
> > > This patch is against 2.5.10.
> > >
> > > Comments?
> >
> > Oh yes.
> >
> > > diff -Nru a/drivers/usb/image/scanner.c
> > > b/drivers/usb/image/scanner.c --- a/drivers/usb/image/scanner.c   Thu
> > > Apr 25 00:01:15 2002
> > > +++ b/drivers/usb/image/scanner.c Thu Apr 25 00:01:15 2002
> > > @@ -953,9 +953,11 @@
> > >
> > >   down(&scn_mutex);
> > >
> > > - for (scn_minor = 0; scn_minor < SCN_MAX_MNR; scn_minor++) {
> > > -         if (!p_scn_table[scn_minor])
> > > -                 break;
> > > + if (usb_register_dev(&scanner_driver, 1, &scn_minor)) {
> > > +         for (scn_minor = 0; scn_minor < SCN_MAX_MNR; scn_minor++) {
> > > +                 if (!p_scn_table[scn_minor])
> > > +                         break;
> > > +         }
> > >   }
> >
> > What is this supposed to do ?
> > If you have freeable minors, free them.
> > Hoarding minors is sociopathic behaviour.
>
> Huh?  We are asking for a minor when we really have a device plugged in.
> That's all, no hoarding.  So in the probe() function, we call
> usb_register_dev() to get a minor, and in the disconnect, we call
> usb_deregister_dev() to free it up.

What is the loop supposed to do then ?
It has some comments about looking internally for a minor.

> > > @@ -642,8 +649,11 @@
> > >   minor = dev->minor;
> > >
> > >   /* remove our devfs node */
> > > - devfs_unregister(dev->devfs);
> > > + devfs_unregister (dev->devfs);
> > >
> > > + /* give back our dynamic minor */
> > > + usb_deregister_dev (&skel_driver, 1, minor);
> > > +
> > >   /* if the device is not opened, then we clean up right now */
> > >   if (!dev->open_count) {
> > >           up (&dev->sem);
> >
> > You are redefining the meaning of a minor while somebody
> > can still have the file open. Is this safe ?
>
> It's ok for devfs, why not for us?  :)

Some drivers keep an internal array to assign minors to device
descriptors.

> Or do you mean the minor table in the usb core.  Hm, yeah you're right,
> that probably needs a lock.  I'll go add that, thanks.

Yes.

> > Last, but not least, this looks like an attempt to do a one off
> > solution to a common problem. In other words, your are frantic to
> > avoid devfs.
>
> No, this is orthogonal to devfs.  devfs still works just fine, and
> doesn't reuse minor numbers.  Even using devfs you still have the
> "limited to 16 minors" problem that this solution solves.  All devfs
> does for the USB major is to dynamically create the /dev nodes, which is
> much different from this.  Hey look, I don't even touch the devfs calls
> in the usb drivers :)

You should have.
You allow yourself to be bound to the concept of minors
without regard for meaning.
In fact your solution gets you the worst of both worlds.
You no longer have easy, simple persistent storage of names
and permissions in /dev. In fact you have now stale names
and permissions on disk.
Yet you have none of the features devfs offers.

        Regards
                Oliver


_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to