Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Andrew Morton]

James said:

: OK ... I looked ... and it's not pretty.  The problem is that the 3ware
: assumes certain commands (INQUIRY, READ_CAPACITY, MODE_SENSE + a few
: others) are only generated internally and thus only have use_sg == 0.
: The scsi-block-tree breaks this assumption because we're trying to
: eliminate the use_sg == 0 special case.
: 
: It's important to note that this behaviour is already broken, as anyone
: using SG_IO to send commands to the device would have discovered.
: 
: Could you send this bug report to the scsi list and cc adam radford
: <aradford@amcc.com>.  The solution is probably to convert this driver to
: using the libata style command emulation.

Begin forwarded message:

Date: Sun, 26 Jun 2005 14:14:09 -0400
From: Nick Orlov <bugfixer@list.ru>
To: linux-kernel@vger.kernel.org
Cc: akpm@osdl.org
Subject: 2.6.12-mm2: 3ware SATA RAID inaccessible


3Ware SATA RAID inaccessible with 2.6.12-mm2:

==================================================

Jun 26 11:49:51 nikolas kernel: 3ware Storage Controller device driver for Linux v1.26.02.001.
Jun 26 11:49:51 nikolas kernel: PCI: Found IRQ 9 for device 0000:00:0f.0
Jun 26 11:49:51 nikolas kernel: IRQ routing conflict for 0000:00:0f.0, have irq 5, want irq 9
Jun 26 11:49:51 nikolas kernel: scsi0 : 3ware Storage Controller
Jun 26 11:49:51 nikolas kernel: 3w-xxxx: scsi0: Found a 3ware Storage Controller at 0xa400, IRQ: 5.
Jun 26 11:49:51 nikolas kernel:   Vendor:           Model:                   Rev:     
Jun 26 11:49:51 nikolas kernel:   Type:   Direct-Access                      ANSI SCSI revision: 00
Jun 26 11:49:51 nikolas kernel: sda : sector size 0 reported, assuming 512.
Jun 26 11:49:51 nikolas kernel: SCSI device sda: 1 512-byte hdwr sectors (0 MB)
Jun 26 11:49:51 nikolas kernel: sda: asking for cache data failed
Jun 26 11:49:51 nikolas kernel: sda: assuming drive cache: write through
Jun 26 11:49:51 nikolas kernel: sda : sector size 0 reported, assuming 512.
Jun 26 11:49:51 nikolas kernel: SCSI device sda: 1 512-byte hdwr sectors (0 MB)
Jun 26 11:49:51 nikolas kernel: sda: asking for cache data failed
Jun 26 11:49:51 nikolas kernel: sda: assuming drive cache: write through
Jun 26 11:49:51 nikolas kernel:  sda: sda1 sda2 sda3
Jun 26 11:49:51 nikolas kernel: Attached scsi disk sda at scsi0, channel 0, id 0, lun 0

..........

Jun 26 11:49:51 nikolas kernel: ReiserFS: sda2: warning: sh-2006: read_super_block: bread failed (dev sda2, block 2, size 4096)
Jun 26 11:49:51 nikolas kernel: ReiserFS: sda2: warning: sh-2006: read_super_block: bread failed (dev sda2, block 16, size 4096)
Jun 26 11:49:51 nikolas kernel: ReiserFS: sda2: warning: sh-2021: reiserfs_fill_super: can not find reiserfs on sda2
Jun 26 11:49:51 nikolas kernel: ReiserFS: sda3: warning: sh-2006: read_super_block: bread failed (dev sda3, block 2, size 4096)
Jun 26 11:49:51 nikolas kernel: ReiserFS: sda3: warning: sh-2006: read_super_block: bread failed (dev sda3, block 16, size 4096)
Jun 26 11:49:51 nikolas kernel: ReiserFS: sda3: warning: sh-2021: reiserfs_fill_super: can not find reiserfs on sda3

==================================================


2.6.12-mm1 works just fine:

==================================================


