2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Matthias Andree]

On Mon, 15 Mar 2004, Linus Torvalds wrote:

> Here's the current set of patches I've merged from various poeple..
> 
> Merging from Andrew, but also i2c updates, ALSA CVS merge, netconsole,
> prism54 driver merge, sata updates, carmel driver, pcmcia and nfs client
> updates..
> 
> And a few architectures updated: ia64, ppc32, sparc32, arm.
> 
 
> Summary of changes from v2.6.4 to v2.6.5-rc1
> ============================================

...

I have some SCSI troubles with 2.6.5-rc1 (from BK) that 2.6.4 didn't
have.

Modprobe, loading the st driver, tries a NULL pointer dereference in
kernel space and my 2nd tape drive isn't found: st1 is not shown. cat
/proc/scsi/scsi (typed after the attempted zero page dereference) hangs
in rwsem_down_read_failed with process state D.

I've re-built the kernel after doing a "make clean"
but to no avail (as expected).

...
sym0: <875> rev 0x26 at pci 0000:00:0d.0 irq 16
sym0: Tekram NVRAM, ID 7, Fast-20, SE, parity checking
sym0: SCSI BUS has been reset.
scsi0 : sym-2.1.18i
  Vendor: FUJITSU   Model: MAH3182MP         Rev: 0114
  Type:   Direct-Access                      ANSI SCSI revision: 04
sym0:1:0: tagged command queuing enabled, command queue depth 32.
  Vendor: PLEXTOR   Model: CD-ROM PX-32TS    Rev: 1.02
  Type:   CD-ROM                             ANSI SCSI revision: 02
  Vendor: TANDBERG  Model:  TDC 4222         Rev: =07:
  Type:   Sequential-Access                  ANSI SCSI revision: 02
  Vendor: TANDBERG  Model: SLR6              Rev: 0404
  Type:   Sequential-Access                  ANSI SCSI revision: 02
DC390: 0 adapters found
libata version 1.01 loaded.
sym0:1: FAST-20 WIDE SCSI 40.0 MB/s ST (50.0 ns, offset 16)
SCSI device sda: 35701260 512-byte hdwr sectors (18279 MB)
SCSI device sda: drive cache: write through
 sda: sda1 sda2 sda3 < sda5 >
Attached scsi disk sda at scsi0, channel 0, id 1, lun 0
...
Attached scsi generic sg0 at scsi0, channel 0, id 1, lun 0,  type 0
Attached scsi generic sg1 at scsi0, channel 0, id 2, lun 0,  type 5
Attached scsi generic sg2 at scsi0, channel 0, id 6, lun 0,  type 1
Attached scsi generic sg3 at scsi0, channel 0, id 8, lun 0,  type 1
sym0:2: FAST-20 SCSI 20.0 MB/s ST (50.0 ns, offset 15)
sr0: scsi-1 drive
Attached scsi CD-ROM sr0 at scsi0, channel 0, id 2, lun 0
st: Version 20040226, fixed bufsize 32768, s/g segs 256
Unable to handle kernel NULL pointer dereference at virtual address 0000002e
 printing eip:
d1ba00fc
*pde = 00000000
Oops: 0002 [#1]
CPU:    0
EIP:    0060:[<d1ba00fc>]    Not tainted
EFLAGS: 00010282   (2.6.5-rc1) 
EIP is at st_probe+0x7dc/0x99f [st]
eax: cd20024c   ebx: ffffffea   ecx: 00000000   edx: 0000000a
esi: 00000004   edi: 00000000   ebp: cd10bdbc   esp: cd10bd34
ds: 007b   es: 007b   ss: 0068
Process modprobe (pid: 1303, threadinfo=cd10a000 task=cd148d80)
Stack: cd5f9cc0 00900000 cfa3597c d1ba0628 cd70c6c4 00000000 cd10bd68 cd10bd68 
       00000000 cd20024c 00000000 cd20024c cc623c80 cd10bd84 00000000 cd200200 
       cd9d2980 00000008 cd88b000 00000000 d1ba2bf0 00000000 00000000 cd20024c 
Call Trace:
 [<c027eaeb>] bus_match+0x3b/0x70
 [<c027ed00>] driver_attach+0x50/0x90
 [<c027eddb>] bus_add_driver+0x9b/0xc0
 [<c027f3c1>] driver_register+0x31/0x40
 [<d19e50b6>] init_st+0xb6/0x155 [st]
 [<c0131200>] sys_init_module+0x130/0x1670
 [<c01bd5c1>] journal_stop+0x171/0x210
 [<c01af4b7>] ext3_mark_inode_dirty+0x47/0x60
 [<c0157110>] cdev_alloc+0x0/0x60
 [<c0144c55>] do_mmap_pgoff+0x365/0x6c0
 [<c014cda7>] filp_close+0x57/0x90
 [<c014ce2f>] sys_close+0x4f/0x60
 [<c01070ff>] syscall_call+0x7/0xb

Code: 89 43 44 89 1c 24 c7 44 24 04 5c 2a ba d1 e8 f1 fb 6d ee c7 
 <6>sym0:6: FAST-5 SCSI 4.8 MB/s ST (208.0 ns, offset 8)
st0: Block limits 1 - 16777215 bytes.
...

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Matthew Wilcox]

