[PATCH] kdevt-diff

[PATCH] kdevt-diff

[Andries.Brouwer]

Below a kdev_t.h diff. I suppose it can be applied.
(For myself I prefer a complete check of all code involved
but I can see that people are somewhat impatient.)
This is the part that changes MAJOR/MINOR/MKDEV,
that is, the structure of dev_t.

The structure here is 8+8, except when more bits are
present, in which case it is 16+16, except when more bits
are present, in which case it is 32+32.
Since dev_t is 64-bit the structure of the middle part is not
very important, but in some contexts we naturally get 16+16
(e.g. from CDROM) and 16+16 avoids messy conversion.

The macros here are written with the casts and typechecking
that is otherwise implicit in the use of inline functions.
Because of name clashes they cannot be inline functions.
The MKDEV as given is not accepted by gcc in an enum,
that is why I changed root_dev.h.

Since MINORBITS disappears I gave md_k.h and dasd_int.h
local definitions.

Andries


diff -u --recursive --new-file -X /linux/dontdiff a/include/linux/kdev_t.h b/include/linux/kdev_t.h
--- a/include/linux/kdev_t.h	Tue Apr  8 09:36:46 2003
+++ b/include/linux/kdev_t.h	Sat Apr 12 19:46:18 2003
@@ -1,74 +1,9 @@
 #ifndef _LINUX_KDEV_T_H
 #define _LINUX_KDEV_T_H
 #ifdef __KERNEL__
-/*
-As a preparation for the introduction of larger device numbers,
-we introduce a type kdev_t to hold them. No information about
-this type is known outside of this include file.
-
-Objects of type kdev_t designate a device. Outside of the kernel
-the corresponding things are objects of type dev_t - usually an
-integral type with the device major and minor in the high and low
-bits, respectively. Conversion is done by
-
-extern kdev_t to_kdev_t(int);
-
-It is up to the various file systems to decide how objects of type
-dev_t are stored on disk.
-The only other point of contact between kernel and outside world
-are the system calls stat and mknod, new versions of which will
-eventually have to be used in libc.
-
-[Unfortunately, the floppy control ioctls fail to hide the internal
-kernel structures, and the fd_device field of a struct floppy_drive_struct
-is user-visible. So, it remains a dev_t for the moment, with some ugly
-conversions in floppy.c.]
-
-Inside the kernel, we aim for a kdev_t type that is a pointer
-to a structure with information about the device (like major,
-minor, size, blocksize, sectorsize, name, read-only flag,
-struct file_operations etc.).
-
-However, for the time being we let kdev_t be almost the same as dev_t:
-
-typedef struct { unsigned short major, minor; } kdev_t;
-
-Admissible operations on an object of type kdev_t:
-- passing it along
-- comparing it for equality with another such object
-- storing it in inode->i_rdev or tty->device
-- using its bit pattern as argument in a hash function
-- finding its major and minor
-- complaining about it
-
-An object of type kdev_t is created only by the function MKDEV(),
-with the single exception of the constant 0 (no device).
-
-Right now the other information mentioned above is usually found
-in static arrays indexed by major or major,minor.
-
-An obstacle to immediately using
-    typedef struct { ... (* lots of information *) } *kdev_t
-is the case of mknod used to create a block device that the
-kernel doesn't know about at present (but first learns about
-when some module is inserted).
-
-aeb - 950811
-*/
 
+#include <linux/types.h>	/* for dev_t */
 
-/*
- * NOTE NOTE NOTE!
- *
- * The kernel-internal "kdev_t" will eventually have
- * 20 bits for minor numbers, and 12 bits for majors.
- *
- * HOWEVER, the external representation is still 8+8
- * bits, and there is no way to generate the extended
- * "kdev_t" format yet. Which is just as well, since
- * we still use "minor" as an index into various
- * static arrays, and they are sized for a 8-bit index.
- */
 typedef struct {
 	unsigned short value;
 } kdev_t;
@@ -113,12 +48,47 @@
 #define major(dev)	(((dev).value >> KDEV_MINOR_BITS) & 0xff)
 
 /* These are for user-level "dev_t" */
-#define MINORBITS	8
-#define MINORMASK	((1U << MINORBITS) - 1)
+/* Going back and forth between dev and (ma,mi) is one-to-one
+   provided ma is nonzero or ma is zero and mi is 8-bit only.
+   Never use major 0 together with a minor larger than 255. */
+#if 0
+/* readable versions */
+static inline unsigned int
+MAJOR(dev_t dev) {
+	return (dev & ~0xffffffff) ? (dev >> 32) :
+		(dev & ~0xffff) ? (dev >> 16) : (dev >> 8);
+}
+
+static inline unsigned int
+MINOR(dev_t dev) {
+	return (dev & ~0xffffffff) ? (dev & 0xffffffff) :
+		(dev & ~0xffff) ? (dev & 0xffff) : (dev & 0xff);
+}
 
