Skip to contentSkip to conversation

NFS "dev_t" issues..

PATCH New Linus Torvalds 7 participants 14 messages Jan 1, 2002 – Jan 9, 2002

Subsystems (8)
VFS SCSI USB Block layer +4 more subsystems
lore
Share & tools
lore

Linus Torvalds

author
#0 lore raw

I made a pre6, which contains a new-and-anal "kdev_t".

The format of the thing is the same as it used to be, ie 16 bits of information, but I made it a structure so that you _couldn't_ mix up "dev_t" and "kdev_t", or use the "kdev_t" as a number (so when kdev_t expands to 12+20 bits later in 2.5.x you shouldn't get surprises)

I fixed up the stuff I use and which showed up in compiles (on a source level, it's so far totally untested), but I'd really like people to check out their own subsystems. _Especially_ NFS and NFSD, which had several cases of mixing the two dev_t's around, and which also used them as numbers. Trond, Neil?

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 _most_ of the time the fixes are trivial - just translate as above. It only gets interesting when you have code that looks at the value or starts mixing the two and compares a "dev_t" against a "kdev_t", which can be quite interesting.

The knfsd file handle thing is also an issue - Neil, please check out that what I did looks sane, and would be on-the-wire-compatible with the old behaviour, even when we expand kdev_t to 12+20 bits, ok?

(Marcelo, for easier backporting of drivers to 2.4.x, we'll probably want to eventually add

	#define mk_kdev(a,b) MKDEV(a,b)
	#define major(d) MAJOR(d)
	...

to the 2.4.x <linux/kdev_t.h> so that you can move drivers back and forth).

Apart from some knfsd issues, most of the kdev_t users were proper. The strict type-checking found one bug in the SCSI layer (which I knew about, and was one of the impetuses for doing it in the first place), and found a lot of small "works-but-will-break-with-a-bigger-kdev_t" issues).

Linus

#1 lore raw
Quoted 10 lines · Linus Torvalds

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.

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

Index: drivers/block/rd.c
===================================================================
RCS file: /cvsroot/gkernel/linux_2_5/drivers/block/rd.c,v retrieving revision 1.7

block/rd.c +5 -5
diff -u -r1.7 rd.c
@@ -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

block/loop.c +14 -14
diff -u -r1.7 loop.c
@@ -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;
author
#2 lore raw
Quoted 2 lines · Jeff Garzik

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

Indeed.

Quoted 1 line

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

Looks good, applied,

Linus

#3 lore raw
In reply to Linus Torvalds
Quoted 4 lines · Linus Torvalds

Apart from some knfsd issues, most of the kdev_t users were proper. The strict type-checking found one bug in the SCSI layer (which I knew about, and was one of the impetuses for doing it in the first place), and found a lot of small "works-but-will-break-with-a-bigger-kdev_t" issues).

Sigh... Most of the ->i_dev instances are crap and ought to be replaced with ->i_sb. At the very least, let's

tmp/namei.c renamed from fs/namei.c +3 -3
@@ -1589,7 +1589,7 @@
 		goto exit_lock;
 
 	error = -EXDEV;