Jun 26 11:59:07 nikolas kernel: 3ware Storage Controller device driver for Linux v1.26.02.001.
Jun 26 11:59:07 nikolas kernel: PCI: Found IRQ 9 for device 0000:00:0f.0
Jun 26 11:59:07 nikolas kernel: IRQ routing conflict for 0000:00:0f.0, have irq 5, want irq 9
Jun 26 11:59:07 nikolas kernel: scsi0 : 3ware Storage Controller
Jun 26 11:59:07 nikolas kernel: 3w-xxxx: scsi0: Found a 3ware Storage Controller at 0xa400, IRQ: 5.
Jun 26 11:59:07 nikolas kernel:   Vendor: 3ware     Model: Logical Disk 0    Rev: 1.2 
Jun 26 11:59:07 nikolas kernel:   Type:   Direct-Access                      ANSI SCSI revision: 00
Jun 26 11:59:07 nikolas kernel: SCSI device sda: 488395120 512-byte hdwr sectors (250058 MB)
Jun 26 11:59:07 nikolas kernel: SCSI device sda: drive cache: write back
Jun 26 11:59:07 nikolas kernel: SCSI device sda: 488395120 512-byte hdwr sectors (250058 MB)
Jun 26 11:59:07 nikolas kernel: SCSI device sda: drive cache: write back
Jun 26 11:59:07 nikolas kernel:  sda: sda1 sda2 sda3
Jun 26 11:59:07 nikolas kernel: Attached scsi disk sda at scsi0, channel 0, id 0, lun 0

..........


Jun 26 11:59:07 nikolas kernel: ReiserFS: sda2: found reiserfs format "3.6" with standard journal
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda2: using ordered data mode
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda2: journal params: device sda2, size 8192, journal first block 18, max trans len 1024, max batch 900, max commit age 30, max trans age 30
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda2: checking transaction log (sda2)
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda2: Using r5 hash to sort names
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda3: found reiserfs format "3.6" with standard journal
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda3: using ordered data mode
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda3: journal params: device sda3, size 8192, journal first block 18, max trans len 1024, max batch 900, max commit age 30, max trans age 30
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda3: checking transaction log (sda3)
Jun 26 11:59:07 nikolas kernel: ReiserFS: sda3: Using r5 hash to sort names

==================================================

reverting git-scsi-block-fix.patch git-scsi-block.patch solves the
problem.


Also, please note that some of the info printed twice:

Jun 26 11:59:07 nikolas kernel: SCSI device sda: 488395120 512-byte hdwr
sectors (250058 MB)
Jun 26 11:59:07 nikolas kernel: SCSI device sda: drive cache: write back
Jun 26 11:59:07 nikolas kernel: SCSI device sda: 488395120 512-byte hdwr
sectors (250058 MB)
Jun 26 11:59:07 nikolas kernel: SCSI device sda: drive cache: write back

This glitch was introduced long time ago, somewhere after 2.6.9-mm1.
I have an archive of all the kernels I've built, so if it'll help I can do
a binary search among them to find when exactly it happend.

-- 
With best wishes,
	Nick Orlov.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[James Bottomley]

On Sun, 2005-06-26 at 15:39 -0700, Andrew Morton wrote:
> : OK ... I looked ... and it's not pretty.  The problem is that the 3ware
> : assumes certain commands (INQUIRY, READ_CAPACITY, MODE_SENSE + a few
> : others) are only generated internally and thus only have use_sg == 0.
> : The scsi-block-tree breaks this assumption because we're trying to
> : eliminate the use_sg == 0 special case.
> : 
> : It's important to note that this behaviour is already broken, as anyone
> : using SG_IO to send commands to the device would have discovered.

OK, I think the attached is the fix.  It makes the 3w-xxxx routines
handle sg data.  Can we verify it works first, and then we'll worry
about how many other internal command processing devices are broken in
this way.

Thanks,

James

diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -1499,22 +1499,53 @@ static int tw_scsiop_inquiry(TW_Device_E
 	return 0;
 } /* End tw_scsiop_inquiry() */
 
