[PATCH] Re: NFS "dev_t" issues..

[PATCH] Re: NFS "dev_t" issues..

[Jeff Garzik]

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

Linus Torvalds wrote:
> Because the types aren't at all compatible any more, the macros that are
> used for user-level "dev_t" are no longer working for a kdev_t. So we have
> 
>         dev_t                   kdev_t
> 
>         MKDEV(major,minor)      mk_kdev(major, minor)
>         MAJOR(dev)              major(dev)
>         MINOR(dev)              minor(dev)
>         dev == dev2             kdev_same(dev, dev2)
>         !dev                    kdev_none(dev)

And, assignments "kdev_t foo = 0" become "kdev_t foo = NODEV".

At least I hope so ;-)  The below patch fixes up rd.c and loop.c.

-- 
Jeff Garzik      | Only so many songs can be sung
Building 1024    | with two lips, two lungs, and one tongue.
MandrakeSoft     |         - nomeansno

[-- Attachment #2: block.patch --]
[-- Type: text/plain, Size: 4331 bytes --]

Index: drivers/block/rd.c
===================================================================
RCS file: /cvsroot/gkernel/linux_2_5/drivers/block/rd.c,v
retrieving revision 1.7
diff -u -r1.7 rd.c
--- drivers/block/rd.c	2001/12/20 18:55:32	1.7
+++ drivers/block/rd.c	2002/01/01 22:35:11
@@ -246,7 +246,7 @@
 	unsigned long offset, len;
 	int rw = sbh->bi_rw;
 
-	minor = MINOR(sbh->bi_dev);
+	minor = minor(sbh->bi_dev);
 
 	if (minor >= NUM_RAMDISKS)
 		goto fail;
@@ -280,10 +280,10 @@
 	int error = -EINVAL;
 	unsigned int minor;
 
-	if (!inode || !inode->i_rdev) 	
+	if (!inode || kdev_none(inode->i_rdev))
 		goto out;
 
-	minor = MINOR(inode->i_rdev);
+	minor = minor(inode->i_rdev);
 
 	switch (cmd) {
 		case BLKFLSBUF:
@@ -407,7 +407,7 @@
 		rd_bdev[i] = NULL;
 		if (bdev)
 			blkdev_put(bdev, BDEV_FILE);
-		destroy_buffers(MKDEV(MAJOR_NR, i));
+		destroy_buffers(mk_kdev(MAJOR_NR, i));
 	}
 
 	devfs_unregister (devfs_handle);
@@ -449,7 +449,7 @@
 			       &rd_bd_op, NULL);
 
 	for (i = 0; i < NUM_RAMDISKS; i++)
-		register_disk(NULL, MKDEV(MAJOR_NR,i), 1, &rd_bd_op, rd_size<<1);
+		register_disk(NULL, mk_kdev(MAJOR_NR,i), 1, &rd_bd_op, rd_size<<1);
 
 #ifdef CONFIG_BLK_DEV_INITRD
 	/* We ought to separate initrd operations here */
Index: drivers/block/loop.c
===================================================================
RCS file: /cvsroot/gkernel/linux_2_5/drivers/block/loop.c,v
retrieving revision 1.7
diff -u -r1.7 loop.c
--- drivers/block/loop.c	2001/12/30 22:22:58	1.7
+++ drivers/block/loop.c	2002/01/01 22:35:21
@@ -155,8 +155,8 @@
 {
 	if (S_ISREG(lo_dentry->d_inode->i_mode))
 		return (lo_dentry->d_inode->i_size - lo->lo_offset) >> BLOCK_SIZE_BITS;
-	if (blk_size[MAJOR(lodev)])
-		return blk_size[MAJOR(lodev)][MINOR(lodev)] -
+	if (blk_size[major(lodev)])
+		return blk_size[major(lodev)][minor(lodev)] -
                                 (lo->lo_offset >> BLOCK_SIZE_BITS);
 	return MAX_DISK_SIZE;
 }
