[PATCH 1/3] misc: clear allocation bit in minor bitmap when device register fails

[PATCH 1/3] misc: clear allocation bit in minor bitmap when device register fails

[Thadeu Lima de Souza Cascardo]

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.

Second, the test currently in misc_deregister is broken, since it does
not test for the 0 minor.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/char/misc.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 07fa612..95d1282 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -219,6 +219,9 @@ int misc_register(struct miscdevice * misc)
 	misc->this_device = device_create(misc_class, misc->parent, dev,
 					  misc, "%s", misc->name);
 	if (IS_ERR(misc->this_device)) {
+		int i = misc->minor;
+		if (i < DYNAMIC_MINORS && i >= 0)
+			misc_minors[i>>3] &= ~(1 << (i & 7));
 		err = PTR_ERR(misc->this_device);
 		goto out;
 	}
@@ -253,9 +256,8 @@ int misc_deregister(struct miscdevice *misc)
 	mutex_lock(&misc_mtx);
 	list_del(&misc->list);
 	device_destroy(misc_class, MKDEV(MISC_MAJOR, misc->minor));
-	if (i < DYNAMIC_MINORS && i>0) {
-		misc_minors[i>>3] &= ~(1 << (misc->minor & 7));
-	}
+	if (i < DYNAMIC_MINORS && i >= 0)
+		misc_minors[i>>3] &= ~(1 << (i & 7));
 	mutex_unlock(&misc_mtx);
 	return 0;
 }
-- 
1.6.3.3

[PATCH 2/3] misc: use bitmap/bitops functions for dynamic minor number allocation

[Thadeu Lima de Souza Cascardo]

Use DECLARE_BITMAP, find_first_zero_bit, set_bit and clear_bit instead
of rewriting code to do it with the minor number dynamic allocation
bitmap.

We need to invert the bit position to keep the code behaviour of using
the last minor numbers first, since we don't have a find_last_zero_bit.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/char/misc.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 95d1282..59fff30 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -61,7 +61,7 @@ static DEFINE_MUTEX(misc_mtx);
  * Assigned numbers, used for dynamic minors
  */
 #define DYNAMIC_MINORS 64 /* like dynamic majors */
-static unsigned char misc_minors[DYNAMIC_MINORS / 8];
+static DECLARE_BITMAP(misc_minors, DYNAMIC_MINORS);
 
 extern int pmu_device_init(void);
 
@@ -201,27 +201,23 @@ int misc_register(struct miscdevice * misc)
 	}
 
 	if (misc->minor == MISC_DYNAMIC_MINOR) {
-		int i = DYNAMIC_MINORS;
-		while (--i >= 0)
-			if ( (misc_minors[i>>3] & (1 << (i&7))) == 0)
-				break;
-		if (i<0) {
+		int i = find_first_zero_bit(misc_minors, DYNAMIC_MINORS);
+		if (i >= DYNAMIC_MINORS) {
 			mutex_unlock(&misc_mtx);
 			return -EBUSY;
 		}
-		misc->minor = i;
+		misc->minor = DYNAMIC_MINORS - i - 1;
+		set_bit(i, misc_minors);
 	}
 
-	if (misc->minor < DYNAMIC_MINORS)
-		misc_minors[misc->minor >> 3] |= 1 << (misc->minor & 7);
 	dev = MKDEV(MISC_MAJOR, misc->minor);
 
 	misc->this_device = device_create(misc_class, misc->parent, dev,
 					  misc, "%s", misc->name);
 	if (IS_ERR(misc->this_device)) {
-		int i = misc->minor;
+		int i = DYNAMIC_MINORS - misc->minor - 1;
 		if (i < DYNAMIC_MINORS && i >= 0)
-			misc_minors[i>>3] &= ~(1 << (i & 7));
+			clear_bit(i, misc_minors);
 		err = PTR_ERR(misc->this_device);
 		goto out;
 	}
@@ -248,7 +244,7 @@ int misc_register(struct miscdevice * misc)
 
 int misc_deregister(struct miscdevice *misc)
 {
-	int i = misc->minor;
+	int i = DYNAMIC_MINORS - misc->minor - 1;
 
 	if (list_empty(&misc->list))
 		return -EINVAL;
@@ -257,7 +253,7 @@ int misc_deregister(struct miscdevice *misc)
 	list_del(&misc->list);
 	device_destroy(misc_class, MKDEV(MISC_MAJOR, misc->minor));
 	if (i < DYNAMIC_MINORS && i >= 0)
-		misc_minors[i>>3] &= ~(1 << (i & 7));
+		clear_bit(i, misc_minors);
 	mutex_unlock(&misc_mtx);
 	return 0;
 }
-- 
1.6.3.3

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

[Thadeu Lima de Souza Cascardo]

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.

The proposed solution uses the not yet reserved range from 64 to 127. If
more devices are needed, we may push 64 to 16.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/char/misc.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 59fff30..5681d3c 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -60,6 +60,7 @@ static DEFINE_MUTEX(misc_mtx);
 /*
  * Assigned numbers, used for dynamic minors
  */
+#define DYNAMIC_MINOR_BASE 64
 #define DYNAMIC_MINORS 64 /* like dynamic majors */
 static DECLARE_BITMAP(misc_minors, DYNAMIC_MINORS);
 
@@ -206,7 +207,7 @@ int misc_register(struct miscdevice * misc)
 			mutex_unlock(&misc_mtx);
 			return -EBUSY;
 		}