+static void *tw_map_internal(TW_Device_Extension *tw_dev, int request_id,
+			     int *len)
+{
+	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
+	void *buf;
+
+	if (cmd->use_sg) {
+		struct scatterlist *sg =
+			(struct scatterlist *)cmd->request_buffer;
+		buf = kmap_atomic(sg->page, KM_USER0) + sg->offset;
+		*len = sg->length;
+	} else {
+		buf = cmd->request_buffer;
+		*len = cmd->request_bufflen;
+	}
+	return buf;
+}
+
+static void tw_unmap_internal(TW_Device_Extension *tw_dev, int request_id)
+{
+	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
+
+	if (cmd->use_sg) {
+		struct scatterlist *sg;
+
+		sg = (struct scatterlist *)cmd->request_buffer;
+		kunmap_atomic(sg->page, KM_USER0);
+	}
+}
+
 /* This function is called by the isr to complete an inquiry command */
 static int tw_scsiop_inquiry_complete(TW_Device_Extension *tw_dev, int request_id)
 {
 	unsigned char *is_unit_present;
 	unsigned char *request_buffer;
+	int len;
 	TW_Param *param;
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_inquiry_complete()\n");
 
 	/* Fill request buffer */
-	if (tw_dev->srb[request_id]->request_buffer == NULL) {
+	request_buffer = tw_map_internal(tw_dev, request_id, &len);
+	if (request_buffer == NULL || len < 36) {
 		printk(KERN_WARNING "3w-xxxx: tw_scsiop_inquiry_complete(): Request buffer NULL.\n");
 		return 1;
 	}
-	request_buffer = tw_dev->srb[request_id]->request_buffer;
-	memset(request_buffer, 0, tw_dev->srb[request_id]->request_bufflen);
+	memset(request_buffer, 0, len);
 	request_buffer[0] = TYPE_DISK; /* Peripheral device type */
 	request_buffer[1] = 0;	       /* Device type modifier */
 	request_buffer[2] = 0;	       /* No ansi/iso compliance */
@@ -1522,6 +1553,7 @@ static int tw_scsiop_inquiry_complete(TW
 	memcpy(&request_buffer[8], "3ware   ", 8);	 /* Vendor ID */
 	sprintf(&request_buffer[16], "Logical Disk %-2d ", tw_dev->srb[request_id]->device->id);
 	memcpy(&request_buffer[32], TW_DRIVER_VERSION, 3);
+	tw_unmap_internal(tw_dev, request_id);
 
 	param = (TW_Param *)tw_dev->alignment_virtual_address[request_id];
 	if (param == NULL) {
@@ -1613,6 +1645,7 @@ static int tw_scsiop_mode_sense_complete
 	TW_Param *param;
 	unsigned char *flags;
 	unsigned char *request_buffer;
+	int len;
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_mode_sense_complete()\n");
 
@@ -1622,8 +1655,9 @@ static int tw_scsiop_mode_sense_complete
 		return 1;
 	}
 	flags = (char *)&(param->data[0]);
-	request_buffer = tw_dev->srb[request_id]->buffer;
-	memset(request_buffer, 0, tw_dev->srb[request_id]->request_bufflen);
+	request_buffer = tw_map_internal(tw_dev, request_id, &len);
+	memset(request_buffer, 0, len);
+	BUG_ON(len < 7);
 
 	request_buffer[0] = 0xf;        /* mode data length */
 	request_buffer[1] = 0;          /* default medium type */
@@ -1635,6 +1669,7 @@ static int tw_scsiop_mode_sense_complete
 		request_buffer[6] = 0x4;        /* WCE on */
 	else
 		request_buffer[6] = 0x0;        /* WCE off */
+	tw_unmap_internal(tw_dev, request_id);
 
 	return 0;
 } /* End tw_scsiop_mode_sense_complete() */
@@ -1703,15 +1738,16 @@ static int tw_scsiop_read_capacity_compl
 	u32 capacity;
 	char *buff;
 	TW_Param *param;
+	int len;
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_read_capacity_complete()\n");
 
-	buff = tw_dev->srb[request_id]->request_buffer;
-	if (buff == NULL) {
+	buff = tw_map_internal(tw_dev, request_id, &len);
+	if (buff == NULL || len < 8) {
 		printk(KERN_WARNING "3w-xxxx: tw_scsiop_read_capacity_complete(): Request buffer NULL.\n");
 		return 1;
 	}
-	memset(buff, 0, tw_dev->srb[request_id]->request_bufflen);
+	memset(buff, 0, len);
 	param = (TW_Param *)tw_dev->alignment_virtual_address[request_id];
 	if (param == NULL) {
 		printk(KERN_WARNING "3w-xxxx: tw_scsiop_read_capacity_complete(): Bad alignment virtual address.\n");
@@ -1739,6 +1775,8 @@ static int tw_scsiop_read_capacity_compl
 	buff[6] = (TW_BLOCK_SIZE >> 8) & 0xff;
 	buff[7] = TW_BLOCK_SIZE & 0xff;
 
+	tw_unmap_internal(tw_dev, request_id);
+
 	return 0;
 } /* End tw_scsiop_read_capacity_complete() */
 

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Nick Orlov]

On Sun, Jun 26, 2005 at 06:37:11PM -0500, James Bottomley wrote:
> On Sun, 2005-06-26 at 15:39 -0700, Andrew Morton wrote:
> > : OK ... I looked ... and it's not pretty.  The problem is that the 3ware
> > : assumes certain commands (INQUIRY, READ_CAPACITY, MODE_SENSE + a few
> > : others) are only generated internally and thus only have use_sg == 0.
> > : The scsi-block-tree breaks this assumption because we're trying to
> > : eliminate the use_sg == 0 special case.
> > : 
> > : It's important to note that this behaviour is already broken, as anyone
> > : using SG_IO to send commands to the device would have discovered.
> 
> OK, I think the attached is the fix.  It makes the 3w-xxxx routines
> handle sg data.  Can we verify it works first, and then we'll worry
> about how many other internal command processing devices are broken in
> this way.
> 

This fix caused the following Oops + kernel panic:

c0267437 tw_interrupt+0x317/0x480
c013ac9d handle_IRQ_event+0x3d/0x70
c013adb0 __do_IRQ+0xe0/0x170
c0104fc3 do_IRQ+0x23/0x40
c01031ce common_interrupt+0x1a/0x20
c0100ca6 default_idle+0x26/0x30
c0110bbc apm_cpu_idle+0xce/0x160
c0100d24 cpu_idle+0x34/0x50
c03947e5 start_kernel+0x165/0x180
c0394360 unknown_bootoption+0x0/0x1e0

I've manually copied that from the screen, so it can contain some typos.

Thank you,
	Nick Orlov.
-- 
With best wishes,
	Nick Orlov.

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Jens Axboe]

On Sun, Jun 26 2005, James Bottomley wrote:
> On Sun, 2005-06-26 at 15:39 -0700, Andrew Morton wrote:
> > : OK ... I looked ... and it's not pretty.  The problem is that the 3ware
> > : assumes certain commands (INQUIRY, READ_CAPACITY, MODE_SENSE + a few
> > : others) are only generated internally and thus only have use_sg == 0.
> > : The scsi-block-tree breaks this assumption because we're trying to
> > : eliminate the use_sg == 0 special case.
> > : 
> > : It's important to note that this behaviour is already broken, as anyone
> > : using SG_IO to send commands to the device would have discovered.
> 
> OK, I think the attached is the fix.  It makes the 3w-xxxx routines
> handle sg data.  Can we verify it works first, and then we'll worry
> about how many other internal command processing devices are broken in
> this way.

This wont work at all, I'm afraid. You cannot atomically kmap() the
buffer for queueing, and unmap it from interrupt context. The type
probably isn't correct, and you are not guaranteed to be on the same
CPU when doing the unmap. Thus you end up with corrupted maps and
preemption counts.

-- 
Jens Axboe

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[James Bottomley]

On Mon, 2005-06-27 at 00:46 -0400, Nick Orlov wrote:
> c0267437 tw_interrupt+0x317/0x480
> c013ac9d handle_IRQ_event+0x3d/0x70
> c013adb0 __do_IRQ+0xe0/0x170
> c0104fc3 do_IRQ+0x23/0x40
> c01031ce common_interrupt+0x1a/0x20
> c0100ca6 default_idle+0x26/0x30
> c0110bbc apm_cpu_idle+0xce/0x160
> c0100d24 cpu_idle+0x34/0x50
> c03947e5 start_kernel+0x165/0x180
> c0394360 unknown_bootoption+0x0/0x1e0

tw_interrupt wasn't actually modified.  However, it should have executed
another call, probably where it's doing a null ptr deref, probably in
one of the completion routines, which should be identified by the EIP;
could you tell me what this is?

Thanks,

James

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[James Bottomley]

On Mon, 2005-06-27 at 09:52 +0200, Jens Axboe wrote:
> This wont work at all, I'm afraid. You cannot atomically kmap() the
> buffer for queueing, and unmap it from interrupt context. The type
> probably isn't correct, and you are not guaranteed to be on the same
> CPU when doing the unmap. Thus you end up with corrupted maps and
> preemption counts.

Actually, it's a lot simpler than that.  We're generating the data for
the command, so all we do is send an equivalent command to the array (if
any) and then in the irq routine, kmap_atomic the buffer, copy the array
data into the buffer in the right order for the command then kunmap the
buffer again.  This should work fine, I think.

James

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Jens Axboe]