-#define MAJOR(dev)	((unsigned int) ((dev) >> MINORBITS))
-#define MINOR(dev)	((unsigned int) ((dev) & MINORMASK))
-#define MKDEV(ma,mi)	(((ma) << MINORBITS) | (mi))
+static inline dev_t
+MKDEV(unsigned int major, unsigned int minor) {
+	unsigned int both = (major | minor);
+	return ((both & ~0xffff) ? (((dev_t) major) << 32) :
+		(both & ~0xff) ? (((dev_t) major) << 16) :
+		(((dev_t) major) << 8) ) | minor;
+}
+#else
+/* ugly macro versions */
+#define MAJOR(dev) ((unsigned int)({ dev_t __dev = dev; \
+   (__dev & ~0xffffffff) ? (__dev >> 32) : \
+   (__dev & ~0xffff) ? (__dev >> 16) : (__dev >> 8); }))
+#define MINOR(dev) ((unsigned int)({ dev_t __dev = dev; \
+   (__dev & ~0xffffffff) ? (__dev & 0xffffffff) : \
+   (__dev & ~0xffff) ? (__dev & 0xffff) : (__dev & 0xff); }))
+#define constant_MKDEV(ma, mi) \
+   ((((ma)|(mi)) & ~0xffff) ? ((ma) << 32) | (mi) : \
+    (((ma)|(mi)) & ~0xff) ? ((ma) << 16) | (mi) : ((ma) << 8) | (mi))
+#define MKDEV(major, minor) ({ \
+   unsigned int __ma = major, __mi = minor, __both = (__ma | __mi); \
+   ((sizeof(dev_t) > 4 && (__both & ~0xffff)) ? (((dev_t) __ma) << 32) : \
+    (__both & ~0xff) ? (((dev_t) __ma) << 16) : (((dev_t) __ma) << 8) \
+   ) | __mi; })
+#endif
 
 /*
  * Conversion functions
diff -u --recursive --new-file -X /linux/dontdiff a/include/linux/root_dev.h b/include/linux/root_dev.h
--- a/include/linux/root_dev.h	Fri Nov 22 22:40:24 2002
+++ b/include/linux/root_dev.h	Sat Apr 12 19:49:53 2003
@@ -1,18 +1,16 @@
 #ifndef _ROOT_DEV_H_
 #define _ROOT_DEV_H_
 
-enum {
-	Root_NFS = MKDEV(UNNAMED_MAJOR, 255),
-	Root_RAM0 = MKDEV(RAMDISK_MAJOR, 0),
-	Root_RAM1 = MKDEV(RAMDISK_MAJOR, 1),
-	Root_FD0 = MKDEV(FLOPPY_MAJOR, 0),
-	Root_HDA1 = MKDEV(IDE0_MAJOR, 1),
-	Root_HDA2 = MKDEV(IDE0_MAJOR, 2),
-	Root_SDA1 = MKDEV(SCSI_DISK0_MAJOR, 1),
-	Root_SDA2 = MKDEV(SCSI_DISK0_MAJOR, 2),
-	Root_HDC1 = MKDEV(IDE1_MAJOR, 1),
-	Root_SR0 = MKDEV(SCSI_CDROM_MAJOR, 0),
-};
+#define	Root_NFS	MKDEV(UNNAMED_MAJOR, 255)
+#define	Root_RAM0	MKDEV(RAMDISK_MAJOR, 0)
+#define	Root_RAM1	MKDEV(RAMDISK_MAJOR, 1)
+#define	Root_FD0	MKDEV(FLOPPY_MAJOR, 0)
+#define	Root_HDA1	MKDEV(IDE0_MAJOR, 1)
+#define	Root_HDA2	MKDEV(IDE0_MAJOR, 2)
+#define	Root_SDA1	MKDEV(SCSI_DISK0_MAJOR, 1)
+#define	Root_SDA2	MKDEV(SCSI_DISK0_MAJOR, 2)
+#define	Root_HDC1	MKDEV(IDE1_MAJOR, 1)
+#define	Root_SR0	MKDEV(SCSI_CDROM_MAJOR, 0)
 
 extern dev_t ROOT_DEV;
 
diff -u --recursive --new-file -X /linux/dontdiff a/include/linux/raid/md_k.h b/include/linux/raid/md_k.h
--- a/include/linux/raid/md_k.h	Tue Apr  8 09:36:47 2003
+++ b/include/linux/raid/md_k.h	Sat Apr 12 13:42:52 2003
@@ -64,11 +64,7 @@
 typedef struct mddev_s mddev_t;
 typedef struct mdk_rdev_s mdk_rdev_t;
 
-#if (MINORBITS != 8)
-#error MD does not handle bigger kdev yet
-#endif
-
-#define MAX_MD_DEVS  (1<<MINORBITS)	/* Max number of md dev */
+#define MAX_MD_DEVS  256	/* Max number of md dev */
 
 /*
  * options passed in raidrun:
diff -u --recursive --new-file -X /linux/dontdiff a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h
--- a/drivers/s390/block/dasd_int.h	Tue Dec 10 18:42:33 2002
+++ b/drivers/s390/block/dasd_int.h	Sat Apr 12 13:44:41 2003
@@ -19,7 +19,8 @@
 
 #ifdef __KERNEL__
 
-#define DASD_PER_MAJOR ( 1U<<(MINORBITS-DASD_PARTN_BITS))
+#define DASD_MINORBITS 8
+#define DASD_PER_MAJOR ( 1U<<(DASD_MINORBITS-DASD_PARTN_BITS))
 #define DASD_PARTN_MASK ((1 << DASD_PARTN_BITS) - 1)
 
 /*

Re: [PATCH] kdevt-diff

[Joel Becker]

On Mon, Apr 14, 2003 at 12:45:36AM +0200, Andries.Brouwer@cwi.nl wrote:
> The structure here is 8+8, except when more bits are
> present, in which case it is 16+16, except when more bits
> are present, in which case it is 32+32.

	Why complicate things?  Is it that bad?  We'd all have to know
about the mess when dealing with userspace.

Joel

-- 

Life's Little Instruction Book #237

	"Seek out the good in people."

Joel Becker
Senior Member of Technical Staff
Oracle Corporation
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH] kdevt-diff

[Linus Torvalds]

On Mon, 14 Apr 2003, Joel Becker wrote:
>
> On Mon, Apr 14, 2003 at 12:45:36AM +0200, Andries.Brouwer@cwi.nl wrote:
> > The structure here is 8+8, except when more bits are
> > present, in which case it is 16+16, except when more bits
> > are present, in which case it is 32+32.
> 
> 	Why complicate things?  Is it that bad?  We'd all have to know
> about the mess when dealing with userspace.

Well, the thing is, we absolutely _do_ need to have the 8+8 split, in
order to make old devices look the same old way for old binaries.

And the 32+32 split is what the new maximum would be, so ..

The 16+16 split is not strictly necessary, but Andries pointed out to me
that there are filesystems etc external storage that only support a 32-bit
opaque dev_t, so we'd need to marshall the device number _some_ way for
them anyway, and having a standard way to do that is better than having
everybody come up with their own variations.

(My prefernce for the 32-bit version would be 12+20 bits, but it's not a
very strong one, and it doesn't really matter for the kernel proper, so I
think Andries who has been tirelessly working on this for five years or
more gets the final say on it).

		Linus

Re: [PATCH] kdevt-diff

[Joel Becker]

On Mon, Apr 14, 2003 at 11:00:27AM -0700, Linus Torvalds wrote:
> Well, the thing is, we absolutely _do_ need to have the 8+8 split, in
> order to make old devices look the same old way for old binaries.

	Yes, and I support this 100%.

> The 16+16 split is not strictly necessary, but Andries pointed out to me
> that there are filesystems etc external storage that only support a 32-bit
> opaque dev_t, so we'd need to marshall the device number _some_ way for
> them anyway, and having a standard way to do that is better than having
> everybody come up with their own variations.

	Sure, but it's a marshall, not a reality.  One of the reasons
for choosing 64bits is that we can have large spaces for large things.
If a driver happens to get a number in the 16:16 space (or the 12:20
space, which I prefer as well), then it could run out of space and end
up with the multiple major problem.
	True, a truly dynamic scheme could make all of this irrelevant,
but I was just postulating that complexity isn't strictly necessary.
	I guess it is a trade off.  Do all devices greater than 8:8
become 32:32 and merely are masked to 16:16 on limited filesystems, or
do all devices smaller than 16:16/12:20 appear the same on all
filesystems, limited or not?

JOel

-- 

"You can get more with a kind word and a gun than you can with
 a kind word alone."
         - Al Capone

Joel Becker
Senior Member of Technical Staff
Oracle Corporation
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH] kdevt-diff

[Roman Zippel]

Hi,

On Mon, 14 Apr 2003, Linus Torvalds wrote:

> And the 32+32 split is what the new maximum would be, so ..
> 
> The 16+16 split is not strictly necessary, but Andries pointed out to me
> that there are filesystems etc external storage that only support a 32-bit
> opaque dev_t, so we'd need to marshall the device number _some_ way for
> them anyway, and having a standard way to do that is better than having
> everybody come up with their own variations.
> 
> (My prefernce for the 32-bit version would be 12+20 bits, but it's not a
> very strong one, and it doesn't really matter for the kernel proper, so I
> think Andries who has been tirelessly working on this for five years or
> more gets the final say on it).

If you want to argue this way, I urge to ask Al as well, as he did most of 
the hard work to remove the block device limits. For some reason everyone 
likes to disagree with me, but I would accept his judgement.

bye, Roman

Re: [PATCH] kdevt-diff

[H. Peter Anvin]

Followup to:  <Pine.LNX.4.44.0304141056450.19302-100000@home.transmeta.com>
By author:    Linus Torvalds <torvalds@transmeta.com>
In newsgroup: linux.dev.kernel
> 
> (My prefernce for the 32-bit version would be 12+20 bits, but it's not a
> very strong one, and it doesn't really matter for the kernel proper, so I
> think Andries who has been tirelessly working on this for five years or
> more gets the final say on it).
> 

I think 12+20 is a good choice for the 32-bit version (NFSv2 and those
guys.)

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64

Re: [PATCH] kdevt-diff

[Roman Zippel]

Hi,

On Mon, 14 Apr 2003 Andries.Brouwer@cwi.nl wrote:

> Below a kdev_t.h diff. I suppose it can be applied.
> (For myself I prefer a complete check of all code involved
> but I can see that people are somewhat impatient.)
> This is the part that changes MAJOR/MINOR/MKDEV,
> that is, the structure of dev_t.

Linus, if you still want to go for a single block device major, this patch 
is bad idea (at least in this form).
The patch below demonstrates how we can use the space above 0x10000 as one 
big major. Drivers only have to set disk->major to 0 and it gets a device 
number. 
Simply expanding the dev_t number does not solve the problems, e.g. 
changing the number of partitions is still a problem. Below I added a 
GENHD_FL_DYNAMIC flag so the upper layer knows, that some values are only 
a hint and that it can change them (e.g. when the user requests it).

bye, Roman

diff -ur linux-2.5.67-cdev4/drivers/block/genhd.c linux-2.5.67-cdev5/drivers/block/genhd.c
--- linux-2.5.67-cdev4/drivers/block/genhd.c	2003-04-04 19:29:33.000000000 +0200
+++ linux-2.5.67-cdev5/drivers/block/genhd.c	2003-04-13 22:56:26.000000000 +0200
@@ -244,6 +244,12 @@
 	return 0;
 }
 
+#define DISK_NR		16384
+
+static unsigned long disk_index_bits[DISK_NR / BITS_PER_LONG];
+static spinlock_t disk_index_lock = SPIN_LOCK_UNLOCKED;
+int force_dyndisks = 1;
+
 /**
  * add_gendisk - add partitioning information to kernel list
  * @disk: per-device partitioning information
@@ -251,13 +257,40 @@
  * This function registers the partitioning information in @disk
  * with the kernel.
  */