-	if (!kdev_same(dir->i_dev, inode->i_dev))
+	if (dir->i_sb != inode->i_sb)
 		goto exit_lock;
 
 	/*

@@ -1707,7 +1707,7 @@
 	if (error)
 		return error;
 
-	if (!kdev_same(new_dir->i_dev, old_dir->i_dev))
+	if (new_dir->i_sb != old_dir->i_sb)
 		return -EXDEV;
 
 	if (!new_dentry->d_inode)

@@ -1787,7 +1787,7 @@
 	if (error)
 		return error;
 
-	if (!kdev_same(new_dir->i_dev, old_dir->i_dev))
+	if (new_dir->i_sb != old_dir->i_sb)
 		return -EXDEV;
 
 	if (!new_dentry->d_inode)
author
#4 lore raw
Quoted 3 lines · Alexander Viro

Sigh... Most of the ->i_dev instances are crap and ought to be replaced with ->i_sb. At the very least, let's

Looks good, and yes, it makes a lot more sense from an EXDEV standpoint to check the superblock, as "i_dev" really has no good meaning for many filesystems.

Linus

#5 lore raw
In reply to Linus Torvalds

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.

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

Index: include/linux/kdev_t.h
===================================================================
RCS file: /cvsroot/gkernel/linux_2_5/include/linux/kdev_t.h,v retrieving revision 1.2

linux/kdev_t.h +1 -1
diff -u -r1.2 kdev_t.h
@@ -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;
 }
author
#6 lore raw
Quoted 3 lines · Jeff Garzik

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

#7 lore raw
Quoted 10 lines · Linus Torvalds
Quoted 3 lines · Jeff Garzik

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...

Signature · 3 lines
Jeff Garzik      | Only so many songs can be sung
Building 1024    | with two lips, two lungs, and one tongue.
MandrakeSoft     |         - nomeansno
#8 lore raw
In reply to Linus Torvalds
Quoted 2 lines · Linus Torvalds

I made a pre6, which contains a new-and-anal "kdev_t".

Here's a patch to fix the usb code drivers.

thanks,

greg k-h

drivers/usb/acm.c +1 -1
diff -Nru a/drivers/usb/acm.c b/drivers/usb/acm.c
@@ -297,7 +297,7 @@
 
 static int acm_tty_open(struct tty_struct *tty, struct file *filp)
 {
-	struct acm *acm = acm_table[MINOR(tty->device)];
+	struct acm *acm = acm_table[minor(tty->device)];
 
 	if (!acm || !acm->dev) return -EINVAL;
drivers/usb/bluetooth.c +1 -1
diff -Nru a/drivers/usb/bluetooth.c b/drivers/usb/bluetooth.c
@@ -360,7 +360,7 @@
 	tty->driver_data = NULL;
 
 	/* get the bluetooth object associated with this tty pointer */