@@ -379,7 +379,7 @@
  */
 static int loop_end_io_transfer(struct bio *bio, int nr_sectors)
 {
-	struct loop_device *lo = &loop_dev[MINOR(bio->bi_dev)];
+	struct loop_device *lo = &loop_dev[minor(bio->bi_dev)];
 	int uptodate = test_bit(BIO_UPTODATE, &bio->bi_flags);
 
 	if (!uptodate || bio_rw(bio) == WRITE) {
@@ -429,10 +429,10 @@
 	unsigned long IV;
 	int rw = bio_rw(rbh);
 
-	if (MINOR(rbh->bi_dev) >= max_loop)
+	if (minor(rbh->bi_dev) >= max_loop)
 		goto out;
 
-	lo = &loop_dev[MINOR(rbh->bi_dev)];
+	lo = &loop_dev[minor(rbh->bi_dev)];
 	spin_lock_irq(&lo->lo_lock);
 	if (lo->lo_state != Lo_bound)
 		goto inactive;
@@ -615,7 +615,7 @@
 
 	if (S_ISBLK(inode->i_mode)) {
 		lo_device = inode->i_rdev;
-		if (lo_device == dev) {
+		if (kdev_same(lo_device, dev)) {
 			error = -EBUSY;
 			goto out;
 		}
@@ -725,7 +725,7 @@
 	loop_release_xfer(lo);
 	lo->transfer = NULL;
 	lo->ioctl = NULL;
-	lo->lo_device = 0;
+	lo->lo_device = NODEV;
 	lo->lo_encrypt_type = 0;
 	lo->lo_offset = 0;
 	lo->lo_encrypt_key_size = 0;
@@ -818,12 +818,12 @@
 
 	if (!inode)
 		return -EINVAL;
-	if (MAJOR(inode->i_rdev) != MAJOR_NR) {
+	if (major(inode->i_rdev) != MAJOR_NR) {
 		printk(KERN_WARNING "lo_ioctl: pseudo-major != %d\n",
 		       MAJOR_NR);
 		return -ENODEV;
 	}
-	dev = MINOR(inode->i_rdev);
+	dev = minor(inode->i_rdev);
 	if (dev >= max_loop)
 		return -ENODEV;
 	lo = &loop_dev[dev];
@@ -873,11 +873,11 @@
 
 	if (!inode)
 		return -EINVAL;
-	if (MAJOR(inode->i_rdev) != MAJOR_NR) {
+	if (major(inode->i_rdev) != MAJOR_NR) {
 		printk(KERN_WARNING "lo_open: pseudo-major != %d\n", MAJOR_NR);
 		return -ENODEV;
 	}
-	dev = MINOR(inode->i_rdev);
+	dev = minor(inode->i_rdev);
 	if (dev >= max_loop)
 		return -ENODEV;
 
@@ -900,12 +900,12 @@
 
 	if (!inode)
 		return 0;
-	if (MAJOR(inode->i_rdev) != MAJOR_NR) {
+	if (major(inode->i_rdev) != MAJOR_NR) {
 		printk(KERN_WARNING "lo_release: pseudo-major != %d\n",
 		       MAJOR_NR);
 		return 0;
 	}
-	dev = MINOR(inode->i_rdev);
+	dev = minor(inode->i_rdev);
 	if (dev >= max_loop)
 		return 0;
 
@@ -1016,7 +1016,7 @@
 	blk_size[MAJOR_NR] = loop_sizes;
 	blksize_size[MAJOR_NR] = loop_blksizes;
 	for (i = 0; i < max_loop; i++)
-		register_disk(NULL, MKDEV(MAJOR_NR, i), 1, &lo_fops, 0);
+		register_disk(NULL, mk_kdev(MAJOR_NR, i), 1, &lo_fops, 0);
 
 	printk(KERN_INFO "loop: loaded (max %d devices)\n", max_loop);
 	return 0;

Re: [PATCH] Re: NFS "dev_t" issues..

[Linus Torvalds]

On Tue, 1 Jan 2002, Jeff Garzik wrote:
>
> And, assignments "kdev_t foo = 0" become "kdev_t foo = NODEV".

Indeed.

> At least I hope so ;-)  The below patch fixes up rd.c and loop.c.

Looks good, applied,

		Linus

[PATCH] Re: NFS "dev_t" issues..

[Jeff Garzik]

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

Linus,

What do you think about the attached simple patch, making the cookie
size more explicit?

I was looking at fixing up reiserfs, which already has 32-bit storage
for dev_t on-disk, and the following change came to mind.

-- 
Jeff Garzik      | Only so many songs can be sung
Building 1024    | with two lips, two lungs, and one tongue.
MandrakeSoft     |         - nomeansno

[-- Attachment #2: kdev.patch --]
[-- Type: text/plain, Size: 528 bytes --]

Index: include/linux/kdev_t.h
===================================================================
RCS file: /cvsroot/gkernel/linux_2_5/include/linux/kdev_t.h,v
retrieving revision 1.2
diff -u -r1.2 kdev_t.h
--- include/linux/kdev_t.h	2002/01/01 22:41:11	1.2
+++ include/linux/kdev_t.h	2002/01/01 23:02:10
@@ -86,7 +86,7 @@
  * internal equality comparisons and for things
  * like NFS filehandle conversion.
  */
-static inline unsigned int kdev_val(kdev_t dev)
+static inline u32 kdev_val(kdev_t dev)
 {
 	return dev.value;
 }

Re: [PATCH] Re: NFS "dev_t" issues..

[Linus Torvalds]

On Tue, 1 Jan 2002, Jeff Garzik wrote:
>
> What do you think about the attached simple patch, making the cookie
> size more explicit?

Well, I suspect that we actually should also make the format explicit, and
basically use the same translation that I did for the NFS filehandle. That
way it's still just a cookie, but it's a cookie with (a) explicit size and
(b) meaning that won't change over different kernel revisions.

Hmm?

			Linus

Re: [PATCH] Re: NFS "dev_t" issues..

[Jeff Garzik]

Linus Torvalds wrote:
> 
> On Tue, 1 Jan 2002, Jeff Garzik wrote:
> >
> > What do you think about the attached simple patch, making the cookie
> > size more explicit?
> 
> Well, I suspect that we actually should also make the format explicit, and
> basically use the same translation that I did for the NFS filehandle. That
> way it's still just a cookie, but it's a cookie with (a) explicit size and
> (b) meaning that won't change over different kernel revisions.

true, each filesystem needs to figure out how to make sure their on-disk
format doesn't change across kernel revisions...  Storing the raw i_rdev
onto disk definitely silly but it appears to be an issue some
filesystems will have to deal with.  I'm leaving reiserfs alone so they
can make a policy decision...

-- 
Jeff Garzik      | Only so many songs can be sung
Building 1024    | with two lips, two lungs, and one tongue.
MandrakeSoft     |         - nomeansno

Re: [PATCH] Re: NFS "dev_t" issues..

[Andries.Brouwer]

I see lots of people sending patches for kdev_t.
In order to possibly avoid duplication of work,
I put my patch at ftp.kernel.org:
2.5.2pre6-kdev_t-diff (415841 bytes)

It has kminor and kmajor, but if that is not desired
a very simple edit on the patch will turn them into
minor and major.

(It is incomplete, but a good start.)

Andries

Re: [PATCH] Re: NFS "dev_t" issues..

[Andries.Brouwer]

    From alessandro.suardi@oracle.com Thu Jan  3 00:22:23 2002

    Andries.Brouwer@cwi.nl wrote:
    > 
    > I see lots of people sending patches for kdev_t.
    > In order to possibly avoid duplication of work,
    > I put my patch at ftp.kernel.org:
    > 2.5.2pre6-kdev_t-diff (415841 bytes)
    > 
    > It has kminor and kmajor, but if that is not desired
    > a very simple edit on the patch will turn them into
    > minor and major.
    > 
    > (It is incomplete, but a good start.)

    It doesn't build for me in make_rdonly() in ext3 with debug
     configured in:

Yes. Still w.i.p. but a better version is now
2.5.2pre6-kdev_t-diff-v3 (443024 bytes).

Andries

Re: [PATCH] Re: NFS "dev_t" issues..

[Alessandro Suardi]

Andries.Brouwer@cwi.nl wrote:
> 
>     From alessandro.suardi@oracle.com Thu Jan  3 00:22:23 2002
> 
>     It doesn't build for me in make_rdonly() in ext3 with debug
>      configured in:
> 
> Yes. Still w.i.p. but a better version is now
> 2.5.2pre6-kdev_t-diff-v3 (443024 bytes).

Compiles and boots fine with my config - laptop including ext2,
 ext3, fat, vfat, reiserfs, iso9660, tmpfs, ramfs, IrDA, PPP,
 Xircom Cardbus (old driver), parport, floppy, ramdisk, loop,
 IDE, ipv4, nfs/nfsd, samba, maestro3, usb UHCI and more. Not
 everything tested and one possibly related warning:

gcc -D__KERNEL__ -I/usr/src/linux-2.5.2-pre6/include -Wall -Wstrict-prototypes -Wno-trigraphs -O2 -fomit-frame-pointer -fno-strict-aliasing -fno-common -pipe -mpreferred-stack-boundary=2 -march=i686
-DMODULE   -c -o write.o write.c
write.c: In function `nfs_commit_done':
write.c:1204: warning: unsigned int format, kdev_t arg (arg 2)

Quite good so far :) thanks a lot,

--alessandro

 "this machine will, will not communicate
   these thoughts and the strain I am under
  be a world child, form a circle before we all go under"
                         (Radiohead, "Street Spirit [fade out]")