-void add_disk(struct gendisk *disk)
+int add_disk(struct gendisk *disk)
 {
+	if (!disk->major || (force_dyndisks && disk->flags & GENHD_FL_DYNAMIC)) {
+		char name[8], *p = name + 7;
+		int index;
+
+		spin_lock(&disk_index_lock);
+		index = find_first_zero_bit(disk_index_bits, DISK_NR);
+		if (index == DISK_NR) {
+			spin_unlock(&disk_index_lock);
+			return -EBUSY;
+		}
+		__set_bit(index, disk_index_bits);
+		spin_unlock(&disk_index_lock);
+
+		disk->major = 256 + index;
+		disk->first_minor = 0;
+
+		*p-- = 0;
+		while (index > 25) {
+			*p-- = 'a' + index % 26;
+			index = index / 26 - 1;
+		}
+		*p = 'a' + index;
+		sprintf(disk->disk_name, "disk%s", p);
+	}
+
 	disk->flags |= GENHD_FL_UP;
 	blk_register_region(MKDEV(disk->major, disk->first_minor),
 			    disk->minors, NULL, exact_match, exact_lock, disk);
 	register_disk(disk);
 	elv_register_queue(disk);
+
+	return 0;
 }
 
 EXPORT_SYMBOL(add_disk);