-	bluetooth = get_bluetooth_by_minor (MINOR(tty->device));
+	bluetooth = get_bluetooth_by_minor (minor(tty->device));
 
 	if (bluetooth_paranoia_check (bluetooth, __FUNCTION__)) {
 		return -ENODEV;
drivers/usb/dabusb.c +1 -1
diff -Nru a/drivers/usb/dabusb.c b/drivers/usb/dabusb.c
@@ -579,7 +579,7 @@
 
 static int dabusb_open (struct inode *inode, struct file *file)
 {
-	int devnum = MINOR (inode->i_rdev);
+	int devnum = minor (inode->i_rdev);
 	pdabusb_t s;
 
 	if (devnum < DABUSB_MINOR || devnum >= (DABUSB_MINOR + NRDABUSB))
drivers/usb/dc2xx.c +1 -1
diff -Nru a/drivers/usb/dc2xx.c b/drivers/usb/dc2xx.c
@@ -298,7 +298,7 @@
 	int			value = 0;
 
 	down (&state_table_mutex);
-	subminor = MINOR (inode->i_rdev) - USB_CAMERA_MINOR_BASE;
+	subminor = minor (inode->i_rdev) - USB_CAMERA_MINOR_BASE;
 	if (subminor < 0 || subminor >= MAX_CAMERAS
 			|| !(camera = minor_data [subminor])) {
 		up (&state_table_mutex);
drivers/usb/hiddev.c +1 -1
diff -Nru a/drivers/usb/hiddev.c b/drivers/usb/hiddev.c
@@ -218,7 +218,7 @@
 static int hiddev_open(struct inode * inode, struct file * file) {
 	struct hiddev_list *list;
 
-	int i = MINOR(inode->i_rdev) - HIDDEV_MINOR_BASE;
+	int i = minor(inode->i_rdev) - HIDDEV_MINOR_BASE;
 
 	if (i >= HIDDEV_MINORS || !hiddev_table[i])
 		return -ENODEV;
drivers/usb/printer.c +1 -1
diff -Nru a/drivers/usb/printer.c b/drivers/usb/printer.c
@@ -201,7 +201,7 @@
 
 static int usblp_open(struct inode *inode, struct file *file)
 {
-	int minor = MINOR(inode->i_rdev) - USBLP_MINOR_BASE;
+	int minor = minor(inode->i_rdev) - USBLP_MINOR_BASE;
 	struct usblp *usblp;
 	int retval;
drivers/usb/scanner.c +6 -8
diff -Nru a/drivers/usb/scanner.c b/drivers/usb/scanner.c
@@ -365,7 +365,7 @@
 	struct scn_usb_data *scn;
 	struct usb_device *dev;
 
-	kdev_t scn_minor;
+	int scn_minor;
 
 	int err=0;
 

@@ -432,7 +432,7 @@
 {
 	struct scn_usb_data *scn;
 
-	kdev_t scn_minor;
+	int scn_minor;
 
 	scn_minor = USB_SCN_MINOR (inode);
 

@@ -469,7 +469,7 @@
 	ssize_t bytes_written = 0; /* Overall count of bytes written */
 	ssize_t ret = 0;
 
-	kdev_t scn_minor;
+	int scn_minor;
 
 	int this_write;		/* Number of bytes to write */
 	int partial;		/* Number of bytes successfully written */

@@ -556,8 +556,7 @@
 	ssize_t bytes_read;	/* Overall count of bytes_read */
 	ssize_t ret;
 
-	kdev_t scn_minor;
-
+	int scn_minor;
 	int partial;		/* Number of bytes successfully read */
 	int this_read;		/* Max number of bytes to read */
 	int result;

@@ -671,7 +670,7 @@
 {
 	struct usb_device *dev;
 
-	kdev_t scn_minor;
+	int scn_minor;
 
 	scn_minor = USB_SCN_MINOR(inode);
 

@@ -810,8 +809,7 @@
 
 	int ep_cnt;
 	int ix;
-
-	kdev_t scn_minor;
+	int scn_minor;
 
 	char valid_device = 0;
 	char have_bulk_in, have_bulk_out, have_intr;
drivers/usb/scanner.h +2 -2
diff -Nru a/drivers/usb/scanner.h b/drivers/usb/scanner.h
@@ -203,7 +203,7 @@
 #define IS_EP_BULK_OUT(ep) (IS_EP_BULK(ep) && ((ep).bEndpointAddress & USB_ENDPOINT_DIR_MASK) == USB_DIR_OUT)
 #define IS_EP_INTR(ep) ((ep).bmAttributes == USB_ENDPOINT_XFER_INT ? 1 : 0)
 
-#define USB_SCN_MINOR(X) MINOR((X)->i_rdev) - SCN_BASE_MNR
+#define USB_SCN_MINOR(X) minor((X)->i_rdev) - SCN_BASE_MNR
 
 #ifdef DEBUG
 #define SCN_DEBUG(X) X

@@ -243,7 +243,7 @@
 	devfs_handle_t devfs;	/* devfs device */
 	struct urb scn_irq;
 	unsigned int ifnum;	/* Interface number of the USB device */
-	kdev_t scn_minor;	/* Scanner minor - used in disconnect() */
+	int scn_minor;		/* Scanner minor - used in disconnect() */
 	unsigned char button;	/* Front panel buffer */
 	char isopen;		/* Not zero if the device is open */
 	char present;		/* Not zero if device is present */
drivers/usb/serial/usbserial.c +2 -2
diff -Nru a/drivers/usb/serial/usbserial.c b/drivers/usb/serial/usbserial.c
@@ -514,14 +514,14 @@
 	tty->driver_data = NULL;
 
 	/* get the serial object associated with this tty pointer */
-	serial = get_serial_by_minor (MINOR(tty->device));
+	serial = get_serial_by_minor (minor(tty->device));
 
 	if (serial_paranoia_check (serial, __FUNCTION__)) {
 		return -ENODEV;
 	}
 
 	/* set up our port structure making the tty driver remember our port object, and us it */
-	portNumber = MINOR(tty->device) - serial->minor;
+	portNumber = minor(tty->device) - serial->minor;
 	port = &serial->port[portNumber];
 	tty->driver_data = port;
 	port->tty = tty;
#9 lore raw

PATCH 1/1 Make 2.5.2-pre6 usable

In reply to Linus Torvalds

The attached patch is making the mentioned kernel usable by propagating the kdev_t handling to much more dirvers.

Of esp. interrest is the fact that there where some places, where NODEV was used instead of ENODEV:

linux-new/drivers/net/defxx.c
linux-new/fs/dquot.c
linux-new/drivers/media/video/cpia.c

#10 lore raw

PATCH 1/1 usabe2-2.5.2-pre6

In reply to Linus Torvalds

The attached patch is making the SCSI "mid-layer" vomit bag usable again. I have fixed some very ugly offenders as well in due course:

1. Moved the "informative" scsi_device_types[MAX_SCSI_DEVICE_CODE];
2. Unneccessary/inappriopriate locking in ppa.c
3. "Write only" field in Scsi_CD struct:
unsigned readcd_cdda:1; /* reading audio data using READ_CD */

It get's only set but is never used and never needed.

After having a look at this SCSI code I really got the opinnion that it was written buy someone who didn't know what functions and local variables are good for. Everything there seems to be done in a maximally terse style...

Anyway.

#11 lore raw
In reply to Linus Torvalds
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

" " == Linus Torvalds <[email protected]> writes:

> I made a pre6, which contains a new-and-anal "kdev_t".

<snip>

> I fixed up the stuff I use and which showed up in compiles (on > a source level, it's so far totally untested), but I'd really > like people to check out their own subsystems. _Especially_ NFS > and NFSD, which had several cases of mixing the two dev_t's > around, and which also used them as numbers. Trond, Neil?

Hi Linus & Marcelo,

Sorry I'm a bit late in replying. AFAICS as of 2.5.2-pre9, all is
more or less well, however when reviewing that code, I noticed what is probably a bug:

Given that (for character devices) the value of inode->i_cdev in
2.[45].x depends on the i_rdev, it would appear to be a bug for us to be able to change inode->i_rdev *after* we've called init_special_inode().
For this reason, I'd advocate removing the lines in nfs_refresh_inode() that reset the inode->i_rdev (as per the patch below) and instead rely on the ordinary stale inode checks to tell us if/when the inode->i_rdev has changed.

It's hardly a new bug. It's been around ever since we added init_special_inode(), so it's clearly not one that bites us every day of the year. Even so, the same patch should probably be applied to 2.4.x.

Cheers,
Trond

fs/nfs/inode.c +0 -3
diff -u --recursive --new-file linux-2.5.2-pre9/fs/nfs/inode.c linux-2.5.2-fix/fs/nfs/inode.c
@@ -1107,9 +1107,6 @@
  		inode->i_blocks = fattr->du.nfs2.blocks;
  		inode->i_blksize = fattr->du.nfs2.blocksize;
  	}
- 	inode->i_rdev = NODEV;
- 	if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode))
- 		inode->i_rdev = to_kdev_t(fattr->rdev);
  
 	/* Update attrtimeo value */
 	if (!invalid && time_after(jiffies, NFS_ATTRTIMEO_UPDATE(inode)+NFS_ATTRTIMEO(inode))) {
#12 lore raw

PATCH 2.5.2-pre9 scsi cleanup

In reply to Linus Torvalds

The attached patch does the following.

1. Clean up some ifdef confusion in do_mount

2. Clean up the scsi code to make ppa.c work.

3. Clean up some unneccessary unneeded globals from scsi code.

4. Make a bit more sure, that the minor() and friends end up in unsigned int's.

drivers/scsi/dpt_i2o.c +3 -3
diff -ur linux-old/drivers/scsi/dpt_i2o.c linux/drivers/scsi/dpt_i2o.c
@@ -1551,7 +1551,7 @@
 
 static int adpt_open(struct inode *inode, struct file *file)
 {
-	int minor;
+	unsigned int minor;
 	adpt_hba* pHba;
 
 	//TODO check for root access

@@ -1584,7 +1584,7 @@
 
 static int adpt_close(struct inode *inode, struct file *file)
 {
-	int minor;
+	unsigned int minor;
 	adpt_hba* pHba;
 
 	minor = minor(inode->i_rdev);

@@ -1878,7 +1878,7 @@
 static int adpt_ioctl(struct inode *inode, struct file *file, uint cmd,
 	      ulong arg)
 {
-	int minor;
+	unsigned int minor;
 	int error = 0;
 	adpt_hba* pHba;
 	ulong flags;
drivers/scsi/fcal.c +0 -2
diff -ur linux-old/drivers/scsi/fcal.c linux/drivers/scsi/fcal.c
@@ -249,8 +249,6 @@
 				if (scd->host->host_no == hostno && scd->id == target) {
 					SPRINTF ("  [AL-PA: %02x, Id: %02d, Port WWN: %08x%08x, Node WWN: %08x%08x]  ",
 						alpa, target, u1[0], u1[1], u2[0], u2[1]);
-					SPRINTF ("%s ", (scd->type < MAX_SCSI_DEVICE_CODE) ?
-						scsi_device_types[(short) scd->type] : "Unknown device");
 
 					for (j = 0; (j < 8) && (scd->vendor[j] >= 0x20); j++)
 						SPRINTF ("%c", scd->vendor[j]);
drivers/scsi/g_NCR5380.c +1 -2
diff -ur linux-old/drivers/scsi/g_NCR5380.c linux/drivers/scsi/g_NCR5380.c
@@ -789,7 +789,6 @@
     struct NCR5380_hostdata *hostdata;
 #ifdef NCR5380_STATS
     Scsi_Device *dev;
-    extern const char *const scsi_device_types[MAX_SCSI_DEVICE_CODE];
 #endif
     save_flags(flags);
     cli();

@@ -833,7 +832,7 @@
 	    long tr = hostdata->time_read[dev->id] / HZ;
 	    long tw = hostdata->time_write[dev->id] / HZ;
 
-	    PRINTP("  T:%d %s " ANDP dev->id ANDP (dev->type < MAX_SCSI_DEVICE_CODE) ? scsi_device_types[(int)dev->type] : "Unknown");
+	    PRINTP("  T:%d " ANDP dev->id );
 	    for (i=0; i<8; i++)
 		if (dev->vendor[i] >= 0x20)
 		    *(buffer+(len++)) = dev->vendor[i];
drivers/scsi/ppa.c +3 -13
diff -ur linux-old/drivers/scsi/ppa.c linux/drivers/scsi/ppa.c
@@ -115,11 +115,6 @@
     int i, nhosts, try_again;
     struct parport *pb;
 
-    /*
-     * unlock to allow the lowlevel parport driver to probe
-     * the irqs
-     */
-    spin_unlock_irq(&io_request_lock);
     pb = parport_enumerate();
 
     printk("ppa: Version %s\n", PPA_VERSION);

@@ -128,7 +123,6 @@
 
     if (!pb) {
 	printk("ppa: parport reports no devices.\n");
-	spin_lock_irq(&io_request_lock);
 	return 0;
     }
   retry_entry:

@@ -154,7 +148,6 @@
 		      "pardevice is owning the port for too longtime!\n",
 			   i);
 		    parport_unregister_device(ppa_hosts[i].dev);
-		    spin_lock_irq(&io_request_lock);
 		    return 0;
 		}
 	    }

@@ -223,15 +216,12 @@
 	    printk("  supported by the imm (ZIP Plus) driver. If the\n");
 	    printk("  cable is marked with \"AutoDetect\", this is what has\n");
 	    printk("  happened.\n");
-	    spin_lock_irq(&io_request_lock);
 	    return 0;
 	}
 	try_again = 1;
 	goto retry_entry;
-    } else {
-	spin_lock_irq(&io_request_lock);
+    } else
 	return 1;		/* return number of hosts detected */
-    }
 }
 
 /* This is to give the ppa driver a way to modify the timings (and other

@@ -847,9 +837,9 @@
 
     tmp->cur_cmd = 0;
     
-    spin_lock_irqsave(&io_request_lock, flags);
+    spin_lock_irqsave(&cmd->host->host_lock, flags);
     cmd->scsi_done(cmd);
-    spin_unlock_irqrestore(&io_request_lock, flags);
+    spin_unlock_irqrestore(&cmd->host->host_lock, flags);
     return;
 }
drivers/scsi/scsi.c +1 -19
diff -ur linux-old/drivers/scsi/scsi.c linux/drivers/scsi/scsi.c
@@ -139,25 +139,7 @@
  */
 unsigned int scsi_logging_level;
 
-const char *const scsi_device_types[MAX_SCSI_DEVICE_CODE] =
-{
-	"Direct-Access    ",
-	"Sequential-Access",
-	"Printer          ",
-	"Processor        ",
-	"WORM             ",
-	"CD-ROM           ",
-	"Scanner          ",
-	"Optical Device   ",
-	"Medium Changer   ",
-	"Communications   ",
-	"Unknown          ",
-	"Unknown          ",
-	"Unknown          ",
-	"Enclosure        ",
-};
-
-/* 
+/*
  * Function prototypes.
  */
 extern void scsi_times_out(Scsi_Cmnd * SCpnt);
drivers/scsi/scsi.h +0 -3
diff -ur linux-old/drivers/scsi/scsi.h linux/drivers/scsi/scsi.h
@@ -89,9 +89,6 @@
 #define FALSE 0
 #endif
 
-#define MAX_SCSI_DEVICE_CODE 14
-extern const char *const scsi_device_types[MAX_SCSI_DEVICE_CODE];
-
 #ifdef DEBUG
 #define SCSI_TIMEOUT (5*HZ)
 #else
drivers/scsi/scsi_proc.c +2 -8
diff -ur linux-old/drivers/scsi/scsi_proc.c linux/drivers/scsi/scsi_proc.c
@@ -260,7 +260,6 @@
 {
 
 	int x, y = *size;
-	extern const char *const scsi_device_types[MAX_SCSI_DEVICE_CODE];
 
 	y = sprintf(buffer + len,
 	     "Host: scsi%d Channel: %02d Id: %02d Lun: %02d\n  Vendor: ",

@@ -285,13 +284,8 @@
 		else
 			y += sprintf(buffer + len + y, " ");
 	}
-	y += sprintf(buffer + len + y, "\n");
-
-	y += sprintf(buffer + len + y, "  Type:   %s ",
-		     scd->type < MAX_SCSI_DEVICE_CODE ?
-	       scsi_device_types[(int) scd->type] : "Unknown          ");
-	y += sprintf(buffer + len + y, "               ANSI"
-		     " SCSI revision: %02x", (scd->scsi_level - 1) ? scd->scsi_level - 1 : 1);
+	y += sprintf(buffer + len + y, "\n  ANSI SCSI revision: %02x",
+		(scd->scsi_level - 1) ? scd->scsi_level - 1 : 1);
 	if (scd->scsi_level == 2)
 		y += sprintf(buffer + len + y, " CCS\n");
 	else
drivers/scsi/scsi_scan.c +1 -7
diff -ur linux-old/drivers/scsi/scsi_scan.c linux/drivers/scsi/scsi_scan.c
@@ -231,13 +231,7 @@
 			printk(" ");
 	}
 
-	printk("\n");
-
-	i = data[0] & 0x1f;
-
-	printk("  Type:   %s ",
-	       i < MAX_SCSI_DEVICE_CODE ? scsi_device_types[i] : "Unknown          ");
-	printk("                 ANSI SCSI revision: %02x", data[2] & 0x07);
+	printk("\n  ANSI SCSI revision: %02x", data[2] & 0x07);
 	if ((data[2] & 0x07) == 1 && (data[3] & 0x0f) == 1)
 		printk(" CCS\n");
 	else
drivers/scsi/scsi_syms.c +0 -1
diff -ur linux-old/drivers/scsi/scsi_syms.c linux/drivers/scsi/scsi_syms.c
@@ -87,7 +87,6 @@
 EXPORT_SYMBOL(scsi_hostlist);
 EXPORT_SYMBOL(scsi_hosts);
 EXPORT_SYMBOL(scsi_devicelist);
-EXPORT_SYMBOL(scsi_device_types);
 
 /*
  * Externalize timers so that HBAs can safely start/restart commands.
drivers/scsi/sg.c +4 -3
diff -ur linux-old/drivers/scsi/sg.c linux/drivers/scsi/sg.c
@@ -254,7 +254,7 @@
 
 static int sg_open(struct inode * inode, struct file * filp)
 {
-    int dev = minor(inode->i_rdev);
+    unsigned int dev = minor(inode->i_rdev);
     int flags = filp->f_flags;
     Sg_device * sdp;
     Sg_fd * sfp;

@@ -1035,7 +1035,7 @@
 static void sg_cmd_done_bh(Scsi_Cmnd * SCpnt)
 {
     Scsi_Request * SRpnt = SCpnt->sc_request;
-    int dev = minor(SRpnt->sr_request.rq_dev);
+    unsigned int dev = minor(SRpnt->sr_request.rq_dev);
     Sg_device * sdp = NULL;
     Sg_fd * sfp;
     Sg_request * srp = NULL;

@@ -2687,7 +2687,8 @@
 	    Sg_fd * fp;
 	    Sg_request * srp;
 	    struct scsi_device * scsidp;
-	    int dev, k, m, blen, usg;
+	    unsigned int dev;
+	    int k, m, blen, usg;
  
 	    scsidp = sdp->device;
 	    if (NULL == scsidp) {
drivers/scsi/sr.c +32 -26

152 lines not on this page · raw message

drivers/scsi/sr.h +0 -1
diff -ur linux-old/drivers/scsi/sr.h linux/drivers/scsi/sr.h
@@ -30,7 +30,6 @@
 	unsigned use:1;		/* is this device still supportable     */
 	unsigned xa_flag:1;	/* CD has XA sectors ? */
 	unsigned readcd_known:1;	/* drive supports READ_CD (0xbe) */
-	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
 	struct cdrom_device_info cdi;
 } Scsi_CD;
drivers/scsi/sr_ioctl.c +3 -2
diff -ur linux-old/drivers/scsi/sr_ioctl.c linux/drivers/scsi/sr_ioctl.c
@@ -333,7 +333,8 @@
 int sr_audio_ioctl(struct cdrom_device_info *cdi, unsigned int cmd, void *arg)
 {
 	u_char sr_cmd[10];
-	int result, target = minor(cdi->dev);
+	int result;
+	unsigned int target = minor(cdi->dev);
 	unsigned char buffer[32];
 
 	memset(sr_cmd, 0, sizeof(sr_cmd));

@@ -539,7 +540,7 @@
 int sr_dev_ioctl(struct cdrom_device_info *cdi,
 		 unsigned int cmd, unsigned long arg)
 {
-	int target;
+	unsigned int target;
 
 	target = minor(cdi->dev);
drivers/scsi/sr_vendor.c +2 -1
diff -ur linux-old/drivers/scsi/sr_vendor.c linux/drivers/scsi/sr_vendor.c
@@ -156,7 +156,8 @@
 	unsigned long sector;
 	unsigned char *buffer;	/* the buffer for the ioctl */
 	unsigned char cmd[MAX_COMMAND_SIZE];	/* the scsi-command */
-	int rc, no_multi, minor;
+	int rc, no_multi;
+	unsigned int minor;
 
 	minor = minor(cdi->dev);
 	if (scsi_CDs[minor].cdi.mask & CDC_MULTI_SESSION)
drivers/scsi/wd7000.c +0 -2
diff -ur linux-old/drivers/scsi/wd7000.c linux/drivers/scsi/wd7000.c
@@ -1469,8 +1469,6 @@
 	if (scd->host->host_no == hostno) {
 	    SPRINTF ("  [Channel: %02d, Id: %02d, Lun: %02d]  ",
 		     scd->channel, scd->id, scd->lun);
-	    SPRINTF ("%s ", (scd->type < MAX_SCSI_DEVICE_CODE) ?
-		     scsi_device_types[(short) scd->type] : "Unknown device");
 
 	    for (i = 0; (i < 8) && (scd->vendor[i] >= 0x20); i++)
 		SPRINTF ("%c", scd->vendor[i]);
init/do_mounts.c +3 -3
diff -ur linux-old/init/do_mounts.c linux/init/do_mounts.c
@@ -364,6 +364,7 @@
 	return sys_symlink(path + n + 5, name);
 }
 
+#ifdef CONFIG_BLK_DEV_RAM
 static void __init change_floppy(char *fmt, ...)
 {
 	struct termios termios;

@@ -392,8 +393,6 @@
 	}
 }
 
-#ifdef CONFIG_BLK_DEV_RAM
-
 int __initdata rd_prompt = 1;	/* 1 = prompt for RAM disk, 0 = don't prompt */
 
 static int __init prompt_ramdisk(char *str)

@@ -843,6 +842,7 @@
 }
 
 #ifdef BUILD_CRAMDISK
+# ifdef CONFIG_BLK_DEV_RAM
 
 /*
  * gzip declarations

@@ -986,5 +986,5 @@
 	kfree(window);
 	return result;
 }
-
+# endif
 #endif  /* BUILD_CRAMDISK */
#13 lore raw

Re: PATCH 2.5.2-pre9 scsi cleanup

At Tue, 08 Jan 2002 10:29:48 +0100,

Quoted 7 lines · Martin Dalecki

The attached patch does the following.

1. Clean up some ifdef confusion in do_mount

2. Clean up the scsi code to make ppa.c work.

3. Clean up some unneccessary unneeded globals from scsi code.

I don't understand why below removal are needed.
Do you think to replace with function like scsi_device_types()?

Quoted 9 lines
drivers/scsi/scsi.c +0 -9
-const char *const scsi_device_types[MAX_SCSI_DEVICE_CODE] =
-{
-	"Direct-Access    ",
-	"Sequential-Access",
-	"Printer          ",
-	"Processor        ",
-	"WORM             ",
-	"CD-ROM           ",
-	"Scanner          ",

drivers/scsi/scsi.c line 160

Quoted 10 lines
drivers/scsi/scsi.c +0 -10
-	"Optical Device   ",
-	"Medium Changer   ",
-	"Communications   ",
-	"Unknown          ",
-	"Unknown          ",
-	"Unknown          ",
-	"Enclosure        ",
-};
-
-/*

-- gotom