[PATCH 1/2] chrdev: emit a warning when we go below dynamic major range

[PATCH 1/2] chrdev: emit a warning when we go below dynamic major range

[Linus Walleij]

Currently a dynamically allocated character device major is taken
from 254 and downward. This mechanism is used for RTC, IIO and a
few other subsystems.

The kernel currently has no check prevening these dynamic
allocations from eating into the assigned numbers at 233 and
downward.

In a recent test it was reported that so many dynamic device
majors were used on a test server, that the major number for
infiniband (231) was stolen. This occurred when allocating a new
major number for GPIO chips. The error messages from the kernel
were not helpful. (See: https://lkml.org/lkml/2016/2/14/124)

This patch adds a defined lower limit of the dynamic major
allocation region will henceforth emit a warning if we start to
eat into the assigned numbers. It does not do any semantic
changes and will not change the kernels behaviour: numbers will
still continue to be stolen, but we will know from dmesg what
is going on.

This also updates the Documentation/devices.txt to clearly
reflect that we are using this range of major numbers for dynamic
allocation.

Reported-by: Ying Huang <ying.huang@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Alan: is your list at LANANA even maintained? It seems wildly
out of sync with what's in the kernel. Maybe time to patch out
some of the text in devices.txt directing people over there?
---
 Documentation/devices.txt | 6 +++---
 fs/char_dev.c             | 4 ++++
 include/linux/fs.h        | 2 ++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/Documentation/devices.txt b/Documentation/devices.txt
index 87b4c5e82d39..0a3588a9798d 100644
--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -3099,9 +3099,9 @@ Your cooperation is appreciated.
 		129 = /dev/ipath_sma    Device used by Subnet Management Agent
 		130 = /dev/ipath_diag   Device used by diagnostics programs
 
-234-239		UNASSIGNED
-
-240-254 char	LOCAL/EXPERIMENTAL USE
+234-254	char	RESERVED FOR DYNAMIC ASSIGNMENT
+		Character devices that request a dynamic allocation of major number will
+		take numbers starting from 254 and downward.
 
 240-254 block	LOCAL/EXPERIMENTAL USE
 		Allocated for local/experimental use.  For devices not
diff --git a/fs/char_dev.c b/fs/char_dev.c
index 24b142569ca9..687471dc04a0 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -91,6 +91,10 @@ __register_chrdev_region(unsigned int major, unsigned int baseminor,
 				break;
 		}
 
+		if (i < CHRDEV_MAJOR_DYN_END)
+			pr_warn("CHRDEV \"%s\" major number %d goes below the dynamic allocation range",
+				name, i);
+
 		if (i == 0) {
 			ret = -EBUSY;
 			goto out;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1a2046275cdf..6301ac091e54 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2384,6 +2384,8 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev,
 
 /* fs/char_dev.c */
 #define CHRDEV_MAJOR_HASH_SIZE	255
+/* Marks the bottom of the first segment of free char majors */
+#define CHRDEV_MAJOR_DYN_END 234
 extern int alloc_chrdev_region(dev_t *, unsigned, unsigned, const char *);
 extern int register_chrdev_region(dev_t, unsigned, const char *);
 extern int __register_chrdev(unsigned int major, unsigned int baseminor,
-- 
2.4.3

[PATCH 2/2] RFC: chrdev: allocate chardevs in all unused holes

[Linus Walleij]

This is a duct-tape-and-chewing-gum solution to the problem with
the major numbers running out when allocating major numbers
dynamically.

To avoid collissions in the major space, we supply a list of
"holes" that exist in the lower range of major numbers [0-254]
and pick numbers from there once the first dynamic range in
234-254 is used up.

This starts to produce INFO prints if we run down and start
allocating major numbers in the "holes" below major 234 so
we know what is going on.

It will also FAIL if we actually fill up all free major
numbers. This seems to me like the reasonable thing to do
since these numbers are, after all, reserved.

This also deletes the comment /* temporary */ which must be
one of the biggest lies ever.

This also updates the Documentation/devices.txt document to
reflect that all these numbers are used for dynamic assignment.

Reported-by: Ying Huang <ying.huang@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
What do I know of the "real" solution to this problem, I like
Gregs solution to have *only* dynamic allocation of major numbers
(give take some like tty's I guess) but cannot really test such
an approach properly. Take this as a discussion starting point.

Maybe Intel's test server with 20+ dynamic major devices is
something extremely unlikely-in-actual-reality virtualized or
whatnot, Ying it'd be nice if you could shed some light on this.
See: https://lkml.org/lkml/2016/2/14/124
---
 Documentation/devices.txt | 22 ++++++++++++++--------
 fs/char_dev.c             | 39 ++++++++++++++++++++++++++++++++-------
 2 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/Documentation/devices.txt b/Documentation/devices.txt
index 0a3588a9798d..f1c441e085e9 100644
--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -278,6 +278,8 @@ Your cooperation is appreciated.
 		associated with block devices.	The binding to the
 		loop devices is handled by mount(8) or losetup(8).
 
+  8 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
+
   8 block	SCSI disk devices (0-15)
 		  0 = /dev/sda		First SCSI disk whole disk
 		 16 = /dev/sdb		Second SCSI disk whole disk
@@ -650,7 +652,7 @@ Your cooperation is appreciated.
 		  2 = /dev/sbpcd2	Panasonic CD-ROM controller 0 unit 2
 		  3 = /dev/sbpcd3	Panasonic CD-ROM controller 0 unit 3
 
- 26 char
+ 26 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
 
  26 block	Second Matsushita (Panasonic/SoundBlaster) CD-ROM
 		  0 = /dev/sbpcd4	Panasonic CD-ROM controller 1 unit 0
@@ -893,7 +895,7 @@ Your cooperation is appreciated.
 		      ...
  39 block
 
- 40 char
+ 40 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
 
  40 block
 
@@ -1157,7 +1159,7 @@ Your cooperation is appreciated.
 
 		NAMING CONFLICT -- PROPOSED REVISED NAME /dev/rpda0 etc
 
- 60-63 char	LOCAL/EXPERIMENTAL USE
+ 60-63 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
 
  60-63 block	LOCAL/EXPERIMENTAL USE
 		Allocated for local/experimental use.  For devices not
@@ -1671,7 +1673,7 @@ Your cooperation is appreciated.
 		disks (see major number 3) except that the limit on
 		partitions is 15.
 
- 93 char
+ 93 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
 
  93 block	NAND Flash Translation Layer filesystem
 		  0 = /dev/nftla	First NFTL layer
@@ -1679,7 +1681,7 @@ Your cooperation is appreciated.
 		    ...
 		240 = /dev/nftlp	16th NTFL layer
 
- 94 char
+ 94 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
 
  94 block	IBM S/390 DASD block storage
     		  0 = /dev/dasda First DASD device, major
@@ -1772,7 +1774,7 @@ Your cooperation is appreciated.
 		    ...
 		 15 = /dev/amiraid/ar?p15 15th partition
 
-102 char
+102 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
 
 102 block	Compressed block device
 		  0 = /dev/cbd/a	First compressed block device, whole device
@@ -2057,7 +2059,7 @@ Your cooperation is appreciated.
 		  1 = /dev/vnet1	2nd virtual network
 		    ...
 
-120-127 char	LOCAL/EXPERIMENTAL USE
+120-127 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
 
 120-127 block	LOCAL/EXPERIMENTAL USE
 		Allocated for local/experimental use.  For devices not
@@ -2371,7 +2373,7 @@ Your cooperation is appreciated.
 		  1 = /dev/gfax1	GammaLink channel 1
 		    ...
 
-159 char	RESERVED
+159 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
 
 159 block	RESERVED
 
@@ -2959,6 +2961,8 @@ Your cooperation is appreciated.
 		    ...
 		196 = /dev/dvb/adapter3/video0    first video decoder of fourth card
 
+213-215 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
+
 216 char	Bluetooth RFCOMM TTY devices
 		  0 = /dev/rfcomm0		First Bluetooth RFCOMM TTY device
 		  1 = /dev/rfcomm1		Second Bluetooth RFCOMM TTY device
@@ -3001,6 +3005,8 @@ Your cooperation is appreciated.
 		same interface.  For interface documentation see
 		http://www.vmelinux.org/.
 
+222-223 char	RESERVED FOR DYNAMIC ALLOCATION OF MAJOR NUMBERS
+
 224 char	A2232 serial card
 		  0 = /dev/ttyY0		First A2232 port
 		  1 = /dev/ttyY1		Second A2232 port
diff --git a/fs/char_dev.c b/fs/char_dev.c
index 687471dc04a0..001b037f6b98 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -37,6 +37,15 @@ static struct char_device_struct {
 	struct cdev *cdev;		/* will die */
 } *chrdevs[CHRDEV_MAJOR_HASH_SIZE];
 
+/*
+ * Duct tape: free major numbers that can be used below the
+ * primary dynamic character major assignment range 234-254
+ */
+static int free_majors[] = {223, 222, 215, 214, 213, 159, 127,
+			   126, 125, 124, 123, 122, 121, 120,
+			   102, 94, 93, 63, 62, 61, 60, 40,
+			   26, 8 };
+
 /* index in the above */
 static inline int major_to_index(unsigned major)
 {
@@ -84,21 +93,37 @@ __register_chrdev_region(unsigned int major, unsigned int baseminor,
 
 	mutex_lock(&chrdevs_lock);
 
-	/* temporary */
 	if (major == 0) {
 		for (i = ARRAY_SIZE(chrdevs)-1; i > 0; i--) {
+			if (i < CHRDEV_MAJOR_DYN_END)
+				break;
 			if (chrdevs[i] == NULL)
 				break;
 		}
 
-		if (i < CHRDEV_MAJOR_DYN_END)
-			pr_warn("CHRDEV \"%s\" major number %d goes below the dynamic allocation range",
+		/*
+		 * Out of the first dynamic range, OK try this duct-tape
+		 * array of lower majors.
+		 */
+		if (i < CHRDEV_MAJOR_DYN_END) {
+			int j;
+
+			for (j = 0; j < ARRAY_SIZE(free_majors); j++) {
+				if (chrdevs[free_majors[j]] == NULL) {
+					i = free_majors[j];
+					break;
+				}
+			}
+			if (j == ARRAY_SIZE(free_majors)) {
+				pr_warn("CHRDEV \"%s\" out of major numbers",
+					name);
+				ret = -EBUSY;
+				goto out;
+			}
+			pr_info("CHRDEV: \"%s\" using extended dynamic range for major number %d\n",
 				name, i);
-
-		if (i == 0) {
-			ret = -EBUSY;
-			goto out;
 		}
+
 		major = i;
 	}
 
-- 
2.4.3

Re: [PATCH 1/2] chrdev: emit a warning when we go below dynamic major range

[Alan Cox]

> Alan: is your list at LANANA even maintained? It seems wildly
> out of sync with what's in the kernel. Maybe time to patch out
> some of the text in devices.txt directing people over there?

Someone took my access to update the database away, removed my access
to the email and then ignored repeated queries about it. 

So it's broken and I simply don't care.

Alan

Re: [PATCH 1/2] chrdev: emit a warning when we go below dynamic major range

[Linus Walleij]

On Fri, Feb 19, 2016 at 5:06 PM, Alan Cox <alan@linux.intel.com> wrote:
>> Alan: is your list at LANANA even maintained? It seems wildly
>> out of sync with what's in the kernel. Maybe time to patch out
>> some of the text in devices.txt directing people over there?
>
> Someone took my access to update the database away, removed my access
> to the email and then ignored repeated queries about it.
>
> So it's broken and I simply don't care.

Aha OK, let's treat that as a technical problem and delete the
references to it, and then henceforth regard the file in the kernel
as the master.

Thanks!
Linus Walleij

Re: [PATCH 2/2] RFC: chrdev: allocate chardevs in all unused holes

[Linus Torvalds]

On Fri, Feb 19, 2016 at 6:36 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This is a duct-tape-and-chewing-gum solution to the problem with
> the major numbers running out when allocating major numbers
> dynamically.

Ugh. This is too ugly to live.

Can't you do something with a simple bitmap, and just pre-allocate the
fixed numbers? Allocate new major numbers using find_next_zero_bit()
or something..

I can't imagine that we couldn't do something prettier in less code
than that too-disgusting-to-live free_majors[] array..

               Linus