-		misc->minor = DYNAMIC_MINORS - i - 1;
+		misc->minor = DYNAMIC_MINOR_BASE + i;
 		set_bit(i, misc_minors);
 	}
 
@@ -215,7 +216,7 @@ int misc_register(struct miscdevice * misc)
 	misc->this_device = device_create(misc_class, misc->parent, dev,
 					  misc, "%s", misc->name);
 	if (IS_ERR(misc->this_device)) {
-		int i = DYNAMIC_MINORS - misc->minor - 1;
+		int i = misc->minor - DYNAMIC_MINOR_BASE;
 		if (i < DYNAMIC_MINORS && i >= 0)
 			clear_bit(i, misc_minors);
 		err = PTR_ERR(misc->this_device);
@@ -244,7 +245,7 @@ int misc_register(struct miscdevice * misc)
 
 int misc_deregister(struct miscdevice *misc)
 {
-	int i = DYNAMIC_MINORS - misc->minor - 1;
+	int i = misc->minor - DYNAMIC_MINOR_BASE;
 
 	if (list_empty(&misc->list))
 		return -EINVAL;
-- 
1.6.3.3

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

[H. Peter Anvin]

On 11/09/2009 04:30 PM, Thadeu Lima de Souza Cascardo 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.
> 
> The proposed solution uses the not yet reserved range from 64 to 127. If
> more devices are needed, we may push 64 to 16.
> 

Again, why not push these up above 256?

	-hpa

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

[Alan Cox]

> > The proposed solution uses the not yet reserved range from 64 to 127. If
> > more devices are needed, we may push 64 to 16.
> > 
> 
> Again, why not push these up above 256?

misc was always intended for one off devices not something creating 40 or
50 misc entries. DLM should be moved out of misc IMHO, or 256+ used for
misc "ranges" such as DLM.

Pushing the existing stuff above minor 256 risks breaking some old setups

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

[Thadeu Lima de Souza Cascardo]

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

On Mon, Nov 09, 2009 at 04:34:07PM -0800, H. Peter Anvin wrote:
> On 11/09/2009 04:30 PM, Thadeu Lima de Souza Cascardo 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.
> > 
> > The proposed solution uses the not yet reserved range from 64 to 127. If
> > more devices are needed, we may push 64 to 16.
> > 
> 
> Again, why not push these up above 256?
> 
> 	-hpa

I don't have a problem with this. However, as Alan has pointed out,
there may be old setups that rely on minor numbers below 256. I have
thought of that, but I'm sure there's argument against it. I've decided
to submit it more closely to what is done today, that is, 64 devices. As
I said, there's still space for 48 more devices, and, besides DLM, I
couldn't find at first glance any other user that requests more than one
single device.

Anyway, the only thing that worries me when pushing it above 256 is
that, right now, the bitmap is statically allocated, and I think a
single page would be pretty more than we have and we need right now.
Otherwise, I would suggest using a dynamic allocation system, which
would be less simple than what we have right now, and Andrew has pointed
out.

Best Regards,
Cascardo.

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

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

[Andrew Morton]

On Mon, 09 Nov 2009 16:34:07 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:

> On 11/09/2009 04:30 PM, Thadeu Lima de Souza Cascardo 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.
> > 
> > The proposed solution uses the not yet reserved range from 64 to 127. If
> > more devices are needed, we may push 64 to 16.
> > 
> 
> Again, why not push these up above 256?
> 

I merged this patch, but made a note-to-self that there are remaining
open issues..

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

[Andrew Morton]

On Wed, 11 Nov 2009 15:36:32 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 09 Nov 2009 16:34:07 -0800
> "H. Peter Anvin" <hpa@zytor.com> wrote:
> 
> > On 11/09/2009 04:30 PM, Thadeu Lima de Souza Cascardo 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.
> > > 
> > > The proposed solution uses the not yet reserved range from 64 to 127. If
> > > more devices are needed, we may push 64 to 16.
> > > 
> > 
> > Again, why not push these up above 256?
> > 
> 
> I merged this patch, but made a note-to-self that there are remaining
> open issues..

And nothing else happened.  Can we revisit this please?




From: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>

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.

The proposed solution uses the not yet reserved range from 64 to 127.  If
more devices are needed, we may push 64 to 16.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/char/misc.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff -puN drivers/char/misc.c~drivers-char-miscc-use-a-proper-range-for-minor-number-dynamic-allocation drivers/char/misc.c
--- a/drivers/char/misc.c~drivers-char-miscc-use-a-proper-range-for-minor-number-dynamic-allocation
+++ a/drivers/char/misc.c
@@ -59,6 +59,7 @@ static DEFINE_MUTEX(misc_mtx);
 /*
  * Assigned numbers, used for dynamic minors
  */
+#define DYNAMIC_MINOR_BASE 64
 #define DYNAMIC_MINORS 64 /* like dynamic majors */
 static DECLARE_BITMAP(misc_minors, DYNAMIC_MINORS);
 
@@ -201,7 +202,7 @@ int misc_register(struct miscdevice * mi
 			mutex_unlock(&misc_mtx);
 			return -EBUSY;
 		}
-		misc->minor = DYNAMIC_MINORS - i - 1;
+		misc->minor = DYNAMIC_MINOR_BASE + i;
 		set_bit(i, misc_minors);
 	}
 
@@ -210,7 +211,7 @@ int misc_register(struct miscdevice * mi
 	misc->this_device = device_create(misc_class, misc->parent, dev,
 					  misc, "%s", misc->name);
 	if (IS_ERR(misc->this_device)) {
-		int i = DYNAMIC_MINORS - misc->minor - 1;
+		int i = misc->minor - DYNAMIC_MINOR_BASE;
 		if (i < DYNAMIC_MINORS && i >= 0)
 			clear_bit(i, misc_minors);
 		err = PTR_ERR(misc->this_device);
@@ -239,7 +240,7 @@ int misc_register(struct miscdevice * mi
 
 int misc_deregister(struct miscdevice *misc)
 {
-	int i = DYNAMIC_MINORS - misc->minor - 1;
+	int i = misc->minor - DYNAMIC_MINOR_BASE;
 
 	if (list_empty(&misc->list))
 		return -EINVAL;
_

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

[H. Peter Anvin]

On 12/15/2009 02:34 PM, Andrew Morton wrote:
>>>>
>>>> The proposed solution uses the not yet reserved range from 64 to 127. If
>>>> more devices are needed, we may push 64 to 16.
>>>>
>>>
>>> Again, why not push these up above 256?
>>>
>>
>> I merged this patch, but made a note-to-self that there are remaining
>> open issues..
> 
> And nothing else happened.  Can we revisit this please?
> 

There seem to be people still worried about breaking userspace with
majors/minors >= 256.  I'm starting to think it is time to actually
break userspace, and dynamic majors/minors seem as good as any place to
start, especially since they by definition has to be managed by
something like udev.  We have had large dev_t for something like six
years now, and most pieces of software isn't affected at all -- only the
stuff that manages /dev.

	-hpa

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

[Greg KH]

On Tue, Dec 15, 2009 at 02:42:30PM -0800, H. Peter Anvin wrote:
> On 12/15/2009 02:34 PM, Andrew Morton wrote:
> >>>>
> >>>> The proposed solution uses the not yet reserved range from 64 to 127. If
> >>>> more devices are needed, we may push 64 to 16.
> >>>>
> >>>
> >>> Again, why not push these up above 256?
> >>>
> >>
> >> I merged this patch, but made a note-to-self that there are remaining
> >> open issues..
> > 
> > And nothing else happened.  Can we revisit this please?
> > 
> 
> There seem to be people still worried about breaking userspace with
> majors/minors >= 256.  I'm starting to think it is time to actually
> break userspace, and dynamic majors/minors seem as good as any place to
> start, especially since they by definition has to be managed by
> something like udev.  We have had large dev_t for something like six
> years now, and most pieces of software isn't affected at all -- only the
> stuff that manages /dev.

No objection from me, as long as the patch actually works.

But you might want to provide a config option so that loons that still
are running Fedora 3 PPC machines who keep insisting on updating to the
latest kernel version still keep their machines limping along
successfully :)

thanks,

greg k-h

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

[Alan Cox]

What I have been doing in response to requests for misc device
allocations is to suggest people simply go and allocate some dynamic
device space. There are very very few cases a misc device needs a
specific minor and the large dynamic device space has really made misc a
"historical" entity which should be treated that way.

If you want 2048 minors for a random new device, the kernel will just
magic them for you, udev will magic the device nodes, and the tmpfs will
handle it nicely

(and if your udev isn't on its own private file system then I hope its an
old one, because the current one has a hole in it, which doesn't seem to
have been fixed yet)

Alan

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

[cascardo]

On Tue, Dec 15, 2009 at 11:56:39PM +0000, Alan Cox wrote:
> What I have been doing in response to requests for misc device
> allocations is to suggest people simply go and allocate some dynamic
> device space. There are very very few cases a misc device needs a
> specific minor and the large dynamic device space has really made misc a
> "historical" entity which should be treated that way.
> 
> If you want 2048 minors for a random new device, the kernel will just
> magic them for you, udev will magic the device nodes, and the tmpfs will
> handle it nicely
> 
> (and if your udev isn't on its own private file system then I hope its an
> old one, because the current one has a hole in it, which doesn't seem to
> have been fixed yet)
> 
> Alan

I have one "issue" to raise about using anything else: misc devices are
easy to register: they have a class of its own, you don't need to
allocate a range and, then, create the device with its respective file
operations and all that. For one-shot devices, they are really a good
thing.

Does it make sense to create an easier and simpler API for creating
character devices that does not allocate a major with minor 0 and 255
minors which also creates your own class?

I think many device driver writers go for proc filesystem these days
because you just have to call a function with your file name and file
operations and that's it. Most of these people are damn lazy people, I
would agree. But, perhaps, we should make it easier for these lazy
people to write better interfaces. Or perhaps this would spoil them and
they would not do their homework and use the right interface anyway.

I've seen code that should have been using the input subsystem and was
using proc files. And that's why I'm saying they could be spoiled with
such an easy interface. They would still use the wrong interface anyway,
with character devices instead of using the input subsystem.

Well, I'll have a go for such an interface and request comments. It
would not be much different from the misc system anyway, but would allow
more than one device, any (dynamic) major to be used, and a class name
for each registerer.

Regards,
Cascardo.

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

[cascardo]

On Tue, Dec 15, 2009 at 02:42:30PM -0800, H. Peter Anvin wrote:
> On 12/15/2009 02:34 PM, Andrew Morton wrote:
> >>>>
> >>>> The proposed solution uses the not yet reserved range from 64 to 127. If
> >>>> more devices are needed, we may push 64 to 16.
> >>>>
> >>>
> >>> Again, why not push these up above 256?
> >>>
> >>
> >> I merged this patch, but made a note-to-self that there are remaining
> >> open issues..
> > 
> > And nothing else happened.  Can we revisit this please?
> > 
> 
> There seem to be people still worried about breaking userspace with
> majors/minors >= 256.  I'm starting to think it is time to actually
> break userspace, and dynamic majors/minors seem as good as any place to
> start, especially since they by definition has to be managed by
> something like udev.  We have had large dev_t for something like six
> years now, and most pieces of software isn't affected at all -- only the
> stuff that manages /dev.
> 
> 	-hpa

I see no problem in this. Can we make this configurable like the random
minor for block devices was done? Or, perhaps, before making a decision,
we can check what kind of devices are using misc dynamic minor, which
was what I did before writing this patch to count the number of current
in-tree users.

I am currently downloading source code from git since I've lost my disk
recently. That's why I'm late replying. I may send a list of current
users if that's interesting for a better analysis.

Regards,
Cascardo.

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

[Alan Cox]

> > I merged this patch, but made a note-to-self that there are remaining
> > open issues..
> 
> And nothing else happened.  Can we revisit this please?

The last I saw was a request for an explanation of the users
configuration and how he was getting so many misc devices. Its a bug in
dlm if the dlm is doing that but I didn't see any follow up to the DLM
folks on the subject from the reporter, so it stays NACKed and conflicts
with the device registry.

We need to know from the DLM folks/Thaddeu what is actually going on with
that system, especially as it seems to be producing multiple
registrations of the same misc device name which is completely broken and
should probably be made to error anyway.

Whatever is going on this is the wrong "fix". If DLM should only register
it once (as seems the intent) then DLM needs fixing or the users config
or both. If it can register many then DLM needs fixing to not eat misc
devices.

<eviltwin>Possibly we need to make it BUG_ON() adding the same name twice, then
perhaps we'd get more feedback</eviltwin>

Alan

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

[Andrew Morton]

On Tue, 15 Dec 2009 23:37:38 +0000
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > > I merged this patch, but made a note-to-self that there are remaining
> > > open issues..
> > 
> > And nothing else happened.  Can we revisit this please?
> 
> The last I saw was a request for an explanation of the users
> configuration and how he was getting so many misc devices. Its a bug in
> dlm if the dlm is doing that but I didn't see any follow up to the DLM
> folks on the subject from the reporter, so it stays NACKed and conflicts
> with the device registry.
> 
> We need to know from the DLM folks/Thaddeu what is actually going on with
> that system, especially as it seems to be producing multiple
> registrations of the same misc device name which is completely broken and
> should probably be made to error anyway.
> 
> Whatever is going on this is the wrong "fix". If DLM should only register
> it once (as seems the intent) then DLM needs fixing or the users config
> or both. If it can register many then DLM needs fixing to not eat misc
> devices.
> 
> <eviltwin>Possibly we need to make it BUG_ON() adding the same name twice, then
> perhaps we'd get more feedback</eviltwin>
> 

OK, thanks, I dropped it.  We can always reappyly it (or a variant)
once this is all sorted out.

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

[David Teigland]

On Tue, Dec 15, 2009 at 11:37:38PM +0000, Alan Cox wrote:
> > > I merged this patch, but made a note-to-self that there are remaining
> > > open issues..
> > 
> > And nothing else happened.  Can we revisit this please?
> 
> The last I saw was a request for an explanation of the users
> configuration and how he was getting so many misc devices. Its a bug in
> dlm if the dlm is doing that but I didn't see any follow up to the DLM
> folks on the subject from the reporter, so it stays NACKed and conflicts
> with the device registry.

I never saw an indication that the dlm ate up all of someone's misc
devices.  Can we ask the reporter what they were running?

> We need to know from the DLM folks/Thaddeu what is actually going on with
> that system, especially as it seems to be producing multiple
> registrations of the same misc device name which is completely broken and
> should probably be made to error anyway.

I don't believe the dlm will ever register multiple devices with the same
name.

> Whatever is going on this is the wrong "fix". If DLM should only register
> it once (as seems the intent) then DLM needs fixing or the users config
> or both. If it can register many then DLM needs fixing to not eat misc
> devices.

I explained here http://lkml.org/lkml/2009/11/9/379 that the dlm does not
use as many misc devices as has been implied.  It starts with 3, and adds
one for each *userspace* lockspace.  There are very few applications (I
know of 3) that create userspace lockspaces, and they each create about
one each.

That said, I still intend to rework the dlm to use a single device for all
lockspaces.

Dave

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

[H. Peter Anvin]

On 12/16/2009 10:01 AM, David Teigland wrote:
> 
> I explained here http://lkml.org/lkml/2009/11/9/379 that the dlm does not
> use as many misc devices as has been implied.  It starts with 3, and adds
> one for each *userspace* lockspace.  There are very few applications (I
> know of 3) that create userspace lockspaces, and they each create about
> one each.
> 
> That said, I still intend to rework the dlm to use a single device for all
> lockspaces.
> 

Still seems to make more sense to simply move it to a major and/or a
dynamic allocation.  misc devices were always meant to be *one of a
kind* devices, which simply didn't need anything but a single allocation.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

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

[cascardo]

On Wed, Dec 16, 2009 at 12:01:56PM -0600, David Teigland wrote:
> On Tue, Dec 15, 2009 at 11:37:38PM +0000, Alan Cox wrote:
> > > > I merged this patch, but made a note-to-self that there are remaining
> > > > open issues..
> > > 
> > > And nothing else happened.  Can we revisit this please?
> > 
> > The last I saw was a request for an explanation of the users
> > configuration and how he was getting so many misc devices. Its a bug in
> > dlm if the dlm is doing that but I didn't see any follow up to the DLM
> > folks on the subject from the reporter, so it stays NACKed and conflicts
> > with the device registry.
> 
> I never saw an indication that the dlm ate up all of someone's misc
> devices.  Can we ask the reporter what they were running?
> 

That was not such ocurrence. I don't use DLM. Sorry for all the fuss.
I've simply noticed that DLM was using misc devices to create devices
dynamically after an ioctl. I consider that that ioctl can be called as
many times as I want my userspace program to call, am I right?

> > We need to know from the DLM folks/Thaddeu what is actually going on with
> > that system, especially as it seems to be producing multiple
> > registrations of the same misc device name which is completely broken and
> > should probably be made to error anyway.
> 
> I don't believe the dlm will ever register multiple devices with the same
> name.
> 

No. That was another issue, not related to DLM at all. Again, sorry for
any confusion.

> > Whatever is going on this is the wrong "fix". If DLM should only register
> > it once (as seems the intent) then DLM needs fixing or the users config
> > or both. If it can register many then DLM needs fixing to not eat misc
> > devices.
> 
> I explained here http://lkml.org/lkml/2009/11/9/379 that the dlm does not
> use as many misc devices as has been implied.  It starts with 3, and adds
> one for each *userspace* lockspace.  There are very few applications (I
> know of 3) that create userspace lockspaces, and they each create about
> one each.
> 

The raised issue was that of the device for *each* lock. An application
could request as many as it wanted. If there's no such usecase, I don't
think that's a problem as long as that call is privileged (which may be
easily achieved with the simple chown/chmod).

> That said, I still intend to rework the dlm to use a single device for all
> lockspaces.

That would break current userspace implementations. Or do you have any
idea on solving this without this breakage?

> 
> Dave
> 

Regards and sorry for anything,
Cascardo.

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

[cascardo]

On Tue, Dec 15, 2009 at 11:37:38PM +0000, Alan Cox wrote:
> > > I merged this patch, but made a note-to-self that there are remaining
> > > open issues..
> > 
> > And nothing else happened.  Can we revisit this please?
> 
> The last I saw was a request for an explanation of the users
> configuration and how he was getting so many misc devices. Its a bug in
> dlm if the dlm is doing that but I didn't see any follow up to the DLM
> folks on the subject from the reporter, so it stays NACKed and conflicts
> with the device registry.
> 

I don't recall an explanation fro my DLM configuration. Probably my
mistake, sorry.

However, I haven't hit any DLM problem because I am no DLM user. My
concern was that the current code has overlapping ranges for the dynamic
range of device numbers and the first static device numbers. They do not
conflict at first because the dynamic range grows downards, and I kept
that behaviour in the other patch.

When I realized this, I decided to check what code did use misc dynamic
minors. I've accounted about 40 users and one in special caught my
attention. That was DLM, because it could potentially create many
devices.

> We need to know from the DLM folks/Thaddeu what is actually going on with
> that system, especially as it seems to be producing multiple
> registrations of the same misc device name which is completely broken and
> should probably be made to error anyway.
> 
> Whatever is going on this is the wrong "fix". If DLM should only register
> it once (as seems the intent) then DLM needs fixing or the users config
> or both. If it can register many then DLM needs fixing to not eat misc
> devices.
> 

It will only create that many devices if we request it to do so. But
requesting it is simply a matter of doing an ioctl. I think going for a
dynamic major of its own would be a better solution for DLM in the long
term.

> <eviltwin>Possibly we need to make it BUG_ON() adding the same name twice, then
> perhaps we'd get more feedback</eviltwin>

That was an unrelated problem, that made me look at the misc code at
first. I was playing with registering and unregistering misc devices.
And, then, I've hit the bug: after registering two devices with the same
name and unregistering both of them afterwards, a minor number got
unavailable for any further registering. That was fixed in the first
patch in the series.

I think a BUG_ON at driver core is not that bad an idea at all. Let me
try it. This bug would be hit even with the misc fix applied, since misc
would still try to register the device, and, then (now with the patch)
release the allocated minor number, returning error to the registerer.

> 
> Alan

Regards,
Cascardo.