On Mon, Jun 27 2005, James Bottomley wrote:
> On Mon, 2005-06-27 at 09:52 +0200, Jens Axboe wrote:
> > This wont work at all, I'm afraid. You cannot atomically kmap() the
> > buffer for queueing, and unmap it from interrupt context. The type
> > probably isn't correct, and you are not guaranteed to be on the same
> > CPU when doing the unmap. Thus you end up with corrupted maps and
> > preemption counts.
> 
> Actually, it's a lot simpler than that.  We're generating the data for
> the command, so all we do is send an equivalent command to the array (if
> any) and then in the irq routine, kmap_atomic the buffer, copy the array
> data into the buffer in the right order for the command then kunmap the
> buffer again.  This should work fine, I think.

Yup, that would be ok, if that is what you did I misread where the
mapping took place! But you still can't use KM_USER0 in irq context.

-- 
Jens Axboe

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Nick Orlov]

On Mon, Jun 27, 2005 at 09:22:22AM -0500, James Bottomley wrote:
> On Mon, 2005-06-27 at 00:46 -0400, Nick Orlov wrote:
> > c0267437 tw_interrupt+0x317/0x480
> > c013ac9d handle_IRQ_event+0x3d/0x70
> > c013adb0 __do_IRQ+0xe0/0x170
> > c0104fc3 do_IRQ+0x23/0x40
> > c01031ce common_interrupt+0x1a/0x20
> > c0100ca6 default_idle+0x26/0x30
> > c0110bbc apm_cpu_idle+0xce/0x160
> > c0100d24 cpu_idle+0x34/0x50
> > c03947e5 start_kernel+0x165/0x180
> > c0394360 unknown_bootoption+0x0/0x1e0
> 
> tw_interrupt wasn't actually modified.  However, it should have executed
> another call, probably where it's doing a null ptr deref, probably in
> one of the completion routines, which should be identified by the EIP;
> could you tell me what this is?
> 