@@ -265,6 +298,11 @@
 
 void unlink_gendisk(struct gendisk *disk)
 {
+	if (disk->major >= 256 && disk->major < 256 + DISK_NR) {
+		spin_lock(&disk_index_lock);
+		clear_bit(disk->major - 256, disk_index_bits);
+		spin_unlock(&disk_index_lock);
+	}
 	elv_unregister_queue(disk);
 	blk_unregister_region(MKDEV(disk->major, disk->first_minor),
 			      disk->minors);
diff -ur linux-2.5.67-cdev4/drivers/scsi/sd.c linux-2.5.67-cdev5/drivers/scsi/sd.c
--- linux-2.5.67-cdev4/drivers/scsi/sd.c	2003-03-19 02:16:32.000000000 +0100
+++ linux-2.5.67-cdev5/drivers/scsi/sd.c	2003-04-13 22:56:26.000000000 +0200
@@ -1314,12 +1314,8 @@
 
 	spin_lock(&sd_index_lock);
 	index = find_first_zero_bit(sd_index_bits, SD_DISKS);
-	if (index == SD_DISKS) {
-		spin_unlock(&sd_index_lock);
-		error = -EBUSY;
-		goto out_put;
-	}
-	__set_bit(index, sd_index_bits);
+	if (index != SD_DISKS)
+		__set_bit(index, sd_index_bits);
 	spin_unlock(&sd_index_lock);
 
 	sdkp->device = sdp;
@@ -1328,22 +1324,24 @@
 	sdkp->index = index;
 
 	gd->de = sdp->de;
-	gd->major = sd_major(index >> 4);
-	gd->first_minor = (index & 15) << 4;
 	gd->minors = 16;
 	gd->fops = &sd_fops;
-
-	if (index >= 26) {
-		sprintf(gd->disk_name, "sd%c%c",
-			'a' + index/26-1,'a' + index % 26);
-	} else {
-		sprintf(gd->disk_name, "sd%c", 'a' + index % 26);
+	if (index != SD_DISKS) {
+		gd->major = sd_major(index >> 4);
+		gd->first_minor = (index & 15) << 4;
+
+		if (index >= 26) {
+			sprintf(gd->disk_name, "sd%c%c",
+				'a' + index/26-1,'a' + index % 26);
+		} else {
+			sprintf(gd->disk_name, "sd%c", 'a' + index % 26);
+		}
 	}
 
 	sd_init_onedisk(sdkp, gd);
 
 	gd->driverfs_dev = &sdp->sdev_driverfs_dev;
-	gd->flags = GENHD_FL_DRIVERFS | GENHD_FL_DEVFS;
+	gd->flags = GENHD_FL_DRIVERFS | GENHD_FL_DEVFS | GENHD_FL_DYNAMIC;
 	if (sdp->removable)
 		gd->flags |= GENHD_FL_REMOVABLE;
 	gd->private_data = &sdkp->driver;
@@ -1351,7 +1349,9 @@
 
 	sd_devlist_insert(sdkp);
 	set_capacity(gd, sdkp->capacity);
-	add_disk(gd);
+	error = add_disk(gd);
+	if (error)
+		goto out_put;
 
 	printk(KERN_NOTICE "Attached scsi %sdisk %s at scsi%d, channel %d, "
 	       "id %d, lun %d\n", sdp->removable ? "removable " : "",
diff -ur linux-2.5.67-cdev4/include/linux/genhd.h linux-2.5.67-cdev5/include/linux/genhd.h
--- linux-2.5.67-cdev4/include/linux/genhd.h	2003-03-19 02:17:32.000000000 +0100
+++ linux-2.5.67-cdev5/include/linux/genhd.h	2003-04-13 22:56:26.000000000 +0200
@@ -71,6 +71,7 @@
 #define GENHD_FL_DEVFS	4
 #define GENHD_FL_CD	8
 #define GENHD_FL_UP	16
+#define GENHD_FL_DYNAMIC 32
 
 struct disk_stats {
 	unsigned read_sectors, write_sectors;
@@ -190,7 +191,7 @@
 extern void disk_round_stats(struct gendisk *disk);
 
 /* drivers/block/genhd.c */
-extern void add_disk(struct gendisk *disk);
+extern int add_disk(struct gendisk *disk);
 extern void del_gendisk(struct gendisk *gp);
 extern void unlink_gendisk(struct gendisk *gp);
 extern struct gendisk *get_gendisk(dev_t dev, int *part);

Re: [PATCH] kdevt-diff

[Linus Torvalds]

On Mon, 14 Apr 2003, Roman Zippel wrote:
> 
> Linus, if you still want to go for a single block device major, this patch 
> is bad idea (at least in this form).

I disagree.

I think the single block-device major is a totally separate issue, and has 
nothing to do with allowing big device_t representations. I do not see why 
Andries patch would be anything else than infrastructure for future 
expansion.

		Linus

Re: [PATCH] kdevt-diff

[Roman Zippel]

Hi,

On Mon, 14 Apr 2003, Linus Torvalds wrote:

> > Linus, if you still want to go for a single block device major, this patch 
> > is bad idea (at least in this form).
> 
> I disagree.
> 
> I think the single block-device major is a totally separate issue, and has 
> nothing to do with allowing big device_t representations. I do not see why 
> Andries patch would be anything else than infrastructure for future 
> expansion.

Expansion into what?
The knowledge about dev_t is already reduced to a minimum in a lot of 
block device drivers. register_blkdev() is already pretty much a dummy and 
not a requirement anymore.

bye, Roman

Re: [PATCH] kdevt-diff

[Linus Torvalds]

On Mon, 14 Apr 2003, Roman Zippel wrote:
> > I think the single block-device major is a totally separate issue, and has 
> > nothing to do with allowing big device_t representations. I do not see why 
> > Andries patch would be anything else than infrastructure for future 
> > expansion.
> 
> Expansion into what?

That's the point - Andries patch is a pure extension of the number space 
into user space. 

If you think that clashes with anything else, than that "anything else" is 
_broken_. 

> The knowledge about dev_t is already reduced to a minimum in a lot of 
> block device drivers. register_blkdev() is already pretty much a dummy and 
> not a requirement anymore.

So why do you think Andries patch clashes?

Also, I don't think your patch is proper. The point about having a single
disk number space means that IDE and SCSI disks would show up there too -
users simply shouldn't need to care about the differences (which is not
just major numbers, but also the silly difference in how the partitioning
splits minor numbers).

But at the same time, for backwards compatibility clearly they have to
show up in the _old_ places too. Which really implies that there needs to
be a mapping function for the old numbers into the proper block device
queues etc, so that people can still use the old /dev nodes when they
upgrade their kernel.

		Linus

Re: [PATCH] kdevt-diff

[Roman Zippel]

Hi,

On Mon, 14 Apr 2003, Linus Torvalds wrote:

> > Expansion into what?
> 
> That's the point - Andries patch is a pure extension of the number space 
> into user space. 
> 
> If you think that clashes with anything else, than that "anything else" is 
> _broken_. 

It doesn't has to clash, but it only encourages waste. Encoding all kinds 
of information into dev_t has to plan for the worst case and that's the 
only reason to immediately go to 64 bits and most of the number space is 
simply unused and wasted.

> > The knowledge about dev_t is already reduced to a minimum in a lot of 
> > block device drivers. register_blkdev() is already pretty much a dummy and 
> > not a requirement anymore.
> 
> So why do you think Andries patch clashes?
> 
> Also, I don't think your patch is proper. The point about having a single
> disk number space means that IDE and SCSI disks would show up there too -
> users simply shouldn't need to care about the differences (which is not
> just major numbers, but also the silly difference in how the partitioning
> splits minor numbers).

Look at it this way: the whole range from 0 to ... is one big major 
number, so old device numbers show up their as well.
Only user space "knows" about the special from some of the bits, but the 
kernel certainly doesn't care.

> But at the same time, for backwards compatibility clearly they have to
> show up in the _old_ places too. Which really implies that there needs to
> be a mapping function for the old numbers into the proper block device
> queues etc, so that people can still use the old /dev nodes when they
> upgrade their kernel.

Why should the kernel care about this? Most programs only want to open 
/dev/sd... Until the system tools are updated they will only see the old 
numbers anyway, later they won't care anymore either.
You didn't want that a device shows twice under devfs (e.g. as sda and 
scsi/host0/bus0/target0/lun0). How is that suddenly different?

bye, Roman

Re: [PATCH] kdevt-diff

[Linus Torvalds]

On Mon, 14 Apr 2003, Roman Zippel wrote:
> 
> > But at the same time, for backwards compatibility clearly they have to
> > show up in the _old_ places too. Which really implies that there needs to
> > be a mapping function for the old numbers into the proper block device
> > queues etc, so that people can still use the old /dev nodes when they
> > upgrade their kernel.
> 
> Why should the kernel care about this?

Because a kernel that cannot be used with old filesystems is a _useless_ 
kernel as far as I'm concerned.

Backwards compatibility is _important_. It's HUGELY important. It's just
possibly more important than anything else the kernel ever can do.

And new kernels need to be able to seamlessly boot into a disk-image that 
may still need to be used from an old kernel. Without any magic going on.

We can discontinue the old IDE/SCSI majors one or two stable releases 
_after_ we've switched over to a global "disk major". In other words, 
that's about five years down the line after you shouldnä't have to care 
any more.

		Linus

Re: [PATCH] kdevt-diff

[Roman Zippel]

Hi,

On Mon, 14 Apr 2003, Linus Torvalds wrote:

> Because a kernel that cannot be used with old filesystems is a _useless_ 
> kernel as far as I'm concerned.
> 
> Backwards compatibility is _important_. It's HUGELY important. It's just
> possibly more important than anything else the kernel ever can do.
> 
> And new kernels need to be able to seamlessly boot into a disk-image that 
> may still need to be used from an old kernel. Without any magic going on.
> 
> We can discontinue the old IDE/SCSI majors one or two stable releases 
> _after_ we've switched over to a global "disk major". In other words, 
> that's about five years down the line after you shouldnä't have to care 
> any more.

I agree and as long as you don't tell the kernel otherwise it will accept 
the old numbers, but if you start to support larger minor numbers now, you 
will have to support them even longer.

bye, Roman

Re: [PATCH] kdevt-diff

[Roman Zippel]

Hi,

On Mon, 14 Apr 2003, Linus Torvalds wrote:

> > Linus, if you still want to go for a single block device major, this patch 
> > is bad idea (at least in this form).
> 
> I disagree.

Ok, here is a compromise proposal. I don't care very much about the MKDEV 
macro and almost nobody else should care about it either.
My main concern with a larger dev_t is that people start to go wild and 
waste the number range with crap. So what I'd like to see is some usage 
policy, e.g. nobody should assume a certain dev_t size, so that it's still 
possible to scale it down. If the user has only a small number of devices, 
they should be addressable even with a 16 bit dev_t.

BTW there are a few more functions missing, we need a dev_to_u32() and a 
dev_to_u16(), so e.g. file systems can do something useful in mknod if 
they can't store the complete number.

bye, Roman

Re: [PATCH] kdevt-diff

[Pavel Machek]

Hi!

> > > Linus, if you still want to go for a single block device major, this patch 
> > > is bad idea (at least in this form).
> > 
> > I disagree.
> 
> Ok, here is a compromise proposal. I don't care very much about the MKDEV 
> macro and almost nobody else should care about it either.
> My main concern with a larger dev_t is that people start to go wild and 
> waste the number range with crap. So what I'd like to see is some usage 
> policy, e.g. nobody should assume a certain dev_t size, so that it's still 
> possible to scale it down. If the user has only a small number of devices, 
> they should be addressable even with a 16 bit dev_t.

What's the point? Its not like 8 bytes are that much... Your "number
range" is essentially free, and its okay to waste it...
									Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [PATCH] kdevt-diff

[Andries.Brouwer]

> Do all devices smaller than 16:16 appear the same on all
> filesystems, limited or not?

Yes.

Re: [PATCH] kdevt-diff

[Joel Becker]

On Tue, Apr 15, 2003 at 12:01:09AM +0200, Andries.Brouwer@cwi.nl wrote:
> > Do all devices smaller than 16:16 appear the same on all
> > filesystems, limited or not?
> 
> Yes.

	I meant "is that the behavior we want to choose?"  I understand
that this would be the result of your current scheme.
	I guess I'm wondering what made you choose "consistency across
legacy filesystems" over "consistency across our expanded device space".

Joel

-- 

Life's Little Instruction Book #444

	"Never underestimate the power of a kind word or deed."

Joel Becker
Senior Member of Technical Staff
Oracle Corporation
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH] kdevt-diff

