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

[Andrew Morton]

On Mon, 9 Nov 2009 21:29:25 -0200
Thadeu Lima de Souza Cascardo <cascardo at holoscopio.com> wrote:

> On Mon, Nov 09, 2009 at 08:02:57PM -0200, Thadeu Lima de Souza Cascardo wrote:
> > > 
> > > 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.
> 
> I've just tried to create a single and separate patch, but that would
> let lots of related bugs around.
> 
> First of all, the current code does not use the bitmap idiom. Should I
> use it on my fix and let all the other bitmap manipulations as is, or
> should I use the current and less readable style?

If you think that the fix is needed in 2.6.32 and perhaps -stable then
yes please, something minimal is desired.

If you think that the bug is sufficiently minor that that the fix xcan
be deferred into 2.6.33 then no intermediate patch is needed.  Bear in
mind that others might disagree with your designation of the priority,
and they might want a fix which they can backport into their
2.6.29/2.6.30/etc kernels.

> Second, this single fix would match the same test currently in
> misc_deregister, which is broken, since it does not test for the 0
> minor.

I don't know what that means.

> Thus, I am sending a patch which fixes those two issues using the
> current style, a fix for the style itself, and a change from the current
> range to something that could have its range easily fixed.

Sounds good.

> However,
> regarding that last change, it will still use bitmaps, which may not be
> appropriate for large ranges.
> 
> Perhaps, using a idr instead of the list and bitmap couple, may be
> sensible. What do you think about it?

miscdev registration/deregistration is hardly a high-frequency
operation.  I'd opt for simplicity and compactness over speed here.