Sorry for the long delay with the response: I'm very limited in time during
the weekdays :(

Here is the info you asked for:

EFLAGS: 00010293
EIP is at tw_scsiop_mode_sense_complete+0xc7/0xe0
eax: 00000000 ebx: c03d4e00 ecx: 00000000 edx: 00000004
esi: dfca2400 edi: c03d4e04 ebp: 00000000 esp: c0393ef4
ds: 007b es: 007b ss:0068
Process swapper (pid: 0, threadinfo=c0392000, task=c031aba0)

Again I've wrote it down manually, so typos are possible.

Hope you'll find it useful.

Thank you,
	Nick Orlov.

-- 
With best wishes,
	Nick Orlov.

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[James Bottomley]

On Mon, 2005-06-27 at 22:08 -0400, Nick Orlov wrote:
> EIP is at tw_scsiop_mode_sense_complete+0xc7/0xe0

OK, so it got to the mode sense.  What happened to the preceeding
INQUIRY?  Did it produce correct values or did it trip the error
condition?

James

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Jens Axboe]

On Sun, Jun 26 2005, James Bottomley wrote:
> +static void tw_unmap_internal(TW_Device_Extension *tw_dev, int request_id)
> +{
> +	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
> +
> +	if (cmd->use_sg) {
> +		struct scatterlist *sg;
> +
> +		sg = (struct scatterlist *)cmd->request_buffer;
> +		kunmap_atomic(sg->page, KM_USER0);

Woops. You have to pass the mapped buffer back in, kunmap_atomic() takes
the returned buffer from kmap_atomic() not the page mapped. Silly yes,
but that is how it is...


-- 
Jens Axboe

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Nick Orlov]

On Mon, Jun 27, 2005 at 11:05:41PM -0500, James Bottomley wrote:
> On Mon, 2005-06-27 at 22:08 -0400, Nick Orlov wrote:
> > EIP is at tw_scsiop_mode_sense_complete+0xc7/0xe0
> 
> OK, so it got to the mode sense.  What happened to the preceeding
> INQUIRY?  Did it produce correct values or did it trip the error
> condition?

Hard to say. Kernel panics right after the small pause during detection.
And scroll back does not work. I don't really know what it prints before.

Thank you,
	Nick Orlov.
-- 
With best wishes,
	Nick Orlov.

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Jens Axboe]

On Tue, Jun 28 2005, Nick Orlov wrote:
> On Mon, Jun 27, 2005 at 11:05:41PM -0500, James Bottomley wrote:
> > On Mon, 2005-06-27 at 22:08 -0400, Nick Orlov wrote:
> > > EIP is at tw_scsiop_mode_sense_complete+0xc7/0xe0
> > 
> > OK, so it got to the mode sense.  What happened to the preceeding
> > INQUIRY?  Did it produce correct values or did it trip the error
> > condition?
> 
> Hard to say. Kernel panics right after the small pause during detection.
> And scroll back does not work. I don't really know what it prints before.