On Tue, Mar 16, 2004 at 10:12:03PM +0100, Matthias Andree wrote:
> I have some SCSI troubles with 2.6.5-rc1 (from BK) that 2.6.4 didn't
> have.
> 
> Modprobe, loading the st driver, tries a NULL pointer dereference in
> kernel space and my 2nd tape drive isn't found: st1 is not shown. cat
> /proc/scsi/scsi (typed after the attempted zero page dereference) hangs
> in rwsem_down_read_failed with process state D.

I notice you're using the sym2 driver.  Could you try backing out the
changes made to it in 2.6.5-rc1, just to be sure we're looking at an st
problem, not a sym2 problem?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Matthias Andree]

On Tue, 16 Mar 2004, Matthew Wilcox wrote:

> On Tue, Mar 16, 2004 at 10:12:03PM +0100, Matthias Andree wrote:
> > I have some SCSI troubles with 2.6.5-rc1 (from BK) that 2.6.4 didn't
> > have.
> > 
> > Modprobe, loading the st driver, tries a NULL pointer dereference in
> > kernel space and my 2nd tape drive isn't found: st1 is not shown. cat
> > /proc/scsi/scsi (typed after the attempted zero page dereference) hangs
> > in rwsem_down_read_failed with process state D.
> 
> I notice you're using the sym2 driver.  Could you try backing out the
> changes made to it in 2.6.5-rc1, just to be sure we're looking at an st
> problem, not a sym2 problem?

I've backed out the 2004-03-12 ChangeSet,
willy@debian.org|ChangeSet|20040312212827|57687

and rebooted, result (does that make sense? Was that a complete
back-out?):

...
sym0: <875> rev 0x26 at pci 0000:00:0d.0 irq 16
sym0: Tekram NVRAM, ID 7, Fast-20, SE, parity checking
sym0: SCSI BUS has been reset.
scsi0 : sym-2.1.18f
  Vendor: FUJITSU   Model: MAH3182MP         Rev: 0114
  Type:   Direct-Access                      ANSI SCSI revision: 04
sym0:1:0: tagged command queuing enabled, command queue depth 32.
  Vendor: PLEXTOR   Model: CD-ROM PX-32TS    Rev: 1.02
  Type:   CD-ROM                             ANSI SCSI revision: 02
  Vendor: TANDBERG  Model:  TDC 4222         Rev: =07:
  Type:   Sequential-Access                  ANSI SCSI revision: 02
  Vendor: TANDBERG  Model: SLR6              Rev: 0404
  Type:   Sequential-Access                  ANSI SCSI revision: 02
DC390: 0 adapters found
libata version 1.01 loaded.
sym0:1: FAST-20 WIDE SCSI 40.0 MB/s ST (50.0 ns, offset 16)
SCSI device sda: 35701260 512-byte hdwr sectors (18279 MB)
SCSI device sda: drive cache: write through
 sda: sda1 sda2 sda3 < sda5 >
Attached scsi disk sda at scsi0, channel 0, id 1, lun 0
...
Attached scsi generic sg0 at scsi0, channel 0, id 1, lun 0,  type 0
Attached scsi generic sg1 at scsi0, channel 0, id 2, lun 0,  type 5
Attached scsi generic sg2 at scsi0, channel 0, id 6, lun 0,  type 1
Attached scsi generic sg3 at scsi0, channel 0, id 8, lun 0,  type 1
sym0:2: FAST-20 SCSI 20.0 MB/s ST (50.0 ns, offset 15)
sr0: scsi-1 drive
Attached scsi CD-ROM sr0 at scsi0, channel 0, id 2, lun 0
st: Version 20040226, fixed bufsize 32768, s/g segs 256
Unable to handle kernel NULL pointer dereference at virtual address 0000002e
 printing eip:
d1ba00fc
*pde = 00000000
Oops: 0002 [#1]
CPU:    0
EIP:    0060:[<d1ba00fc>]    Not tainted
EFLAGS: 00010282   (2.6.5-rc1) 
EIP is at st_probe+0x7dc/0x99f [st]
eax: cd4d824c   ebx: ffffffea   ecx: 00000000   edx: 0000000a
esi: 00000004   edi: 00000000   ebp: c135fdbc   esp: c135fd34
ds: 007b   es: 007b   ss: 0068
Process modprobe (pid: 1304, threadinfo=c135e000 task=c1384620)
Stack: ccab5cc0 00900000 cfa3597c d1ba0628 ccbaf6c4 00000000 c135fd68 c135fd68 
       00000000 cd4d824c 00000000 cd4d824c cc6d0c80 c135fd84 00000000 cd4d8200 
       cd979980 00000008 cdb39000 00000000 d1ba2bf0 00000000 00000000 cd4d824c 
Call Trace:
 [<c027eaeb>] bus_match+0x3b/0x70
 [<c027ed00>] driver_attach+0x50/0x90
 [<c027eddb>] bus_add_driver+0x9b/0xc0
 [<c027f3c1>] driver_register+0x31/0x40
 [<d19e50b6>] init_st+0xb6/0x155 [st]
 [<c0131200>] sys_init_module+0x130/0x1670
 [<c01bd5c1>] journal_stop+0x171/0x210
 [<c01af4b7>] ext3_mark_inode_dirty+0x47/0x60
 [<c0157110>] cdev_alloc+0x0/0x60
 [<c0144c55>] do_mmap_pgoff+0x365/0x6c0
 [<c014cda7>] filp_close+0x57/0x90
 [<c014ce2f>] sys_close+0x4f/0x60
 [<c01070ff>] syscall_call+0x7/0xb

Code: 89 43 44 89 1c 24 c7 44 24 04 5c 2a ba d1 e8 f1 fb 6d ee c7 
 <6>sym0:6: FAST-5 SCSI 4.8 MB/s ST (208.0 ns, offset 8)
st0: Block limits 1 - 16777215 bytes.

-- 
Matthias Andree

Encrypt your mail: my GnuPG key ID is 0x052E7D95

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Matthew Wilcox]

On Tue, Mar 16, 2004 at 10:56:59PM +0100, Matthias Andree wrote:
> I've backed out the 2004-03-12 ChangeSet,
> willy@debian.org|ChangeSet|20040312212827|57687
> 
> and rebooted, result (does that make sense? Was that a complete
> back-out?):
[...]
> Attached scsi CD-ROM sr0 at scsi0, channel 0, id 2, lun 0
> st: Version 20040226, fixed bufsize 32768, s/g segs 256
> Unable to handle kernel NULL pointer dereference at virtual address 0000002e
>  printing eip:
> d1ba00fc
> *pde = 00000000
> Oops: 0002 [#1]

OK, that confirms it's not the sym2 changes that were the problem.
Thanks for testing!

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Kai Makisara]

On Tue, 16 Mar 2004, Matthias Andree wrote:

> On Tue, 16 Mar 2004, Matthew Wilcox wrote:
> 
> > On Tue, Mar 16, 2004 at 10:12:03PM +0100, Matthias Andree wrote:
> > > I have some SCSI troubles with 2.6.5-rc1 (from BK) that 2.6.4 didn't
> > > have.
> > > 
> > > Modprobe, loading the st driver, tries a NULL pointer dereference in
> > > kernel space and my 2nd tape drive isn't found: st1 is not shown. cat
> > > /proc/scsi/scsi (typed after the attempted zero page dereference) hangs
> > > in rwsem_down_read_failed with process state D.
> > 
> > I notice you're using the sym2 driver.  Could you try backing out the
> > changes made to it in 2.6.5-rc1, just to be sure we're looking at an st
> > problem, not a sym2 problem?
> 
I was able to reproduce this with the scsi_debug driver (you can use it 
for simple tape tests by changing TYPE_DISK to TYPE_TAPE). Reverting the 
patch:

# ChangeSet
#   2004/03/12 16:22:36-08:00 greg@kroah.com 
#   remove cdev_set_name completely as it is not needed.

(and editing drivers/char/tty_io.c to get the kernel to compile) solved 
the problem for me. st.c is using the name put into kobj.name in making 
the class file names. I will make a patch that removes this dependency.

While looking at this problem, I noticed that the naming changes already 
committed to BK had disappeared. Looking at st.c history revealed that the 
following change had been committed (sorry for wrapping) by greg@kroah.com 
46 hours ago:

--- 1.80+1.79.1.2/drivers/scsi/st.c	Wed Mar 17 12:25:28 2004
+++ 1.81/drivers/scsi/st.c	Wed Mar 17 12:25:28 2004
@@ -3896,11 +3896,6 @@
 				       dev_num);
 				goto out_free_tape;
 			}
-			/* Make sure that the minor numbers corresponding 
to the four
-			   first modes always get the same names */
-			i = mode << (4 - ST_NBR_MODE_BITS);
-			snprintf(cdev->kobj.name, KOBJ_NAME_LEN, "%s%s%s", 
j ? "n" : "",
-				 disk->disk_name, st_formats[i]);
 			cdev->owner = THIS_MODULE;
 			cdev->ops = &st_fops;

The change comment was "merge" and it resulted in st.c version 1.81. (I am 
not using Bitkeeper but trying to extract information from bkbits.net.)

	Kai

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Kai Makisara]

On Wed, 17 Mar 2004, Kai Makisara wrote:

> On Tue, 16 Mar 2004, Matthias Andree wrote:
> 
> > On Tue, 16 Mar 2004, Matthew Wilcox wrote:
> > 
> > > On Tue, Mar 16, 2004 at 10:12:03PM +0100, Matthias Andree wrote:
> > > > I have some SCSI troubles with 2.6.5-rc1 (from BK) that 2.6.4 didn't
> > > > have.
> > > > 
> > > > Modprobe, loading the st driver, tries a NULL pointer dereference in
> > > > kernel space and my 2nd tape drive isn't found: st1 is not shown. cat
> > > > /proc/scsi/scsi (typed after the attempted zero page dereference) hangs
> > > > in rwsem_down_read_failed with process state D.
> > > 
> > > I notice you're using the sym2 driver.  Could you try backing out the
> > > changes made to it in 2.6.5-rc1, just to be sure we're looking at an st
> > > problem, not a sym2 problem?
> > 
...
> st.c is using the name put into kobj.name in making 
> the class file names. I will make a patch that removes this dependency.
> 
The patch at the end of this message removes the dependency on the kobj 
name being set. It also tries to once more restore the naming that is 
defined in devices.txt. The patch is against 2.6.5-rc1-bk2 and it seems to 
work correctly in my tests.

-- 
Kai
------------------------------8<-------------------------------------------
--- linux-2.6.5-rc1-bk2/drivers/scsi/st.c	2004-03-17 22:37:11.000000000 +0200
+++ linux-2.6.5-rc1-bk2-k1/drivers/scsi/st.c	2004-03-17 23:03:57.000000000 +0200
@@ -17,7 +17,7 @@
    Last modified: 18-JAN-1998 Richard Gooch <rgooch@atnf.csiro.au> Devfs support
  */
 
-static char *verstr = "20040226";
+static char *verstr = "20040317";
 
 #include <linux/module.h>
 
