Re: [PATCH] misc: use a proper range for minor number dynamic allocation

[Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>]

[-- Attachment #1: Type: text/plain, Size: 3141 bytes --]

On Mon, Nov 09, 2009 at 01:28:36PM -0800, Andrew Morton wrote:
> On Fri, 23 Oct 2009 21:28:17 -0200
> Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com> wrote:
> 
> > The current dynamic allocation of minor number for misc devices has some
> > drawbacks.
> > 
> > First of all, the range for dynamic numbers include some statically
> > allocated numbers. It goes from 63 to 0, and we have numbers in the
> > range from 1 to 15 already allocated. Although, it gives priority to the
> > higher and not allocated numbers, we may end up in a situation where we
> > must reject registering a driver which got a static number because a
> > driver got its number with dynamic allocation. Considering fs/dlm/user.c
> > allocates as many misc devices as lockspaces are created, and that we
> > have more than 50 users around, it's not unreasonable to reach that
> > situation.
> 
> What is this DLM behaviour of which you speak?  It sounds broken.
> 

I took a quick look at it, so I may be wrong. The code path is the
following: userspace writes command DLM_USER_CREATE_LOCKSPACE, which
calls device_create_lockspace, which calls dlm_device_register, which
does the following:

        ls->ls_device.fops = &device_fops;
        ls->ls_device.minor = MISC_DYNAMIC_MINOR;

        error = misc_register(&ls->ls_device);
        if (error) {
                kfree(ls->ls_device.name);
        }
fail:
        return error;

So, it will probably return EBUSY right now with about 64 devices at the
most. My patch does not fix this, but avoids conflicts with drivers with
statically allocated minors which come in late in the init process.

> > The proposed solution uses the not yet reserved range from 64 to 127. If
> > more devices are needed, we may push 64 to 16. Moreover, if we don't
> > need to give priority to the higher numbers anymore, we can start using
> > the bitmap/bitops functions.
> 
> So...  misc minors 64 to 127 are presently unused?
> 

As documented at Documentation/devices.txt, yes.

 10 char        Non-serial mice, misc features
[...]
                 15 = /dev/touchscreen/mk712    MK712 touchscreen
                128 = /dev/beep         Fancy beep device

> > Finally, if there's a failure creating the device (because there's
> > already one with the same name, for example), the current implementation
> > does not clear the bit for the allocated minor and that number is lost
> > for future allocations.
> > 
> 
> Is that a bugfix for the existing code?
> 
> If so, please split that out into a separate patch which we can review
> and apply promptly while we consider the broader problem which you've
> identified.

We could consider buggy the caller which asks for the same device name
more than once, without unregistering the first device. But better safe
than sorry: we should protect the correct drivers from the buggy ones
and avoid a depletion of the minor numbers. And, in case the driver core
returns another error for another reason (from device_create), we do the
right thing.

I will send it right now.

Regards,
Cascardo.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]