Try this variant, fixes the mapping type and kunmap_atomic().

diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -1499,22 +1499,50 @@ static int tw_scsiop_inquiry(TW_Device_E
 	return 0;
 } /* End tw_scsiop_inquiry() */
 
+static void *tw_map_internal(TW_Device_Extension *tw_dev, int request_id,
+			     int *len)
+{
+	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
+	void *buf;
+
+	if (cmd->use_sg) {
+		struct scatterlist *sg =
+			(struct scatterlist *)cmd->request_buffer;
+		buf = kmap_atomic(sg->page, KM_IRQ0) + sg->offset;
+		*len = sg->length;
+	} else {
+		buf = cmd->request_buffer;
+		*len = cmd->request_bufflen;
+	}
+	return buf;
+}
+
+static void tw_unmap_internal(TW_Device_Extension *tw_dev, char *ptr,
+			      int request_id)
+{
+	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
+
+	if (cmd->use_sg)
+		kunmap_atomic(ptr, KM_IRQ0);
+}
+
 /* This function is called by the isr to complete an inquiry command */
 static int tw_scsiop_inquiry_complete(TW_Device_Extension *tw_dev, int request_id)
 {
 	unsigned char *is_unit_present;
 	unsigned char *request_buffer;
+	int len;
 	TW_Param *param;
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_inquiry_complete()\n");
 
 	/* Fill request buffer */
-	if (tw_dev->srb[request_id]->request_buffer == NULL) {
+	request_buffer = tw_map_internal(tw_dev, request_id, &len);
+	if (request_buffer == NULL || len < 36) {
 		printk(KERN_WARNING "3w-xxxx: tw_scsiop_inquiry_complete(): Request buffer NULL.\n");
 		return 1;
 	}
-	request_buffer = tw_dev->srb[request_id]->request_buffer;
-	memset(request_buffer, 0, tw_dev->srb[request_id]->request_bufflen);
+	memset(request_buffer, 0, len);
 	request_buffer[0] = TYPE_DISK; /* Peripheral device type */
 	request_buffer[1] = 0;	       /* Device type modifier */
 	request_buffer[2] = 0;	       /* No ansi/iso compliance */
@@ -1522,6 +1550,7 @@ static int tw_scsiop_inquiry_complete(TW
 	memcpy(&request_buffer[8], "3ware   ", 8);	 /* Vendor ID */
 	sprintf(&request_buffer[16], "Logical Disk %-2d ", tw_dev->srb[request_id]->device->id);
 	memcpy(&request_buffer[32], TW_DRIVER_VERSION, 3);
+	tw_unmap_internal(tw_dev, request_buffer, request_id);
 
 	param = (TW_Param *)tw_dev->alignment_virtual_address[request_id];
 	if (param == NULL) {
@@ -1613,6 +1642,7 @@ static int tw_scsiop_mode_sense_complete
 	TW_Param *param;
 	unsigned char *flags;
 	unsigned char *request_buffer;
+	int len;
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_mode_sense_complete()\n");
 
@@ -1622,8 +1652,9 @@ static int tw_scsiop_mode_sense_complete
 		return 1;
 	}
 	flags = (char *)&(param->data[0]);
-	request_buffer = tw_dev->srb[request_id]->buffer;
-	memset(request_buffer, 0, tw_dev->srb[request_id]->request_bufflen);
+	request_buffer = tw_map_internal(tw_dev, request_id, &len);
+	memset(request_buffer, 0, len);
+	BUG_ON(len < 7);
 
 	request_buffer[0] = 0xf;        /* mode data length */
 	request_buffer[1] = 0;          /* default medium type */
@@ -1635,6 +1666,7 @@ static int tw_scsiop_mode_sense_complete
 		request_buffer[6] = 0x4;        /* WCE on */
 	else
 		request_buffer[6] = 0x0;        /* WCE off */
+	tw_unmap_internal(tw_dev, request_buffer, request_id);
 
 	return 0;
 } /* End tw_scsiop_mode_sense_complete() */
@@ -1703,15 +1735,16 @@ static int tw_scsiop_read_capacity_compl
 	u32 capacity;
 	char *buff;
 	TW_Param *param;
+	int len;
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_read_capacity_complete()\n");
 
-	buff = tw_dev->srb[request_id]->request_buffer;
-	if (buff == NULL) {
+	buff = tw_map_internal(tw_dev, request_id, &len);
+	if (buff == NULL || len < 8) {
 		printk(KERN_WARNING "3w-xxxx: tw_scsiop_read_capacity_complete(): Request buffer NULL.\n");
 		return 1;
 	}
-	memset(buff, 0, tw_dev->srb[request_id]->request_bufflen);
+	memset(buff, 0, len);
 	param = (TW_Param *)tw_dev->alignment_virtual_address[request_id];
 	if (param == NULL) {
 		printk(KERN_WARNING "3w-xxxx: tw_scsiop_read_capacity_complete(): Bad alignment virtual address.\n");
@@ -1739,6 +1772,8 @@ static int tw_scsiop_read_capacity_compl
 	buff[6] = (TW_BLOCK_SIZE >> 8) & 0xff;
 	buff[7] = TW_BLOCK_SIZE & 0xff;
 
+	tw_unmap_internal(tw_dev, buff, request_id);
+
 	return 0;
 } /* End tw_scsiop_read_capacity_complete() */
 

-- 
Jens Axboe

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[James Bottomley]

On Tue, 2005-06-28 at 14:09 +0200, Jens Axboe wrote:
> > Hard to say. Kernel panics right after the small pause during detection.
> > And scroll back does not work. I don't really know what it prints before.
> 
> Try this variant, fixes the mapping type and kunmap_atomic().

Actually, I suspect the problem is the BUG_ON(len < 7).  The mode sense
routines have to be really careful for USB, so we request 4 bytes at
first to get the true mode length, hence I think this trips.

The solution, I think is to obey the minimum transfer requests. Note
that 3w-xxxx was always technically overwriting the buffer before, also
note that it always fills in the caching mode page regardless of the
mode page it was actually asked for ...

Anyway, the attached should fix all of these issues (including the ones
Jens pointed out).

James

diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -1499,22 +1499,43 @@ static int tw_scsiop_inquiry(TW_Device_E
 	return 0;
 } /* End tw_scsiop_inquiry() */
 
+static void tw_transfer_internal(TW_Device_Extension *tw_dev, int request_id,
+				 void *data, unsigned int len)
+{
+	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
+	void *buf;
+	unsigned int transfer_len;
+
+	if (cmd->use_sg) {
+		struct scatterlist *sg =
+			(struct scatterlist *)cmd->request_buffer;
+		buf = kmap_atomic(sg->page, KM_IRQ0) + sg->offset;
+		transfer_len = min(sg->length, len);
+	} else {
+		buf = cmd->request_buffer;
+		transfer_len = min(cmd->request_bufflen, len);
+	}
+
+	memcpy(buf, data, transfer_len);
+	
+	if (cmd->use_sg) {
+		struct scatterlist *sg;
+
+		sg = (struct scatterlist *)cmd->request_buffer;
+		kunmap_atomic(buf - sg->offset, KM_IRQ0);
+	}
+}
+
 /* This function is called by the isr to complete an inquiry command */
 static int tw_scsiop_inquiry_complete(TW_Device_Extension *tw_dev, int request_id)
 {
 	unsigned char *is_unit_present;
-	unsigned char *request_buffer;
+	unsigned char request_buffer[36];
 	TW_Param *param;
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_inquiry_complete()\n");
 
-	/* Fill request buffer */
-	if (tw_dev->srb[request_id]->request_buffer == NULL) {
-		printk(KERN_WARNING "3w-xxxx: tw_scsiop_inquiry_complete(): Request buffer NULL.\n");
-		return 1;
-	}
-	request_buffer = tw_dev->srb[request_id]->request_buffer;
-	memset(request_buffer, 0, tw_dev->srb[request_id]->request_bufflen);
+	memset(request_buffer, 0, sizeof(request_buffer));
 	request_buffer[0] = TYPE_DISK; /* Peripheral device type */
 	request_buffer[1] = 0;	       /* Device type modifier */
 	request_buffer[2] = 0;	       /* No ansi/iso compliance */
@@ -1522,6 +1543,8 @@ static int tw_scsiop_inquiry_complete(TW
 	memcpy(&request_buffer[8], "3ware   ", 8);	 /* Vendor ID */
 	sprintf(&request_buffer[16], "Logical Disk %-2d ", tw_dev->srb[request_id]->device->id);
 	memcpy(&request_buffer[32], TW_DRIVER_VERSION, 3);
+	tw_transfer_internal(tw_dev, request_id, request_buffer,
+			     sizeof(request_buffer));
 
 	param = (TW_Param *)tw_dev->alignment_virtual_address[request_id];
 	if (param == NULL) {
@@ -1612,7 +1635,7 @@ static int tw_scsiop_mode_sense_complete
 {
 	TW_Param *param;
 	unsigned char *flags;
-	unsigned char *request_buffer;
+	unsigned char request_buffer[8];
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_mode_sense_complete()\n");
 
@@ -1622,8 +1645,7 @@ static int tw_scsiop_mode_sense_complete
 		return 1;
 	}
 	flags = (char *)&(param->data[0]);
-	request_buffer = tw_dev->srb[request_id]->buffer;
-	memset(request_buffer, 0, tw_dev->srb[request_id]->request_bufflen);
+	memset(request_buffer, 0, sizeof(request_buffer));
 
 	request_buffer[0] = 0xf;        /* mode data length */
 	request_buffer[1] = 0;          /* default medium type */
@@ -1635,6 +1657,8 @@ static int tw_scsiop_mode_sense_complete
 		request_buffer[6] = 0x4;        /* WCE on */
 	else
 		request_buffer[6] = 0x0;        /* WCE off */
+	tw_transfer_internal(tw_dev, request_id, request_buffer,
+			     sizeof(request_buffer));
 
 	return 0;
 } /* End tw_scsiop_mode_sense_complete() */
@@ -1701,17 +1725,12 @@ static int tw_scsiop_read_capacity_compl
 {
 	unsigned char *param_data;
 	u32 capacity;
-	char *buff;
+	char buff[8];
 	TW_Param *param;
 
 	dprintk(KERN_NOTICE "3w-xxxx: tw_scsiop_read_capacity_complete()\n");
 
-	buff = tw_dev->srb[request_id]->request_buffer;
-	if (buff == NULL) {
-		printk(KERN_WARNING "3w-xxxx: tw_scsiop_read_capacity_complete(): Request buffer NULL.\n");
-		return 1;
-	}
-	memset(buff, 0, tw_dev->srb[request_id]->request_bufflen);
+	memset(buff, 0, sizeof(buff));
 	param = (TW_Param *)tw_dev->alignment_virtual_address[request_id];
 	if (param == NULL) {
 		printk(KERN_WARNING "3w-xxxx: tw_scsiop_read_capacity_complete(): Bad alignment virtual address.\n");
@@ -1739,6 +1758,8 @@ static int tw_scsiop_read_capacity_compl
 	buff[6] = (TW_BLOCK_SIZE >> 8) & 0xff;
 	buff[7] = TW_BLOCK_SIZE & 0xff;
 
+	tw_transfer_internal(tw_dev, request_id, buff, sizeof(buff));
+
 	return 0;
 } /* End tw_scsiop_read_capacity_complete() */
 

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Jens Axboe]

On Tue, Jun 28 2005, James Bottomley wrote:
> On Tue, 2005-06-28 at 14:09 +0200, Jens Axboe wrote:
> > > Hard to say. Kernel panics right after the small pause during detection.
> > > And scroll back does not work. I don't really know what it prints before.
> > 
> > Try this variant, fixes the mapping type and kunmap_atomic().
> 
> Actually, I suspect the problem is the BUG_ON(len < 7).  The mode sense
> routines have to be really careful for USB, so we request 4 bytes at
> first to get the true mode length, hence I think this trips.

Sounds plausible, the other bugs wouldn't show up unless Nick is using
highmem (or has it enabled, at least).

-- 
Jens Axboe

Re: Fw: 2.6.12-mm2: 3ware SATA RAID inaccessible

[Nick Orlov]

On Tue, Jun 28, 2005 at 04:23:02PM +0200, Jens Axboe wrote:
> On Tue, Jun 28 2005, James Bottomley wrote:
> > On Tue, 2005-06-28 at 14:09 +0200, Jens Axboe wrote:
> > > > Hard to say. Kernel panics right after the small pause during detection.
> > > > And scroll back does not work. I don't really know what it prints before.
> > > 
> > > Try this variant, fixes the mapping type and kunmap_atomic().
> > 
> > Actually, I suspect the problem is the BUG_ON(len < 7).  The mode sense
> > routines have to be really careful for USB, so we request 4 bytes at
> > first to get the true mode length, hence I think this trips.
> 
> Sounds plausible, the other bugs wouldn't show up unless Nick is using
> highmem (or has it enabled, at least).
> 

I'd like to confirm that this patch solves the problem for me :)
And yes, I don't use highmem.

Thank you,
	Nick Orlov.

-- 
With best wishes,
	Nick Orlov.