@@ -4193,19 +4193,24 @@
 
 static void do_create_class_files(Scsi_Tape *STp, int dev_num, int mode)
 {
-	int rew, error;
+	int i, rew, error;
+	char name[10];
 	struct class_device *st_class_member;
 
 	if (!st_sysfs_class)
 		return;
 
 	for (rew=0; rew < 2; rew++) {
+		/* Make sure that the minor numbers corresponding to the four
+		   first modes always get the same names */
+		i = mode << (4 - ST_NBR_MODE_BITS);
+		snprintf(name, 10, "%s%s%s", rew ? "n" : "",
+			 STp->disk->disk_name, st_formats[i]);
 		st_class_member =
 			class_simple_device_add(st_sysfs_class,
 						MKDEV(SCSI_TAPE_MAJOR,
 						      TAPE_MINOR(dev_num, mode, rew)),
-						&STp->device->sdev_gendev, "%s",
-						STp->modes[mode].cdevs[rew]->kobj.name);
+						&STp->device->sdev_gendev, "%s", name);
 		if (!st_class_member) {
 			printk(KERN_WARNING "st%d: class_simple_device_add failed\n",
 			       dev_num);

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Matthias Andree]

On Wed, 17 Mar 2004, Kai Makisara wrote:

> # ChangeSet
> #   2004/03/12 16:22:36-08:00 greg@kroah.com 
> #   remove cdev_set_name completely as it is not needed.
> 
> (and editing drivers/char/tty_io.c to get the kernel to compile) solved 
> the problem for me. st.c is using the name put into kobj.name in making 
> the class file names. I will make a patch that removes this dependency.

I had backed out that ChangeSet here as I was suspecting it (about the
only one "new enough" to cause these problems) but bumped into tty_io.c
compile failures and didn't have time to investigate.

> While looking at this problem, I noticed that the naming changes already 
> committed to BK had disappeared. Looking at st.c history revealed that the 
> following change had been committed (sorry for wrapping) by greg@kroah.com 
> 46 hours ago:

This is how the change tree looks like in BitKeeper's histtool,
the top line is the date in MM-DD format:

02-22  02-24   03-03     03-13        03-13           03-15
kai  -> axbø -> kai ------------------------------->  greg
1.78    1.79\   1.80                                  1.81
             \                                   /
              `----------corbet-------greg------'
                         1.79.1.1     1.79.1.2

...

Logs:

======== st.c 1.1..1.81 ========
D 1.81 04/03/15 15:02:26-08:00 greg@kroah.com 104 101 0/5/4363
P drivers/scsi/st.c
C merge
------------------------------------------------
D 1.79.1.2 04/03/12 08:22:11-08:00 greg@kroah.com[greg] 103 102 0/1/4350
P drivers/scsi/st.c
C remove cdev_set_name completely as it is not needed.
------------------------------------------------
D 1.79.1.1 04/03/13 00:47:18-08:00 corbet@lwn.net[greg] 102 100 2/3/4349
P drivers/scsi/st.c
C cdev 2/2: hide cdev->kobj
------------------------------------------------
D 1.80 04/02/26 05:24:19-06:00 Kai.Makisara@kolumbus.fi[jejb] 101 100 28/12/4340
P drivers/scsi/st.c
C SCSI tape sysfs name fixes
------------------------------------------------
D 1.79 04/02/23 06:23:46-08:00 axboe@suse.de[torvalds] 100 99 1/1/4351
P drivers/scsi/st.c
C fix SCSI non-sector bio backed IO

> The change comment was "merge" and it resulted in st.c version 1.81. (I am 
> not using Bitkeeper but trying to extract information from bkbits.net.)

BitKeeper doesn't have more info and this doesn't look like an auto-generated
comment of BitKeeper's, but seems to have been entered manually. Blame Greg?

-- 
Matthias Andree

Encrypt your mail: my GnuPG key ID is 0x052E7D95
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Matthias Andree]

On Wed, 17 Mar 2004, Kai Makisara wrote:

> The patch at the end of this message removes the dependency on the kobj 
> name being set. It also tries to once more restore the naming that is 
> defined in devices.txt. The patch is against 2.6.5-rc1-bk2 and it seems to 
> work correctly in my tests.

Your patch fixes my problem. Thank you.

st0* and st1* devices are registered, the devices are properly attached,
mtst -f /dev/nst1 status works (same for nst0).

-- 
Matthias Andree

Encrypt your mail: my GnuPG key ID is 0x052E7D95

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Mike Anderson]

Kai Makisara [Kai.Makisara@kolumbus.fi] wrote:
>  		if (!st_class_member) {
>  			printk(KERN_WARNING "st%d: class_simple_device_add failed\n",
>  			       dev_num);

Could you change the if check to use IS_ERR(st_class_member) so in the
future if do_create_class_files return -E* we will not get a oops.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Kai Makisara]

On Wed, 17 Mar 2004, Mike Anderson wrote:

> Kai Makisara [Kai.Makisara@kolumbus.fi] wrote:
> >  		if (!st_class_member) {
> >  			printk(KERN_WARNING "st%d: class_simple_device_add failed\n",
> >  			       dev_num);
> 
> Could you change the if check to use IS_ERR(st_class_member) so in the
> future if do_create_class_files return -E* we will not get a oops.
> 
A revised patch is at the end of this message. Thanks for pointing out 
this bug.

-- 
Kai
--------------------------------8<----------------------------------------------
--- linux-2.6.5-rc1-bk2/drivers/scsi/st.c	2004-03-17 22:37:11.000000000 +0200
+++ linux-2.6.5-rc1-bk2-k1/drivers/scsi/st.c	2004-03-18 00:09:07.000000000 +0200
@@ -17,7 +17,7 @@
    Last modified: 18-JAN-1998 Richard Gooch <rgooch@atnf.csiro.au> Devfs support
  */
 
-static char *verstr = "20040226";
+static char *verstr = "20040318";
 
 #include <linux/module.h>
 
@@ -4193,20 +4193,25 @@
 
 static void do_create_class_files(Scsi_Tape *STp, int dev_num, int mode)
 {
-	int rew, error;
+	int i, rew, error;
+	char name[10];
 	struct class_device *st_class_member;
 
 	if (!st_sysfs_class)
 		return;
 
 	for (rew=0; rew < 2; rew++) {
+		/* Make sure that the minor numbers corresponding to the four
+		   first modes always get the same names */
+		i = mode << (4 - ST_NBR_MODE_BITS);
+		snprintf(name, 10, "%s%s%s", rew ? "n" : "",
+			 STp->disk->disk_name, st_formats[i]);
 		st_class_member =
 			class_simple_device_add(st_sysfs_class,
 						MKDEV(SCSI_TAPE_MAJOR,
 						      TAPE_MINOR(dev_num, mode, rew)),
-						&STp->device->sdev_gendev, "%s",
-						STp->modes[mode].cdevs[rew]->kobj.name);
-		if (!st_class_member) {
+						&STp->device->sdev_gendev, "%s", name);
+		if (IS_ERR(st_class_member)) {
 			printk(KERN_WARNING "st%d: class_simple_device_add failed\n",
 			       dev_num);
 			goto out;

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Greg KH]

On Thu, Mar 18, 2004 at 12:25:08AM +0200, Kai Makisara wrote:
> On Wed, 17 Mar 2004, Mike Anderson wrote:
> 
> > Kai Makisara [Kai.Makisara@kolumbus.fi] wrote:
> > >  		if (!st_class_member) {
> > >  			printk(KERN_WARNING "st%d: class_simple_device_add failed\n",
> > >  			       dev_num);
> > 
> > Could you change the if check to use IS_ERR(st_class_member) so in the
> > future if do_create_class_files return -E* we will not get a oops.
> > 
> A revised patch is at the end of this message. Thanks for pointing out 
> this bug.

Yeah, this is a much better fix.  Sorry to cause all of this trouble, I
should have checked to see if anyone used the kobject's name of the cdev
structure anywhere before taking that assignement out.

Andrew, this is a better fix than the one that you and Jon came up with
earlier today.

thanks,

greg k-h

> --------------------------------8<----------------------------------------------
> --- linux-2.6.5-rc1-bk2/drivers/scsi/st.c	2004-03-17 22:37:11.000000000 +0200
> +++ linux-2.6.5-rc1-bk2-k1/drivers/scsi/st.c	2004-03-18 00:09:07.000000000 +0200
> @@ -17,7 +17,7 @@
>     Last modified: 18-JAN-1998 Richard Gooch <rgooch@atnf.csiro.au> Devfs support
>   */
>  
> -static char *verstr = "20040226";
> +static char *verstr = "20040318";
>  
>  #include <linux/module.h>
>  
> @@ -4193,20 +4193,25 @@
>  
>  static void do_create_class_files(Scsi_Tape *STp, int dev_num, int mode)
>  {
> -	int rew, error;
> +	int i, rew, error;
> +	char name[10];
>  	struct class_device *st_class_member;
>  
>  	if (!st_sysfs_class)
>  		return;
>  
>  	for (rew=0; rew < 2; rew++) {
> +		/* Make sure that the minor numbers corresponding to the four
> +		   first modes always get the same names */
> +		i = mode << (4 - ST_NBR_MODE_BITS);
> +		snprintf(name, 10, "%s%s%s", rew ? "n" : "",
> +			 STp->disk->disk_name, st_formats[i]);
>  		st_class_member =
>  			class_simple_device_add(st_sysfs_class,
>  						MKDEV(SCSI_TAPE_MAJOR,
>  						      TAPE_MINOR(dev_num, mode, rew)),
> -						&STp->device->sdev_gendev, "%s",
> -						STp->modes[mode].cdevs[rew]->kobj.name);
> -		if (!st_class_member) {
> +						&STp->device->sdev_gendev, "%s", name);
> +		if (IS_ERR(st_class_member)) {
>  			printk(KERN_WARNING "st%d: class_simple_device_add failed\n",
>  			       dev_num);
>  			goto out;

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Mike Anderson]

Kai Makisara [Kai.Makisara@kolumbus.fi] wrote:
> On Wed, 17 Mar 2004, Mike Anderson wrote:
> A revised patch is at the end of this message. Thanks for pointing out 
> this bug.

Thanks for the update. I checked it out on my system with a aic and a
TDC 3800 tape drive. It works fine.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: 2.6.5-rc1 SCSI + st regressions (was: Linux 2.6.5-rc1)

[Andy Isaacson]

On Wed, Mar 17, 2004 at 10:32:01PM +0100, Matthias Andree wrote:
> On Wed, 17 Mar 2004, Kai Makisara wrote:
> > # ChangeSet
> > #   2004/03/12 16:22:36-08:00 greg@kroah.com 
> > #   remove cdev_set_name completely as it is not needed.
> > 
> > (and editing drivers/char/tty_io.c to get the kernel to compile) solved 
> > the problem for me. st.c is using the name put into kobj.name in making 
> > the class file names. I will make a patch that removes this dependency.
> 
> I had backed out that ChangeSet here as I was suspecting it (about the
> only one "new enough" to cause these problems) but bumped into tty_io.c
> compile failures and didn't have time to investigate.
> 
> > While looking at this problem, I noticed that the naming changes already 
> > committed to BK had disappeared. Looking at st.c history revealed that the 
> > following change had been committed (sorry for wrapping) by greg@kroah.com 
> > 46 hours ago:
> 
> This is how the change tree looks like in BitKeeper's histtool,
> the top line is the date in MM-DD format:
> 
> 02-22  02-24   03-03     03-13        03-13           03-15
> kai  -> axbø -> kai ------------------------------->  greg
> 1.78    1.79\   1.80                                  1.81
>              \                                   /
>               `----------corbet-------greg------'
>                          1.79.1.1     1.79.1.2
> 
> ...
> 
> Logs:
> 
> ======== st.c 1.1..1.81 ========
> D 1.81 04/03/15 15:02:26-08:00 greg@kroah.com 104 101 0/5/4363
> P drivers/scsi/st.c
> C merge
> ------------------------------------------------
> D 1.79.1.2 04/03/12 08:22:11-08:00 greg@kroah.com[greg] 103 102 0/1/4350
> P drivers/scsi/st.c
> C remove cdev_set_name completely as it is not needed.
> ------------------------------------------------
> D 1.79.1.1 04/03/13 00:47:18-08:00 corbet@lwn.net[greg] 102 100 2/3/4349
> P drivers/scsi/st.c
> C cdev 2/2: hide cdev->kobj
> ------------------------------------------------
> D 1.80 04/02/26 05:24:19-06:00 Kai.Makisara@kolumbus.fi[jejb] 101 100 28/12/4340
> P drivers/scsi/st.c
> C SCSI tape sysfs name fixes
> ------------------------------------------------
> D 1.79 04/02/23 06:23:46-08:00 axboe@suse.de[torvalds] 100 99 1/1/4351
> P drivers/scsi/st.c
> C fix SCSI non-sector bio backed IO
> 
> > The change comment was "merge" and it resulted in st.c version 1.81. (I am 
> > not using Bitkeeper but trying to extract information from bkbits.net.)
> 
> BitKeeper doesn't have more info and this doesn't look like an auto-generated
> comment of BitKeeper's, but seems to have been entered manually. Blame Greg?

That's a pretty nasty merge, so it's not real suprising that Greg didn't
get it completely right.

Nice ASCII art, BTW.  If you can reduce it to an algorithm I bet Larry
would like to hire you to add it to BK. :)

-andy
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html