[Valdis.Kletnieks]

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

On Mon, 14 Apr 2003 15:11:10 PDT, Joel Becker said:

> 	I guess I'm wondering what made you choose "consistency across
> legacy filesystems" over "consistency across our expanded device space".

I'm going to take a wild stab in the dark and say that being able to
boot using the /dev on an iso9660 CD is a requirement for most distros?

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

Re: [PATCH] kdevt-diff

[Joel Becker]

On Mon, Apr 14, 2003 at 06:18:20PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 14 Apr 2003 15:11:10 PDT, Joel Becker said:
> > 	I guess I'm wondering what made you choose "consistency across
> > legacy filesystems" over "consistency across our expanded device space".
> 
> I'm going to take a wild stab in the dark and say that being able to
> boot using the /dev on an iso9660 CD is a requirement for most distros?

	Um, most distros mount a ramdisk.  Also, the number of devices
needed for a boot surely fit in the 8:8 legacy space?  I'm willing to
hear dissent, I just wanted to know the rationale.

Joel

-- 

"The opposite of a correct statement is a false statement. The
 opposite of a profound truth may well be another profound truth."
         - Niels Bohr 

Joel Becker
Senior Member of Technical Staff
Oracle Corporation
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH] kdevt-diff

[H. Peter Anvin]

Followup to:  <200304142218.h3EMIKIO017775@turing-police.cc.vt.edu>
By author:    Valdis.Kletnieks@vt.edu
In newsgroup: linux.dev.kernel
>
> --==_Exmh_-1812532000P
> Content-Type: text/plain; charset=us-ascii
> 
> On Mon, 14 Apr 2003 15:11:10 PDT, Joel Becker said:
> 
> > 	I guess I'm wondering what made you choose "consistency across
> > legacy filesystems" over "consistency across our expanded device space".
> 
> I'm going to take a wild stab in the dark and say that being able to
> boot using the /dev on an iso9660 CD is a requirement for most distros?
> 

Not really, but it's certainly a nice capability.  However, iso9660
(RockRidge, actually) has 64 bits of dev_t space; it's actually split
into two 32-bit entries specified as "high 32 bits" and "low 32 bits."

I'm not positive if Linux expects those to contain major:minor or
0:<16-bit-dev_t>.

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64

Re: [PATCH] kdevt-diff

[Andries Brouwer]

On Wed, Apr 16, 2003 at 09:48:16AM -0700, H. Peter Anvin wrote:

> Not really, but it's certainly a nice capability.  However, iso9660
> (RockRidge, actually) has 64 bits of dev_t space; it's actually split
> into two 32-bit entries specified as "high 32 bits" and "low 32 bits."
> 
> I'm not positive if Linux expects those to contain major:minor or
> 0:<16-bit-dev_t>.

      case SIG('P','N'):
        { int high, low;
          high = isonum_733(rr->u.PN.dev_high);
          low = isonum_733(rr->u.PN.dev_low);
          /*
           * The Rock Ridge standard specifies that if sizeof(dev_t) <= 4,
           * then the high field is unused, and the device number is completely
           * stored in the low field.  Some writers may ignore this subtlety,
           * and as a result we test to see if the entire device number is
           * stored in the low field, and use that.
           */
          if((low & ~0xff) && high == 0) {
            inode->i_rdev = mk_kdev(low >> 8, low & 0xff);
          } else {
            inode->i_rdev = mk_kdev(high, low);
          }
        }
        break;

(Here isonum_733 gets 32 bits.)

Re: [PATCH] kdevt-diff

[H. Peter Anvin]

Andries Brouwer wrote:
> On Wed, Apr 16, 2003 at 09:48:16AM -0700, H. Peter Anvin wrote:
> 
> 
>>Not really, but it's certainly a nice capability.  However, iso9660
>>(RockRidge, actually) has 64 bits of dev_t space; it's actually split
>>into two 32-bit entries specified as "high 32 bits" and "low 32 bits."
>>
>>I'm not positive if Linux expects those to contain major:minor or
>>0:<16-bit-dev_t>.
> 
>       case SIG('P','N'):
>         { int high, low;
>           high = isonum_733(rr->u.PN.dev_high);
>           low = isonum_733(rr->u.PN.dev_low);
>           /*
>            * The Rock Ridge standard specifies that if sizeof(dev_t) <= 4,
>            * then the high field is unused, and the device number is completely
>            * stored in the low field.  Some writers may ignore this subtlety,
>            * and as a result we test to see if the entire device number is
>            * stored in the low field, and use that.
>            */
>           if((low & ~0xff) && high == 0) {
>             inode->i_rdev = mk_kdev(low >> 8, low & 0xff);
>           } else {
>             inode->i_rdev = mk_kdev(high, low);
>           }
>         }
>         break;
> 
> (Here isonum_733 gets 32 bits.)

OK, in other words, it accepts either format, either:

0x000000000000MMmm or 0x000000MM000000mm

This is perfectly consistent with what dev_t does for a 32:32 split; the
previous form would be the "backwards compatibility" format and the
second the "new" format.

	-hpa

Re: [PATCH] kdevt-diff

[Andries.Brouwer]

> BTW there are a few more functions missing, we need a dev_to_u32()
> and a dev_to_u16(), so e.g. file systems can do something useful
> in mknod if they can't store the complete number.

In such cases I cannot see any meaningful action other than returning
EOVERFLOW (or EINVAL in case you prefer that).

Andries

Re: [PATCH] kdevt-diff

[Roman Zippel]

Hi,
On Tue, 15 Apr 2003 Andries.Brouwer@cwi.nl wrote:

> > BTW there are a few more functions missing, we need a dev_to_u32()
> > and a dev_to_u16(), so e.g. file systems can do something useful
> > in mknod if they can't store the complete number.
> 
> In such cases I cannot see any meaningful action other than returning
> EOVERFLOW (or EINVAL in case you prefer that).

Yes, that's what I had in mind.

bye, Roman