Re: [usb-storage] PATCH: usb-storage-set-last-sector-bug-flag.patch

2008-01-22 Thread Guillaume Bedot
Hello,

Le dimanche 20 janvier 2008 à 11:27 +0100, Hans de Goede a écrit :
 Hi all,
 
 This patch sets the last_sector_bug flag to 1 for all USB disks. This is
 needed to makes the cardreader on various HP multifunction printers work.
 
 Since the performance impact is negible we set this flag for all USB disks to
 avoid an unusual_devs.h nightmare.
 
 Note that this patch depends on:
 PATCH: scsi-sd-last-sector-bug-flag.patch
 
 Which actually adds this flag to the scsi subsystem.
 
 Signed-off-by: Hans de Goede [EMAIL PROTECTED]
 
 Regards,
 
 Hans

Just a line to say these new patches work for me.

Best regards,

Guillaume B.

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


Re: Performance of SCST versus STGT

2008-01-22 Thread Bart Van Assche
On Jan 17, 2008 6:45 PM, Pete Wyckoff [EMAIL PROTECTED] wrote:
 There's nothing particularly stunning here.  Suspect Bart has
 configuration issues if not even IPoIB will do  100 MB/s.

By this time I found out that the BIOS of the test systems (Intel
Server Board S5000PAL) set the PCI-e parameter MaxReadReq to 128
bytes, which explains the low InfiniBand performance. After changing
this parameter to 4096 bytes the InfiniBand throughput was as
expected: ib_rdma_bw now reports a
bandwidth of 933 MB/s.

Bart.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] IB/iSER: add logical unit reset support

2008-01-22 Thread Erez Zilber
eh_device_reset_handler was already added to scsi_host_template
in iscsi_tcp, and is now added also for iscsi_iser.

Signed-off-by: Erez Zilber [EMAIL PROTECTED]
Signed-off-by: Mike Christie [EMAIL PROTECTED]
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index fd69fb3..4cd0705 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -552,6 +552,7 @@ static struct scsi_host_template iscsi_iser_sht = {
.max_sectors= 1024,
.cmd_per_lun= ISCSI_MAX_CMD_PER_LUN,
.eh_abort_handler   = iscsi_eh_abort,
+   .eh_device_reset_handler= iscsi_eh_device_reset,
.eh_host_reset_handler  = iscsi_eh_host_reset,
.use_clustering = DISABLE_CLUSTERING,
.proc_name  = iscsi_iser,
-- 
1.5.3.7



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


Re: [RFC] A SCSI fault injection framework using SystemTap.

2008-01-22 Thread K.Tanaka

The new framework is tested on Fedora8(i386) running with kernel 2.6.23.12.
So far, I'm cleaning up the tool set for release, and plan to post it in the 
near future.

Now it's ready. The scsi fault injection tool is available from the following 
site.
https://sourceforge.net/projects/scsifaultinjtst/
If you have any comments, please let me know.

Additionally, the deadlock problem reproduced also on md RAID10. I think that 
the
same reason for RAID1 deadlock reported earlier cause this problem, because
raid10.c is based on raid1.c.
 e.g.
  -The kernel thread for md RAID1 could cause a deadlock when the error 
 handler for
md RAID1 contends with the write access to the md RAID1 array.

I've reproduced the deadlock on RAID10 using this tool with a small shell 
script for
automatically injecting a fault repeatedly. But I can't come up with any good
idea for the patch to fix this problem so far.

-- 

Kenichi TANAKA| Open Source Software Platform Development Division
  | Computers Software Operations Unit, NEC Corporation
  | [EMAIL PROTECTED]

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


Re: Performance of SCST versus STGT

2008-01-22 Thread Vladislav Bolkhovitin

FUJITA Tomonori wrote:

The big problem of stgt iSER is disk I/Os (move data between disk and
page cache). We need a proper asynchronous I/O mechanism, however,
Linux doesn't provide such and we use a workaround, which incurs large
latency. I guess, we cannot solve this until syslets is merged into
mainline.


Hmm, SCST also doesn't have ability to use asynchronous I/O, but that 
doesn't prevent it from showing good performance.


Vlad
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Performance of SCST versus STGT

2008-01-22 Thread Vladislav Bolkhovitin

Bart Van Assche wrote:

On Jan 17, 2008 6:45 PM, Pete Wyckoff [EMAIL PROTECTED] wrote:


There's nothing particularly stunning here.  Suspect Bart has
configuration issues if not even IPoIB will do  100 MB/s.



By this time I found out that the BIOS of the test systems (Intel
Server Board S5000PAL) set the PCI-e parameter MaxReadReq to 128
bytes, which explains the low InfiniBand performance. After changing
this parameter to 4096 bytes the InfiniBand throughput was as
expected: ib_rdma_bw now reports a
bandwidth of 933 MB/s.


What are the new SRPT/iSER numbers?


Bart.



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


Re: Performance of SCST versus STGT

2008-01-22 Thread FUJITA Tomonori
On Tue, 22 Jan 2008 14:33:13 +0300
Vladislav Bolkhovitin [EMAIL PROTECTED] wrote:

 FUJITA Tomonori wrote:
  The big problem of stgt iSER is disk I/Os (move data between disk and
  page cache). We need a proper asynchronous I/O mechanism, however,
  Linux doesn't provide such and we use a workaround, which incurs large
  latency. I guess, we cannot solve this until syslets is merged into
  mainline.
 
 Hmm, SCST also doesn't have ability to use asynchronous I/O, but that 
 doesn't prevent it from showing good performance.

I don't know how SCST performs I/Os, but surely, in kernel space, you
can performs I/Os asynchronously. Or you use an event notification
mechanism with multiple kernel threads performing I/Os synchronously.

Xen blktap has the same problem as stgt. IIRC, Xen mainline uses a
kernel patch to add a proper event notification to AIO though redhat
uses the same workaround as stgt instead of applying the kernel patch.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Performance of SCST versus STGT

2008-01-22 Thread Vladislav Bolkhovitin

FUJITA Tomonori wrote:

On Tue, 22 Jan 2008 14:33:13 +0300
Vladislav Bolkhovitin [EMAIL PROTECTED] wrote:



FUJITA Tomonori wrote:


The big problem of stgt iSER is disk I/Os (move data between disk and
page cache). We need a proper asynchronous I/O mechanism, however,
Linux doesn't provide such and we use a workaround, which incurs large
latency. I guess, we cannot solve this until syslets is merged into
mainline.


Hmm, SCST also doesn't have ability to use asynchronous I/O, but that 
doesn't prevent it from showing good performance.



I don't know how SCST performs I/Os, but surely, in kernel space, you
can performs I/Os asynchronously.


Sure, but currently it all synchronous


Or you use an event notification
mechanism with multiple kernel threads performing I/Os synchronously.

Xen blktap has the same problem as stgt. IIRC, Xen mainline uses a
kernel patch to add a proper event notification to AIO though redhat
uses the same workaround as stgt instead of applying the kernel patch.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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


Re: Performance of SCST versus STGT

2008-01-22 Thread Bart Van Assche
On Jan 22, 2008 12:33 PM, Vladislav Bolkhovitin [EMAIL PROTECTED] wrote:

 What are the new SRPT/iSER numbers?

You can find the new performance numbers below. These are all numbers
for reading from the remote buffer cache, no actual disk reads were
performed. The read tests have been performed with dd, both for a
block size of 512 bytes and of 1 MB. The tests with small block size
learn more about latency, while the tests with large block size learn
more about the maximal possible throughput.

.
.   .   STGT read SCST read.STGT
read  SCST read.
.   .  performance   performance   .
performanceperformance   .
.   .  (0.5K, MB/s)  (0.5K, MB/s)  .   (1 MB,
MB/s)   (1 MB, MB/s)  .
.
. Ethernet (1 Gb/s network) .  77 78   .77
89   .
. IPoIB(8 Gb/s network) . 163185   .   201
   239   .
. iSER (8 Gb/s network) . 250N/A   .   360
   N/A   .
. SRP  (8 Gb/s network) . N/A421   .   N/A
   683   .
.

My conclusion from the above numbers: the performance difference
between STGT and SCST is small for a Gigabit Ethernet network. The
faster the network technology, the larger the difference between SCST
and STGT.

Bart.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Performance of SCST versus STGT

2008-01-22 Thread Vladislav Bolkhovitin

Bart Van Assche wrote:
On Jan 22, 2008 12:33 PM, Vladislav Bolkhovitin [EMAIL PROTECTED] 
mailto:[EMAIL PROTECTED] wrote:


 What are the new SRPT/iSER numbers?


You can find the new performance numbers below. These are all numbers 
for reading from the remote buffer cache, no actual disk reads were 
performed. The read tests have been performed with dd, both for a block 
size of 512 bytes and of 1 MB. The tests with small block size learn 
more about latency, while the tests with large block size learn more 
about the maximal possible throughput.


If you want to compare performance of 512b vs 1MB blocks, your 
experiment isn't fully correct. You should use iflag=direct dd option 
for that.


. 

.   .   STGT read SCST read.STGT 
read  SCST read.
.   .  performance   performance   .   
performanceperformance   .
.   .  (0.5K, MB/s)  (0.5K, MB/s)  .   (1 MB, 
MB/s)   (1 MB, MB/s)  .

.
. Ethernet (1 Gb/s network) .  77 78   .
77 89   .
. IPoIB(8 Gb/s network) . 163185   .   
201239   .
. iSER (8 Gb/s network) . 250N/A   .   
360N/A   .
. SRP  (8 Gb/s network) . N/A421   .   
N/A683   .

.

My conclusion from the above numbers: the performance difference between 
STGT and SCST is small for a Gigabit Ethernet network. The faster the 
network technology, the larger the difference between SCST and STGT.


This is what I expected


Bart.


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


Re: Performance of SCST versus STGT

2008-01-22 Thread Bart Van Assche
On Jan 22, 2008 4:26 AM, FUJITA Tomonori [EMAIL PROTECTED] wrote:

 First, I recommend you to examine iSER stuff more since it has some
 parameters unlike SRP, which effects the performance, IIRC. At least,
 you could get the iSER performances similar to Pete's.

Apparently open-iscsi uses the following defaults:
node.session.iscsi.FirstBurstLength = 262144
node.session.iscsi.MaxBurstLength = 16776192
node.conn[0].tcp.window_size = 524288
node.conn[0].iscsi.MaxRecvDataSegmentLength = 131072

I have tried to change some of these parameters to a larger value, but
this did not have a noticeable effect (read bandwidth increased less
than 1%):
$ iscsiadm --mode node --targetname
iqn.2007-05.com.example:storage.disk2.sys1.xyz --portal
192.168.102.5:3260 --op update -n node.session.iscsi.FirstBurstLength
-v 16776192
$ iscsiadm --mode node --targetname
iqn.2007-05.com.example:storage.disk2.sys1.xyz --portal
192.168.102.5:3260 --op update -n node.session.iscsi.MaxBurstLength -v
16776192
$ iscsiadm --mode node --targetname
iqn.2007-05.com.example:storage.disk2.sys1.xyz --portal
192.168.102.5:3260 --op update -n
node.conn[0].iscsi.MaxRecvDataSegmentLength -v 16776192

Bart.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] scsi_debug: add get_data_transfer_info helper function

2008-01-22 Thread FUJITA Tomonori
This adds get_data_transfer_info helper function that get lha and
sectors for READ_* and WRITE_* commands (and XDWRITEREAD_10 later).

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_debug.c |   83 
 1 files changed, 38 insertions(+), 45 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 82c06f0..31f7378 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -311,12 +311,47 @@ static void sdebug_max_tgts_luns(void);
 static struct device pseudo_primary;
 static struct bus_type pseudo_lld_bus;
 
+static void get_data_transfer_info(unsigned char *cmd,
+  unsigned long long *lba, unsigned int *num)
+{
+   int i;
+
+   switch (*cmd) {
+   case WRITE_16:
+   case READ_16:
+   for (*lba = 0, i = 0; i  8; ++i) {
+   if (i  0)
+   *lba = 8;
+   *lba += cmd[2 + i];
+   }
+   *num = cmd[13] + (cmd[12]  8) +
+   (cmd[11]  16) + (cmd[10]  24);
+   break;
+   case WRITE_12:
+   case READ_12:
+   *lba = cmd[5] + (cmd[4]  8) + (cmd[3]  16) + (cmd[2]  24);
+   *num = cmd[9] + (cmd[8]  8) + (cmd[7]  16) + (cmd[6]  24);
+   break;
+   case WRITE_10:
+   case READ_10:
+   *lba = cmd[5] + (cmd[4]  8) + (cmd[3]  16) + (cmd[2]  24);
+   *num = cmd[8] + (cmd[7]  8);
+   break;
+   case WRITE_6:
+   case READ_6:
+   *lba = cmd[3] + (cmd[2]  8) + ((cmd[1]  0x1f)  16);
+   *num = (0 == cmd[4]) ? 256 : cmd[4];
+   break;
+   default:
+   break;
+   }
+}
 
 static
 int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, done_funct_t done)
 {
unsigned char *cmd = (unsigned char *) SCpnt-cmnd;
-   int len, k, j;
+   int len, k;
unsigned int num;
unsigned long long lba;
int errsts = 0;
@@ -452,28 +487,7 @@ int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, 
done_funct_t done)
break;
if (scsi_debug_fake_rw)
break;
-   if ((*cmd) == READ_16) {
-   for (lba = 0, j = 0; j  8; ++j) {
-   if (j  0)
-   lba = 8;
-   lba += cmd[2 + j];
-   }
-   num = cmd[13] + (cmd[12]  8) +
-   (cmd[11]  16) + (cmd[10]  24);
-   } else if ((*cmd) == READ_12) {
-   lba = cmd[5] + (cmd[4]  8) +
-   (cmd[3]  16) + (cmd[2]  24);
-   num = cmd[9] + (cmd[8]  8) +
-   (cmd[7]  16) + (cmd[6]  24);
-   } else if ((*cmd) == READ_10) {
-   lba = cmd[5] + (cmd[4]  8) +
-   (cmd[3]  16) + (cmd[2]  24);
-   num = cmd[8] + (cmd[7]  8);
-   } else {/* READ (6) */
-   lba = cmd[3] + (cmd[2]  8) +
-   ((cmd[1]  0x1f)  16);
-   num = (0 == cmd[4]) ? 256 : cmd[4];
-   }
+   get_data_transfer_info(cmd, lba, num);
errsts = resp_read(SCpnt, lba, num, devip);
if (inj_recovered  (0 == errsts)) {
mk_sense_buffer(devip, RECOVERED_ERROR,
@@ -500,28 +514,7 @@ int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, 
done_funct_t done)
break;
if (scsi_debug_fake_rw)
break;
-   if ((*cmd) == WRITE_16) {
-   for (lba = 0, j = 0; j  8; ++j) {
-   if (j  0)
-   lba = 8;
-   lba += cmd[2 + j];
-   }
-   num = cmd[13] + (cmd[12]  8) +
-   (cmd[11]  16) + (cmd[10]  24);
-   } else if ((*cmd) == WRITE_12) {
-   lba = cmd[5] + (cmd[4]  8) +
-   (cmd[3]  16) + (cmd[2]  24);
-   num = cmd[9] + (cmd[8]  8) +
-   (cmd[7]  16) + (cmd[6]  24);
-   } else if ((*cmd) == WRITE_10) {
-   lba = cmd[5] + (cmd[4]  8) +
-   (cmd[3]  16) + (cmd[2]  24);
-   num = cmd[8] + (cmd[7]  8);
-   } else {/* WRITE (6) */
-   lba = cmd[3] + (cmd[2]  8) +
-   ((cmd[1]  0x1f)  16);
-   num = (0 == cmd[4]) ? 256 : cmd[4];
-   }
+   get_data_transfer_info(cmd, lba, num);

Re: [PATCH 0/3] update bidirectional series to sit on top of sg_table

2008-01-22 Thread FUJITA Tomonori
From: James Bottomley [EMAIL PROTECTED]
Subject: [PATCH 0/3] update bidirectional series to sit on top of sg_table
Date: Fri, 11 Jan 2008 21:09:00 -0600

 OK, I suppose in the scheme of things, it's my turn to bear some of the
 pain.  the SCSI bidirectional series rejects pretty badly with sg_table,
 and since sg_table has to go in, I rebased the series on top of it.
 
 Additionally, I tidied up the patches to take advantages of some of the
 features of sg_table.  I killed both use_sg and sg_count in favour of
 using sg_table.nseg for the count.
 
 Just so you can test all of this to make sure I got it right, you can
 pull the patch series from
 
 master.kernel.org:/pub/scm/linux/kernel/git/jejb/scsi-bidi-2.6.git

I've just started to look at the bidi tree.

I suppose that only panasas guys have tested the bidi tree with their
OSD target devices. To test the bidi tree, I added XDWRITEREAD_10
support to scsi_debug and sgv4_xdwriteread tool to my makeshift bsg
tool collections:

git://git.kernel.org/pub/scm/linux/kernel/git/tomo/sgv4-tools.git

It just sends XDWRITEREAD_10 commands like this:

tulip:~# sgv4_xdwriteread --length 16384 /sys/class/bsg/1:0:0:0
driver:0, transport:0, device:0, din_resid: 0, dout_resid: 0

No errors.

I'll send the patchset (over the bidi tree) to add XDWRITEREAD_10
support to scsi_debug though I'm not sure whether it's worth adding it
to mainline.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/3] scsi_debug: add bidi data transfer support

2008-01-22 Thread FUJITA Tomonori
This enables fill_from_dev_buffer and fetch_to_dev_buffer to handle
bidi commands.

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_debug.c |   21 ++---
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 31f7378..d810aa7 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -594,18 +594,18 @@ static int fill_from_dev_buffer(struct scsi_cmnd * scp, 
unsigned char * arr,
int k, req_len, act_len, len, active;
void * kaddr;
void * kaddr_off;
-   struct scatterlist * sg;
+   struct scatterlist *sg;
+   struct scsi_data_buffer *sdb = scsi_in(scp);
 
-   if (0 == scsi_bufflen(scp))
+   if (!sdb-length)
return 0;
-   if (NULL == scsi_sglist(scp))
+   if (!sdb-table.sgl)
return (DID_ERROR  16);
-   if (! ((scp-sc_data_direction == DMA_BIDIRECTIONAL) ||
- (scp-sc_data_direction == DMA_FROM_DEVICE)))
+   if (!(scsi_bidi_cmnd(scp) || scp-sc_data_direction == DMA_FROM_DEVICE))
return (DID_ERROR  16);
active = 1;
req_len = act_len = 0;
-   scsi_for_each_sg(scp, sg, scsi_sg_count(scp), k) {
+   for_each_sg(sdb-table.sgl, sg, sdb-table.nents, k) {
if (active) {
kaddr = (unsigned char *)
kmap_atomic(sg_page(sg), KM_USER0);
@@ -623,10 +623,10 @@ static int fill_from_dev_buffer(struct scsi_cmnd * scp, 
unsigned char * arr,
}
req_len += sg-length;
}
-   if (scsi_get_resid(scp))
-   scsi_set_resid(scp, scsi_get_resid(scp) - act_len);
+   if (sdb-resid)
+   sdb-resid -= act_len;
else
-   scsi_set_resid(scp, req_len - act_len);
+   sdb-resid = req_len - act_len;
return 0;
 }
 
@@ -643,8 +643,7 @@ static int fetch_to_dev_buffer(struct scsi_cmnd * scp, 
unsigned char * arr,
return 0;
if (NULL == scsi_sglist(scp))
return -1;
-   if (! ((scp-sc_data_direction == DMA_BIDIRECTIONAL) ||
- (scp-sc_data_direction == DMA_TO_DEVICE)))
+   if (!(scsi_bidi_cmnd(scp) || scp-sc_data_direction == DMA_TO_DEVICE))
return -1;
req_len = fin = 0;
scsi_for_each_sg(scp, sg, scsi_sg_count(scp), k) {
-- 
1.5.3.4

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


[PATCH 3/3] scsi_debug: add XDWRITEREAD_10 support

2008-01-22 Thread FUJITA Tomonori

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_debug.c |   70 +
 include/scsi/scsi.h   |1 +
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d810aa7..1541c17 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -280,6 +280,8 @@ static int resp_write(struct scsi_cmnd * SCpnt, unsigned 
long long lba,
  unsigned int num, struct sdebug_dev_info * devip);
 static int resp_report_luns(struct scsi_cmnd * SCpnt,
struct sdebug_dev_info * devip);
+static int resp_xdwriteread(struct scsi_cmnd *scp, unsigned long long lba,
+   unsigned int num, struct sdebug_dev_info *devip);
 static int fill_from_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,
 int arr_len);
 static int fetch_to_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,
@@ -334,6 +336,7 @@ static void get_data_transfer_info(unsigned char *cmd,
break;
case WRITE_10:
case READ_10:
+   case XDWRITEREAD_10:
*lba = cmd[5] + (cmd[4]  8) + (cmd[3]  16) + (cmd[2]  24);
*num = cmd[8] + (cmd[7]  8);
break;
@@ -542,6 +545,28 @@ int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, 
done_funct_t done)
case WRITE_BUFFER:
errsts = check_readiness(SCpnt, 1, devip);
break;
+   case XDWRITEREAD_10:
+   if (!scsi_bidi_cmnd(SCpnt)) {
+   mk_sense_buffer(devip, ILLEGAL_REQUEST,
+   INVALID_FIELD_IN_CDB, 0);
+   errsts = check_condition_result;
+   break;
+   }
+
+   errsts = check_readiness(SCpnt, 0, devip);
+   if (errsts)
+   break;
+   if (scsi_debug_fake_rw)
+   break;
+   get_data_transfer_info(cmd, lba, num);
+   errsts = resp_read(SCpnt, lba, num, devip);
+   if (errsts)
+   break;
+   errsts = resp_write(SCpnt, lba, num, devip);
+   if (errsts)
+   break;
+   errsts = resp_xdwriteread(SCpnt, lba, num, devip);
+   break;
default:
if (SCSI_DEBUG_OPT_NOISE  scsi_debug_opts)
printk(KERN_INFO scsi_debug: Opcode: 0x%x not 
@@ -1948,6 +1973,50 @@ static int resp_report_luns(struct scsi_cmnd * scp,
min((int)alloc_len, SDEBUG_RLUN_ARR_SZ));
 }
 
+static int resp_xdwriteread(struct scsi_cmnd *scp, unsigned long long lba,
+   unsigned int num, struct sdebug_dev_info *devip)
+{
+   int i, j, ret = -1;
+   unsigned char *kaddr, *buf;
+   unsigned int offset;
+   struct scatterlist *sg;
+   struct scsi_data_buffer *sdb = scsi_in(scp);
+
+   /* better not to use temporary buffer. */
+   buf = kmalloc(scsi_bufflen(scp), GFP_ATOMIC);
+   if (!buf)
+   return ret;
+
+   offset = 0;
+   scsi_for_each_sg(scp, sg, scsi_sg_count(scp), i) {
+   kaddr = (unsigned char *)kmap_atomic(sg_page(sg), KM_USER0);
+   if (!kaddr)
+   goto out;
+
+   memcpy(buf + offset, kaddr + sg-offset, sg-length);
+   offset += sg-length;
+   kunmap_atomic(kaddr, KM_USER0);
+   }
+
+   offset = 0;
+   for_each_sg(sdb-table.sgl, sg, sdb-table.nents, i) {
+   kaddr = (unsigned char *)kmap_atomic(sg_page(sg), KM_USER0);
+   if (!kaddr)
+   goto out;
+
+   for (j = 0; j  sg-length; j++)
+   *(kaddr + sg-offset + j) ^= *(buf + offset + j);
+
+   offset += sg-length;
+   kunmap_atomic(kaddr, KM_USER0);
+   }
+   ret = 0;
+out:
+   kfree(buf);
+
+   return ret;
+}
+
 /* When timer goes off this function is called. */
 static void timer_intr_handler(unsigned long indx)
 {
@@ -1981,6 +2050,7 @@ static int scsi_debug_slave_alloc(struct scsi_device * 
sdp)
if (SCSI_DEBUG_OPT_NOISE  scsi_debug_opts)
printk(KERN_INFO scsi_debug: slave_alloc %u %u %u %u\n,
   sdp-host-host_no, sdp-channel, sdp-id, sdp-lun);
+   set_bit(QUEUE_FLAG_BIDI, sdp-request_queue-queue_flags);
return 0;
 }
 
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 056c5af..19ca9e9 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -102,6 +102,7 @@ extern const unsigned char scsi_command_size[8];
 #define READ_TOC  0x43
 #define LOG_SELECT0x4c
 #define LOG_SENSE 0x4d
+#define XDWRITEREAD_100x53
 #define MODE_SELECT_100x55
 #define RESERVE_10

Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread James Bottomley
Added linux-scsi to the cc.

On Tue, 2008-01-22 at 17:11 +, Hugh Dickins wrote:
 2.6.24-rc8-mm1 is corrupting.  smartd does some sg_ioctl into its stack,
 and depending on how its stack randomization worked out, this is liable
 to end up writing into the adjacent physical page too.  If you're lucky
 you have highmem, and ioread16_rep oopses on the virtual address beyond
 what ata_pio_sector's kmap_atomic set up; when you're unlucky, it'll go
 ahead and corrupt the next page more obscurely.
 
 Bisect led to scsi-misc-2.6.git's 465ff3185e0cb76d46137335a4d21d0d9d3ac8a2
 [SCSI] relax scsi dma alignment, and the patch below is good for a hot-fix.
 Though I doubt it'll be the final fix, since it appears to undo the whole
 point of blk_queue_update_dma_alignment: perhaps libata is at fault to be
 going through sectory code for unsectory I/O - I wouldn't know.
 
 Signed-off-by: Hugh Dickins [EMAIL PROTECTED]
 ---
 
  drivers/ata/libata-scsi.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 --- 2.6.24-rc8-mm1/drivers/ata/libata-scsi.c  2008-01-17 16:49:47.0 
 +
 +++ linux/drivers/ata/libata-scsi.c   2008-01-22 15:45:40.0 +
 @@ -826,7 +826,7 @@ static void ata_scsi_sdev_config(struct 
   sdev-max_device_blocked = 1;
  
   /* set the min alignment */
 - blk_queue_update_dma_alignment(sdev-request_queue, ATA_DMA_PAD_SZ - 1);
 + blk_queue_update_dma_alignment(sdev-request_queue, ATA_SECT_SIZE - 1);
  }
  
  static void ata_scsi_dev_config(struct scsi_device *sdev,

Unfortunately, that's likely not the entire hot fix ... the implication
is that we have some mapping error in the way we do direct SG_IO.

What the fix you propose does is make it far more likely that block will
copy, perform I/O then uncopy (almost certain, since most smartd data
transfers are well under ATA_SECT_SIZE, which is 512).  However,
implicating a generic path like this implies that we would get the same
problem for SCSI commands as well, so the correct hot fix is below.

However, I'd like to see if we can track the problem through the SG_IO
direct path ... how many adjacent page bytes are corrupt?  Just a few or
a large number (I'm wondering if it's an off by one or off by alignment
type bug)?

James

---
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 4cf902e..13376e9 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1673,8 +1673,11 @@ struct request_queue *__scsi_alloc_queue(struct 
Scsi_Host *shost,
 * set a reasonable default alignment on word boundaries: the
 * host and device may alter it using
 * blk_queue_update_dma_alignment() later.
+*
+* FIXME: Corruption showing up in SG_IO leads this to be
+* raised to 511 again.
 */
-   blk_queue_dma_alignment(q, 0x03);
+   blk_queue_dma_alignment(q, 511);
 
return q;
 }


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


Re: INITIO scsi driver fails to work properly

2008-01-22 Thread Filippos Papadopoulos
I get the following:
SAH
SSH
SCB Q
SCB EXEC
SCB EXEC DONE

After ~3 secs the system freezes.




On Jan 22, 2008 12:20 AM, Alan Cox [EMAIL PROTECTED] wrote:
 Ok my attempt to get the card failed so we are going to have to do this
 the hard way. See where this patch crashes and what it prints

 (On top of the other patches)

 diff -u --new-file --recursive --exclude-from /usr/src/exclude 
 linux.vanilla-2.6.24-rc8-mm1/drivers/scsi/initio.c 
 linux-2.6.24-rc8-mm1/drivers/scsi/initio.c
 --- linux.vanilla-2.6.24-rc8-mm1/drivers/scsi/initio.c  2008-01-19 
 14:22:43.0 +
 +++ linux-2.6.24-rc8-mm1/drivers/scsi/initio.c  2008-01-21 14:54:48.0 
 +
 @@ -2537,10 +2537,12 @@
 struct Scsi_Host *dev = dev_id;
 unsigned long flags;
 int r;
 -
 +
 +   printk(ISR\n);
 spin_lock_irqsave(dev-host_lock, flags);
 r = initio_isr((struct initio_host *)dev-hostdata);
 spin_unlock_irqrestore(dev-host_lock, flags);
 +   printk(ISR DONE %d\n, r);
 if (r)
 return IRQ_HANDLED;
 else
 @@ -2643,6 +2645,7 @@
 struct initio_host *host = (struct initio_host *) 
 cmd-device-host-hostdata;
 struct scsi_ctrl_blk *cmnd;

 +   printk(SCB QUEUE\n);
 cmd-scsi_done = done;

 cmnd = initio_alloc_scb(host);
 @@ -2650,7 +2653,9 @@
 return SCSI_MLQUEUE_HOST_BUSY;

 initio_build_scb(host, cmnd, cmd);
 +   printk(SCB EXEC\n);
 initio_exec_scb(host, cmnd);
 +   printk(SCB EXEC DONE\n);
 return 0;
  }

 @@ -2766,6 +2771,8 @@
 struct scsi_cmnd *cmnd; /* Pointer to SCSI request block */
 struct initio_host *host;
 struct scsi_ctrl_blk *cblk;
 +
 +   printk(SCB POST\n);

 host = (struct initio_host *) host_mem;
 cblk = (struct scsi_ctrl_blk *) cblk_mem;
 @@ -2934,9 +2941,11 @@

 pci_set_drvdata(pdev, shost);

 +   printk(SAH\n);
 error = scsi_add_host(shost, pdev-dev);
 if (error)
 goto out_free_irq;
 +   printk(SSH\n);
 scsi_scan_host(shost);
 return 0;
  out_free_irq:

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


[trivial patch] scsi/ultrastor: clean up inline asm warnings (fwd)

2008-01-22 Thread Adrian Bunk
James, can you review and apply this patch?

TIA
Adrian


- Forwarded message from Frederik Deweerdt [EMAIL PROTECTED] -

Date: Wed, 16 Jan 2008 17:19:08 +0100
From: Frederik Deweerdt [EMAIL PROTECTED]
To: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Subject: [trivial patch] scsi/ultrastor: clean up inline asm warnings

Hi,

Compiling latest mainline with gcc 4.2.1 spews the following warnings:

drivers/scsi/ultrastor.c: In function 'find_and_clear_bit_16':
drivers/scsi/ultrastor.c:303: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:302: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c: At top level:
drivers/scsi/ultrastor.c:1202: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:1202: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c: In function 'ultrastor_queuecommand':
drivers/scsi/ultrastor.c:698: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:698: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:698: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:698: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:698: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:698: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:698: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:302: warning: matching constraint does not allow a 
register
drivers/scsi/ultrastor.c:302: warning: matching constraint does not allow a 
register

The following patch fixes it by using the '+' operator on the (*field)
operand, marking it as read-write to gcc. I diffed the two resulting .s,
and gcc produced the same code. This was tested with gcc 4.2.1 and gcc 3.4.3

$ diff -u drivers/scsi/ultrastor.s{_,}
--- drivers/scsi/ultrastor.s_   2008-01-16 16:40:01.0 +0100
+++ drivers/scsi/ultrastor.s2008-01-16 16:40:19.0 +0100
@@ -1457,7 +1457,7 @@
je  .L268   #,
 #APP
xorl %ebp,%ebp  # mscp_index
-0: bsfw config+20,%bp  # config.mscp_free, mscp_index
+   0: bsfw config+20,%bp   # config.mscp_free, mscp_index
btr %ebp,config+20  # mscp_index, config.mscp_free
jnc 0b
 #NO_APP
@@ -1726,9 +1726,9 @@
callpanic   #
.size   ultrastor_queuecommand, .-ultrastor_queuecommand
.section.modinfo,a,@progbits
-   .type   __mod_license1184, @object
-   .size   __mod_license1184, 12
-__mod_license1184:
+   .type   __mod_license1191, @object
+   .size   __mod_license1191, 12
+__mod_license1191:
.string license=GPL
.section.rodata.str1.1
 .LC27:


Regards,
Frederik

Signed-off-by: Frederik Deweerdt [EMAIL PROTECTED]

diff --git a/drivers/scsi/ultrastor.c b/drivers/scsi/ultrastor.c
index 6d1f0ed..93c5a67 100644
--- a/drivers/scsi/ultrastor.c
+++ b/drivers/scsi/ultrastor.c
@@ -298,9 +298,16 @@ static inline int find_and_clear_bit_16(unsigned long 
*field)
 {
   int rv;
 
-  if (*field == 0) panic(No free mscp);
-  asm(xorl %0,%0\n0:\tbsfw %1,%w0\n\tbtr %0,%1\n\tjnc 0b
-  : =r (rv), =m (*field) : 1 (*field));
+  if (*field == 0)
+panic(No free mscp);
+
+  asm volatile (
+   xorl %0,%0\n\t
+   0: bsfw %1,%w0\n\t
+   btr %0,%1\n\t
+   jnc 0b
+   : =r (rv), =m (*field) :);
+
   return rv;
 }
 

- End forwarded message -

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


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread Hugh Dickins
On Tue, 22 Jan 2008, James Bottomley wrote:
  --- 2.6.24-rc8-mm1/drivers/ata/libata-scsi.c2008-01-17 
  16:49:47.0 +
  +++ linux/drivers/ata/libata-scsi.c 2008-01-22 15:45:40.0 +
  @@ -826,7 +826,7 @@ static void ata_scsi_sdev_config(struct 
  sdev-max_device_blocked = 1;
   
  /* set the min alignment */
  -   blk_queue_update_dma_alignment(sdev-request_queue, ATA_DMA_PAD_SZ - 1);
  +   blk_queue_update_dma_alignment(sdev-request_queue, ATA_SECT_SIZE - 1);
   }
   
   static void ata_scsi_dev_config(struct scsi_device *sdev,
 
 Unfortunately, that's likely not the entire hot fix ... the implication
 is that we have some mapping error in the way we do direct SG_IO.

Quite possibly, I'm not sure.

 What the fix you propose does is make it far more likely that block will
 copy, perform I/O then uncopy (almost certain, since most smartd data
 transfers are well under ATA_SECT_SIZE, which is 512).  However,
 implicating a generic path like this implies that we would get the same
 problem for SCSI commands as well, so the correct hot fix is below.

I've not noticed any problems from the normal activity of the system,
only from smartd's sg_ioctl.  My impression was that it's a libata
issue, because it's going through ata_pio_sector, which does 

ap-ops-data_xfer(qc-dev, buf + offset, qc-sect_size, do_write);

referring to sect_size, without considering the possibility of any smaller
I/O size.  (Me, I don't even know why it's going PIO rather than DMA:
I'm assuming smartd does things that way, but there's no limit to my
ignorance here.)

 However, I'd like to see if we can track the problem through the SG_IO
 direct path ... how many adjacent page bytes are corrupt?  Just a few or
 a large number (I'm wondering if it's an off by one or off by alignment
 type bug)?

I've assumed it's just the one next page: because ata_pio_sector is
doing a data_xfer of sect_size ATA_SECT_SIZE 512 to an offset above
0xe00 in the smartd stack page.  The time I actually saw corruption
rather than an oops at startup, it was in a tmpfs swap vector page
running 64-bit kernel, and I didn't examine any further pages (just
checked the page before and matched it up to smartd's stack, already
suspecting that).

I don't believe it's an off-by-one at your SCSI end.

Hugh
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Warning: sym53c8xx calls dma_free_coherent() with irqs disabled

2008-01-22 Thread Tony Battersby
When unloading, sym53c8xx calls dma_free_coherent() while holding a
spinlock (sym53c8xx_lock) with irqs disabled, which produces the
following warning with 2.6.24:

modprobe sym53c8xx
modprobe -r sym53c8xx
sym0: detaching ...
sym0: resetting chip
WARNING: at arch/x86/kernel/pci-dma_32.c:66 dma_free_coherent()
Pid: 682, comm: rmmod Not tainted 2.6.24-rc8-git5 #1
 [c0107a05] dma_free_coherent+0x95/0xa0
 [d0942066] ___free_dma_mem_cluster+0x46/0x70 [sym53c8xx]
 [d0941f0d] __sym_mfree+0x6d/0xc0 [sym53c8xx]
 [c010ec6c] smp_call_function+0x1c/0x20
 [c01209c8] on_each_cpu+0x28/0x40
 [d094232e] __sym_mfree_dma+0x5e/0xb0 [sym53c8xx]
 [d093d510] sym_hcb_free+0x70/0x170 [sym53c8xx]
 [d09389dc] sym_free_resources+0x3c/0x70 [sym53c8xx]
 [d093a06c] sym_detach+0x8c/0xb0 [sym53c8xx]
 [d093a0b9] sym2_remove+0x29/0x50 [sym53c8xx]
 [c01d5316] pci_device_remove+0x16/0x40
 [c0223498] __device_release_driver+0x68/0xb0
 [c02239a4] driver_detach+0xa4/0xb0
 [c0222f83] bus_remove_driver+0x73/0xa0
 [c01d549e] pci_unregister_driver+0xe/0x70
 [d0942dd0] sym2_exit+0x0/0x7d [sym53c8xx]
 [d0942dda] sym2_exit+0xa/0x7d [sym53c8xx]
 [c013e51b] sys_delete_module+0x11b/0x1b0
 [c0112dd9] do_page_fault+0xe9/0x640
 [c0154207] do_munmap+0x197/0x1f0
 [c0102a7e] sysenter_past_esp+0x5f/0x85
 [c02f] __lro_proc_segment+0x130/0x300
 ===

However, everything does work OK at least on i386; the warning is for
portability to other archs (ARM and MIPS).  This warning was added in
2.6.24-rc1, so the message does not appear in 2.6.23, although it is not
really a regression.  This appears non-trivial to fix, since the
spinlock protects access to higher-level data structures.

Tony Battersby
Cybernetics

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


Re: Warning: sym53c8xx calls dma_free_coherent() with irqs disabled

2008-01-22 Thread Matthew Wilcox
On Tue, Jan 22, 2008 at 01:19:09PM -0500, Tony Battersby wrote:
 When unloading, sym53c8xx calls dma_free_coherent() while holding a
 spinlock (sym53c8xx_lock) with irqs disabled, which produces the
 following warning with 2.6.24:

Ugh.  I'm slightly torn.  On the one hand, it's probably possible to
fix sym_malloc.  On the other hand, it's really long past time that
sym2 dropped its custom allocator and started using dma pools.

I'll take a look at converting to dma pools once I've got half a dozen
other projects that're higher priority out of the way ;-(

In any case thanks for the report.

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] [SCSI]: constify function pointer tables

2008-01-22 Thread Jan Engelhardt
Signed-off-by: Jan Engelhardt [EMAIL PROTECTED]
---
 drivers/scsi/lpfc/lpfc_debugfs.c |   10 +-
 drivers/scsi/sg.c|8 

diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index d6a98bc..002f541 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -715,7 +715,7 @@ lpfc_debugfs_release(struct inode *inode, struct file *file)
 }
 
 #undef lpfc_debugfs_op_disc_trc
-static struct file_operations lpfc_debugfs_op_disc_trc = {
+static const struct file_operations lpfc_debugfs_op_disc_trc = {
.owner =THIS_MODULE,
.open = lpfc_debugfs_disc_trc_open,
.llseek =   lpfc_debugfs_lseek,
@@ -724,7 +724,7 @@ static struct file_operations lpfc_debugfs_op_disc_trc = {
 };
 
 #undef lpfc_debugfs_op_nodelist
-static struct file_operations lpfc_debugfs_op_nodelist = {
+static const struct file_operations lpfc_debugfs_op_nodelist = {
.owner =THIS_MODULE,
.open = lpfc_debugfs_nodelist_open,
.llseek =   lpfc_debugfs_lseek,
@@ -733,7 +733,7 @@ static struct file_operations lpfc_debugfs_op_nodelist = {
 };
 
 #undef lpfc_debugfs_op_hbqinfo
-static struct file_operations lpfc_debugfs_op_hbqinfo = {
+static const struct file_operations lpfc_debugfs_op_hbqinfo = {
.owner =THIS_MODULE,
.open = lpfc_debugfs_hbqinfo_open,
.llseek =   lpfc_debugfs_lseek,
@@ -742,7 +742,7 @@ static struct file_operations lpfc_debugfs_op_hbqinfo = {
 };
 
 #undef lpfc_debugfs_op_dumpslim
-static struct file_operations lpfc_debugfs_op_dumpslim = {
+static const struct file_operations lpfc_debugfs_op_dumpslim = {
.owner =THIS_MODULE,
.open = lpfc_debugfs_dumpslim_open,
.llseek =   lpfc_debugfs_lseek,
@@ -751,7 +751,7 @@ static struct file_operations lpfc_debugfs_op_dumpslim = {
 };
 
 #undef lpfc_debugfs_op_slow_ring_trc
-static struct file_operations lpfc_debugfs_op_slow_ring_trc = {
+static const struct file_operations lpfc_debugfs_op_slow_ring_trc = {
.owner =THIS_MODULE,
.open = lpfc_debugfs_slow_ring_trc_open,
.llseek =   lpfc_debugfs_lseek,
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index f1871ea..c3f0e32 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1309,7 +1309,7 @@ sg_cmd_done(void *data, char *sense, int result, int 
resid)
}
 }
 
-static struct file_operations sg_fops = {
+static const struct file_operations sg_fops = {
.owner = THIS_MODULE,
.read = sg_read,
.write = sg_write,
@@ -2594,7 +2594,7 @@ static struct file_operations dev_fops = {
.open = sg_proc_open_dev,
.release = seq_release,
 };
-static struct seq_operations dev_seq_ops = {
+static const struct seq_operations dev_seq_ops = {
.start = dev_seq_start,
.next  = dev_seq_next,
.stop  = dev_seq_stop,
@@ -2607,7 +2607,7 @@ static struct file_operations devstrs_fops = {
.open = sg_proc_open_devstrs,
.release = seq_release,
 };
-static struct seq_operations devstrs_seq_ops = {
+static const struct seq_operations devstrs_seq_ops = {
.start = dev_seq_start,
.next  = dev_seq_next,
.stop  = dev_seq_stop,
@@ -2620,7 +2620,7 @@ static struct file_operations debug_fops = {
.open = sg_proc_open_debug,
.release = seq_release,
 };
-static struct seq_operations debug_seq_ops = {
+static const struct seq_operations debug_seq_ops = {
.start = dev_seq_start,
.next  = dev_seq_next,
.stop  = dev_seq_stop,
-- 
1.5.3.4

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


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread James Bottomley
On Tue, 2008-01-22 at 18:36 +, Hugh Dickins wrote:
 On Tue, 22 Jan 2008, James Bottomley wrote:
   --- 2.6.24-rc8-mm1/drivers/ata/libata-scsi.c  2008-01-17 
   16:49:47.0 +
   +++ linux/drivers/ata/libata-scsi.c   2008-01-22 15:45:40.0 
   +
   @@ -826,7 +826,7 @@ static void ata_scsi_sdev_config(struct 
 sdev-max_device_blocked = 1;

 /* set the min alignment */
   - blk_queue_update_dma_alignment(sdev-request_queue, ATA_DMA_PAD_SZ - 1);
   + blk_queue_update_dma_alignment(sdev-request_queue, ATA_SECT_SIZE - 1);
}

static void ata_scsi_dev_config(struct scsi_device *sdev,
  
  Unfortunately, that's likely not the entire hot fix ... the implication
  is that we have some mapping error in the way we do direct SG_IO.
 
 Quite possibly, I'm not sure.
 
  What the fix you propose does is make it far more likely that block will
  copy, perform I/O then uncopy (almost certain, since most smartd data
  transfers are well under ATA_SECT_SIZE, which is 512).  However,
  implicating a generic path like this implies that we would get the same
  problem for SCSI commands as well, so the correct hot fix is below.
 
 I've not noticed any problems from the normal activity of the system,
 only from smartd's sg_ioctl.  My impression was that it's a libata
 issue, because it's going through ata_pio_sector, which does 
 
   ap-ops-data_xfer(qc-dev, buf + offset, qc-sect_size, do_write);
 
 referring to sect_size, without considering the possibility of any smaller
 I/O size.  (Me, I don't even know why it's going PIO rather than DMA:
 I'm assuming smartd does things that way, but there's no limit to my
 ignorance here.)

Actually, I don't think it's a smaller I/O issue.  The SMART protocol
specifically mandates that the transfers for SMART READ DATA and SMART
READ LOG shall be 512 bytes).  However, the pio transfer routine does
seem to be assuming sector alignment as well, which will be where your
problems are coming from.  I think we need to specify sector minimum
alignment for ata (but not atapi, which has its own non sector size pio
routine).  How about the attached?

We have to do this for all ATA devices, because they'll likely all
support SMART, and SMART is defined to be a PIO command.

James

---

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 4bb268b..bc5cf6b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -824,9 +824,6 @@ static void ata_scsi_sdev_config(struct scsi_device *sdev)
 * requests.
 */
sdev-max_device_blocked = 1;
-
-   /* set the min alignment */
-   blk_queue_update_dma_alignment(sdev-request_queue, ATA_DMA_PAD_SZ - 1);
 }
 
 static void ata_scsi_dev_config(struct scsi_device *sdev,
@@ -842,7 +839,14 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
if (dev-class == ATA_DEV_ATAPI) {
struct request_queue *q = sdev-request_queue;
blk_queue_max_hw_segments(q, q-max_hw_segments - 1);
-   }
+
+   /* set the min alignment */
+   blk_queue_update_dma_alignment(sdev-request_queue,
+  ATA_DMA_PAD_SZ - 1);
+   } else
+   /* ATA devices must be sector aligned */
+   blk_queue_update_dma_alignment(sdev-request_queue,
+  ATA_SECT_SIZE - 1);
 
if (dev-flags  ATA_DFLAG_AN)
set_bit(SDEV_EVT_MEDIA_CHANGE, sdev-supported_events);


  However, I'd like to see if we can track the problem through the SG_IO
  direct path ... how many adjacent page bytes are corrupt?  Just a few or
  a large number (I'm wondering if it's an off by one or off by alignment
  type bug)?
 
 I've assumed it's just the one next page: because ata_pio_sector is
 doing a data_xfer of sect_size ATA_SECT_SIZE 512 to an offset above
 0xe00 in the smartd stack page.  The time I actually saw corruption
 rather than an oops at startup, it was in a tmpfs swap vector page
 running 64-bit kernel, and I didn't examine any further pages (just
 checked the page before and matched it up to smartd's stack, already
 suspecting that).
 
 I don't believe it's an off-by-one at your SCSI end.
 
 Hugh
 -
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

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


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread Hugh Dickins
On Tue, 22 Jan 2008, James Bottomley wrote:
 
 Actually, I don't think it's a smaller I/O issue.  The SMART protocol
 specifically mandates that the transfers for SMART READ DATA and SMART
 READ LOG shall be 512 bytes).  However, the pio transfer routine does
 seem to be assuming sector alignment as well, which will be where your
 problems are coming from.  I think we need to specify sector minimum
 alignment for ata (but not atapi, which has its own non sector size pio
 routine).  How about the attached?
 
 We have to do this for all ATA devices, because they'll likely all
 support SMART, and SMART is defined to be a PIO command.

Thanks, you've answered several of my uncertainties (why the PIO,
why the sector size).

I've just tried it, and can confirm that your replacement patch
below fixes the issue for me in practice.

What I can't say, you and Jeff and others will judge, is whether that's
actually the right placement for the blk_queue_update_dma_alignment call
(as an outsider, I'm not convinced that the SMART requirement should be
imposing this limitation on the rest).

Hugh

 diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
 index 4bb268b..bc5cf6b 100644
 --- a/drivers/ata/libata-scsi.c
 +++ b/drivers/ata/libata-scsi.c
 @@ -824,9 +824,6 @@ static void ata_scsi_sdev_config(struct scsi_device *sdev)
* requests.
*/
   sdev-max_device_blocked = 1;
 -
 - /* set the min alignment */
 - blk_queue_update_dma_alignment(sdev-request_queue, ATA_DMA_PAD_SZ - 1);
  }
  
  static void ata_scsi_dev_config(struct scsi_device *sdev,
 @@ -842,7 +839,14 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
   if (dev-class == ATA_DEV_ATAPI) {
   struct request_queue *q = sdev-request_queue;
   blk_queue_max_hw_segments(q, q-max_hw_segments - 1);
 - }
 +
 + /* set the min alignment */
 + blk_queue_update_dma_alignment(sdev-request_queue,
 +ATA_DMA_PAD_SZ - 1);
 + } else
 + /* ATA devices must be sector aligned */
 + blk_queue_update_dma_alignment(sdev-request_queue,
 +ATA_SECT_SIZE - 1);
  
   if (dev-flags  ATA_DFLAG_AN)
   set_bit(SDEV_EVT_MEDIA_CHANGE, sdev-supported_events);
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] fix /proc/scsi/sg/devices when no SCSI devices

2008-01-22 Thread Tony Battersby
The patch [SCSI] sg: use idr to replace static arrays in 2.6.24-rc1
causes a bogus line to appear in /proc/scsi/sg/devices containing
-1 -1 -1 -1 -1 -1 -1 -1 -1 when there are no SCSI devices in the
system.  In 2.6.23, /proc/scsi/sg/devices is empty when there are no
SCSI devices in the system.  A similar problem exists with
/proc/scsi/sg/device_strs.  The following patch restores the behavior
of 2.6.23.

Signed-off-by: Tony Battersby [EMAIL PROTECTED]
---
--- linux-2.6.24-rc8-git5/drivers/scsi/sg.c.orig2008-01-22 
15:08:46.0 -0500
+++ linux-2.6.24-rc8-git5/drivers/scsi/sg.c 2008-01-22 15:09:05.0 
-0500
@@ -2521,7 +2521,7 @@ sg_idr_max_id(int id, void *p, void *dat
 static int
 sg_last_dev(void)
 {
-   int k = 0;
+   int k = -1;
unsigned long iflags;
 
read_lock_irqsave(sg_index_lock, iflags);


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


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread Jeff Garzik

James Bottomley wrote:

Actually, I don't think it's a smaller I/O issue.  The SMART protocol
specifically mandates that the transfers for SMART READ DATA and SMART
READ LOG shall be 512 bytes).  However, the pio transfer routine does
seem to be assuming sector alignment as well, which will be where your
problems are coming from.  I think we need to specify sector minimum
alignment for ata (but not atapi, which has its own non sector size pio
routine).  How about the attached?

We have to do this for all ATA devices, because they'll likely all
support SMART, and SMART is defined to be a PIO command.

James

---

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 4bb268b..bc5cf6b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -824,9 +824,6 @@ static void ata_scsi_sdev_config(struct scsi_device *sdev)
 * requests.
 */
sdev-max_device_blocked = 1;
-
-   /* set the min alignment */
-   blk_queue_update_dma_alignment(sdev-request_queue, ATA_DMA_PAD_SZ - 1);
 }
 
 static void ata_scsi_dev_config(struct scsi_device *sdev,

@@ -842,7 +839,14 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
if (dev-class == ATA_DEV_ATAPI) {
struct request_queue *q = sdev-request_queue;
blk_queue_max_hw_segments(q, q-max_hw_segments - 1);
-   }
+
+   /* set the min alignment */
+   blk_queue_update_dma_alignment(sdev-request_queue,
+  ATA_DMA_PAD_SZ - 1);
+   } else
+   /* ATA devices must be sector aligned */
+   blk_queue_update_dma_alignment(sdev-request_queue,
+  ATA_SECT_SIZE - 1);
 
 	if (dev-flags  ATA_DFLAG_AN)

set_bit(SDEV_EVT_MEDIA_CHANGE, sdev-supported_events);



ACK

Unlike ATAPI, ATA is indeed all 512-byte alignment transfers (_not_ 
sector size, which may or may not be 512 bytes)


Does this apply to libata?  libata + jejb dma alignment patch?

What tree...

Jeff



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


Re: [PATCH] [SCSI]: constify function pointer tables

2008-01-22 Thread James Smart

Jan Engelhardt wrote:

Signed-off-by: Jan Engelhardt [EMAIL PROTECTED]
---
 drivers/scsi/lpfc/lpfc_debugfs.c |   10 +-
 drivers/scsi/sg.c|8 


ACK

-- james s

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


RE: Megaraid: Use of outb_p

2008-01-22 Thread Yang, Bo
Alan/Matthew,

I found inb_p/outb_p are defined as inb/outb in kernel src.  So it
should not have problems to change inb_p/outb_p to inb/outb.

Thanks.

Bo Yang 


-Original Message-
From: Matthew Wilcox [mailto:[EMAIL PROTECTED] 
Sent: Friday, January 18, 2008 3:41 PM
To: Yang, Bo
Cc: Alan Cox; linux-scsi@vger.kernel.org; DL-MegaRAID Linux
Subject: Re: Megaraid: Use of outb_p

On Fri, Jan 18, 2008 at 01:32:12PM -0700, Yang, Bo wrote:
 Alan,
 
 The in/outb_p in MegaRAID scsi driver is used for our old io mapped 
 megaraid controller.  There are still some customers are using those 
 old controller.  Please keep them.

Hi Bo,

I think you've misunderstood the question.  The '_p' versions of
inb/outb introduce a delay after each access.  The current Linux
implementation of _p provokes bugs on some chipsets, so Alan is looking
for alternatives.  Let's ask the question like this:

Replacing outb_p() with outb(); udelay(x); How large does 'x' need to be
for these megaraid controllers?

 Thanks.
  
 Bo Yang
 
 -Original Message-
 From: Alan Cox [mailto:[EMAIL PROTECTED]
 Sent: Friday, January 18, 2008 7:36 AM
 To: linux-scsi@vger.kernel.org; DL-MegaRAID Linux
 Subject: Megaraid: Use of outb_p
 
 
 I notice the MegaRAID driver uses outb_p. Can someone at LSI confirm 
 that the delays between each I/O are required, and if so how long they

 must be.
 
 I'm trying to sort out the use of in/outb_p and where it is 
 unneccessary or used for non ISA devices.
 
 (Please cc me on the reply)
 Alan
 
 -
 To unsubscribe from this list: send the line unsubscribe linux-scsi 
 in the body of a message to [EMAIL PROTECTED] More majordomo 
 info at  http://vger.kernel.org/majordomo-info.html

--
Intel are signing my paycheques ... these opinions are still mine Bill,
look, we understand that you're interested in selling us this operating
system, but compare it to ours.  We can't possibly take such a
retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Megaraid: Use of outb_p

2008-01-22 Thread Matthew Wilcox
On Tue, Jan 22, 2008 at 02:30:12PM -0700, Yang, Bo wrote:
 Alan/Matthew,
 
 I found inb_p/outb_p are defined as inb/outb in kernel src.  So it
 should not have problems to change inb_p/outb_p to inb/outb.

That's not true for x86-32.  Please, can you look up the documentation
for this chip and find out what sort of delay it needs?

-- 
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Megaraid: Use of outb_p

2008-01-22 Thread Yang, Bo

Alan/Matthew,

I will double check and give you back soon.

Regards.

Bo Yang 

-Original Message-
From: Matthew Wilcox [mailto:[EMAIL PROTECTED] 
Sent: Tuesday, January 22, 2008 4:38 PM
To: Yang, Bo
Cc: Alan Cox; linux-scsi@vger.kernel.org; DL-MegaRAID Linux
Subject: Re: Megaraid: Use of outb_p

On Tue, Jan 22, 2008 at 02:30:12PM -0700, Yang, Bo wrote:
 Alan/Matthew,
 
 I found inb_p/outb_p are defined as inb/outb in kernel src.  So it 
 should not have problems to change inb_p/outb_p to inb/outb.

That's not true for x86-32.  Please, can you look up the documentation
for this chip and find out what sort of delay it needs?

--
Intel are signing my paycheques ... these opinions are still mine Bill,
look, we understand that you're interested in selling us this operating
system, but compare it to ours.  We can't possibly take such a
retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] fix /proc/scsi/sg/devices when no SCSI devices

2008-01-22 Thread Douglas Gilbert

Tony Battersby wrote:

The patch [SCSI] sg: use idr to replace static arrays in 2.6.24-rc1
causes a bogus line to appear in /proc/scsi/sg/devices containing
-1 -1 -1 -1 -1 -1 -1 -1 -1 when there are no SCSI devices in the
system.  In 2.6.23, /proc/scsi/sg/devices is empty when there are no
SCSI devices in the system.  A similar problem exists with
/proc/scsi/sg/device_strs.  The following patch restores the behavior
of 2.6.23.

Signed-off-by: Tony Battersby [EMAIL PROTECTED]
---
--- linux-2.6.24-rc8-git5/drivers/scsi/sg.c.orig2008-01-22 
15:08:46.0 -0500
+++ linux-2.6.24-rc8-git5/drivers/scsi/sg.c 2008-01-22 15:09:05.0 
-0500
@@ -2521,7 +2521,7 @@ sg_idr_max_id(int id, void *p, void *dat
 static int
 sg_last_dev(void)
 {
-   int k = 0;
+   int k = -1;
unsigned long iflags;
 
 	read_lock_irqsave(sg_index_lock, iflags);



-


Tony,
Thanks.

Signed-off-by: Douglas Gilbert [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread James Bottomley
On Tue, 2008-01-22 at 20:20 +, Hugh Dickins wrote:
 On Tue, 22 Jan 2008, James Bottomley wrote:
  
  Actually, I don't think it's a smaller I/O issue.  The SMART protocol
  specifically mandates that the transfers for SMART READ DATA and SMART
  READ LOG shall be 512 bytes).  However, the pio transfer routine does
  seem to be assuming sector alignment as well, which will be where your
  problems are coming from.  I think we need to specify sector minimum
  alignment for ata (but not atapi, which has its own non sector size pio
  routine).  How about the attached?
  
  We have to do this for all ATA devices, because they'll likely all
  support SMART, and SMART is defined to be a PIO command.
 
 Thanks, you've answered several of my uncertainties (why the PIO,
 why the sector size).
 
 I've just tried it, and can confirm that your replacement patch
 below fixes the issue for me in practice.

Thanks!

 What I can't say, you and Jeff and others will judge, is whether that's
 actually the right placement for the blk_queue_update_dma_alignment call
 (as an outsider, I'm not convinced that the SMART requirement should be
 imposing this limitation on the rest).

It's certainly the correct placement.  The slight problem is that the
actual alignment checking is only really done for commands coming down
from the user, not for commands incoming from the kernel.  That leaves
us a potential nasty in IDENTIFY_DEVICE; that's also a PIO only 512 byte
transfer command.

libsas looks to be OK because it specifically kmallocs a 512 byte buffer
which should (for off slab data) be 512 byte aligned.  libata actually
has an issue because the usual location for IDENTIFY_DEVICE data is
inside a struct ata_device, which is highly unlikely to be correctly
aligned.  Fortunately, I think we can only get the bug if we actually
cross a page boundary for non contiguous pages in the identify data,
which a kernel allocation will never do, so libata should be safe as
well.

James


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


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread Alan Cox
 However, I'd like to see if we can track the problem through the SG_IO
 direct path ... how many adjacent page bytes are corrupt?  Just a few or
 a large number (I'm wondering if it's an off by one or off by alignment
 type bug)?

Which ATA controller is involved - in theory ATA DMA is byte aligned safe
(or dword anyway) in practice I don't know if we've ever tested the non
512 byte aligned case historically for ATA just ATAPI ?

Alan
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread Hugh Dickins
On Tue, 22 Jan 2008, Alan Cox wrote:
  However, I'd like to see if we can track the problem through the SG_IO
  direct path ... how many adjacent page bytes are corrupt?  Just a few or
  a large number (I'm wondering if it's an off by one or off by alignment
  type bug)?

We moved away from that concern 

 Which ATA controller is involved - in theory ATA DMA is byte aligned safe
 (or dword anyway) in practice I don't know if we've ever tested the non
 512 byte aligned case historically for ATA just ATAPI ?

 but if it's still relevant, this was with ata_piix.

Hugh
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread Hugh Dickins
On Tue, 22 Jan 2008, James Bottomley wrote:
 
 libsas looks to be OK because it specifically kmallocs a 512 byte buffer
 which should (for off slab data) be 512 byte aligned.

I don't remember the various SLAB and SLOB and SLUB rules offhand:
I'm not sure it's safe to rely on such alignment on all of them 

 libata actually
 has an issue because the usual location for IDENTIFY_DEVICE data is
 inside a struct ata_device, which is highly unlikely to be correctly
 aligned.  Fortunately, I think we can only get the bug if we actually
 cross a page boundary for non contiguous pages in the identify data,
 which a kernel allocation will never do, so libata should be safe as
 well.

 but this would trump it: yes, we don't need 512-byte alignment
for this, and it is okay to cross a page boundary, just so long as
the start of the next page really belongs to our buffer not somebody
else's.

There doesn't seem much likelihood of anyone vmalloc'ing the buffer
in which that IDENTIFY_DEVICE gets done.  Though this discussion
does make me wonder whether ata_pio_sector ought to have a BUG_ON
(and yes, a BUG_ON rather than a WARN_ON) against the possibility.

Hugh
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread James Bottomley
On Tue, 2008-01-22 at 15:32 -0500, Jeff Garzik wrote:
 James Bottomley wrote:
  Actually, I don't think it's a smaller I/O issue.  The SMART protocol
  specifically mandates that the transfers for SMART READ DATA and SMART
  READ LOG shall be 512 bytes).  However, the pio transfer routine does
  seem to be assuming sector alignment as well, which will be where your
  problems are coming from.  I think we need to specify sector minimum
  alignment for ata (but not atapi, which has its own non sector size pio
  routine).  How about the attached?
  
  We have to do this for all ATA devices, because they'll likely all
  support SMART, and SMART is defined to be a PIO command.
  
  James
  
  ---
  
  diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
  index 4bb268b..bc5cf6b 100644
  --- a/drivers/ata/libata-scsi.c
  +++ b/drivers/ata/libata-scsi.c
  @@ -824,9 +824,6 @@ static void ata_scsi_sdev_config(struct scsi_device 
  *sdev)
   * requests.
   */
  sdev-max_device_blocked = 1;
  -
  -   /* set the min alignment */
  -   blk_queue_update_dma_alignment(sdev-request_queue, ATA_DMA_PAD_SZ - 1);
   }
   
   static void ata_scsi_dev_config(struct scsi_device *sdev,
  @@ -842,7 +839,14 @@ static void ata_scsi_dev_config(struct scsi_device 
  *sdev,
  if (dev-class == ATA_DEV_ATAPI) {
  struct request_queue *q = sdev-request_queue;
  blk_queue_max_hw_segments(q, q-max_hw_segments - 1);
  -   }
  +
  +   /* set the min alignment */
  +   blk_queue_update_dma_alignment(sdev-request_queue,
  +  ATA_DMA_PAD_SZ - 1);
  +   } else
  +   /* ATA devices must be sector aligned */
  +   blk_queue_update_dma_alignment(sdev-request_queue,
  +  ATA_SECT_SIZE - 1);
   
  if (dev-flags  ATA_DFLAG_AN)
  set_bit(SDEV_EVT_MEDIA_CHANGE, sdev-supported_events);
 
 
 ACK
 
 Unlike ATAPI, ATA is indeed all 512-byte alignment transfers (_not_ 
 sector size, which may or may not be 512 bytes)
 
 Does this apply to libata?  libata + jejb dma alignment patch?
 
 What tree...

It's scsi-misc-2.6; the blk_queue_update_dma_alignment() API doesn't
exist in mainline (yet).

James


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


Re: [PATCH 1/3] scsi_debug: add get_data_transfer_info helper function

2008-01-22 Thread Douglas Gilbert

FUJITA Tomonori wrote:

This adds get_data_transfer_info helper function that get lha and
sectors for READ_* and WRITE_* commands (and XDWRITEREAD_10 later).

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_debug.c |   83 
 1 files changed, 38 insertions(+), 45 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 82c06f0..31f7378 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -311,12 +311,47 @@ static void sdebug_max_tgts_luns(void);
 static struct device pseudo_primary;
 static struct bus_type pseudo_lld_bus;
 
+static void get_data_transfer_info(unsigned char *cmd,

+  unsigned long long *lba, unsigned int *num)
+{
+   int i;
+
+   switch (*cmd) {
+   case WRITE_16:
+   case READ_16:
+   for (*lba = 0, i = 0; i  8; ++i) {
+   if (i  0)
+   *lba = 8;
+   *lba += cmd[2 + i];
+   }
+   *num = cmd[13] + (cmd[12]  8) +
+   (cmd[11]  16) + (cmd[10]  24);
+   break;
+   case WRITE_12:
+   case READ_12:
+   *lba = cmd[5] + (cmd[4]  8) +   (cmd[3]  16) + (cmd[2]  
24);
+   *num = cmd[9] + (cmd[8]  8) +   (cmd[7]  16) + (cmd[6]  
24);
+   break;
+   case WRITE_10:
+   case READ_10:
+   *lba = cmd[5] + (cmd[4]  8) +   (cmd[3]  16) + (cmd[2]  
24);
+   *num = cmd[8] + (cmd[7]  8);
+   break;
+   case WRITE_6:
+   case READ_6:
+   *lba = cmd[3] + (cmd[2]  8) + ((cmd[1]  0x1f)  16);
+   *num = (0 == cmd[4]) ? 256 : cmd[4];
+   break;
+   default:
+   break;
+   }
+}
 
 static

 int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, done_funct_t done)
 {
unsigned char *cmd = (unsigned char *) SCpnt-cmnd;
-   int len, k, j;
+   int len, k;
unsigned int num;
unsigned long long lba;
int errsts = 0;
@@ -452,28 +487,7 @@ int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, 
done_funct_t done)
break;
if (scsi_debug_fake_rw)
break;
-   if ((*cmd) == READ_16) {
-   for (lba = 0, j = 0; j  8; ++j) {
-   if (j  0)
-   lba = 8;
-   lba += cmd[2 + j];
-   }
-   num = cmd[13] + (cmd[12]  8) +
-   (cmd[11]  16) + (cmd[10]  24);
-   } else if ((*cmd) == READ_12) {
-   lba = cmd[5] + (cmd[4]  8) +
-   (cmd[3]  16) + (cmd[2]  24);
-   num = cmd[9] + (cmd[8]  8) +
-   (cmd[7]  16) + (cmd[6]  24);
-   } else if ((*cmd) == READ_10) {
-   lba = cmd[5] + (cmd[4]  8) +
-   (cmd[3]  16) + (cmd[2]  24);
-   num = cmd[8] + (cmd[7]  8);
-   } else {/* READ (6) */
-   lba = cmd[3] + (cmd[2]  8) +
-   ((cmd[1]  0x1f)  16);
-   num = (0 == cmd[4]) ? 256 : cmd[4];
-   }
+   get_data_transfer_info(cmd, lba, num);
errsts = resp_read(SCpnt, lba, num, devip);
if (inj_recovered  (0 == errsts)) {
mk_sense_buffer(devip, RECOVERED_ERROR,
@@ -500,28 +514,7 @@ int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, 
done_funct_t done)
break;
if (scsi_debug_fake_rw)
break;
-   if ((*cmd) == WRITE_16) {
-   for (lba = 0, j = 0; j  8; ++j) {
-   if (j  0)
-   lba = 8;
-   lba += cmd[2 + j];
-   }
-   num = cmd[13] + (cmd[12]  8) +
-   (cmd[11]  16) + (cmd[10]  24);
-   } else if ((*cmd) == WRITE_12) {
-   lba = cmd[5] + (cmd[4]  8) +
-   (cmd[3]  16) + (cmd[2]  24);
-   num = cmd[9] + (cmd[8]  8) +
-   (cmd[7]  16) + (cmd[6]  24);
-   } else if ((*cmd) == WRITE_10) {
-   lba = cmd[5] + (cmd[4]  8) +
-   (cmd[3]  16) + (cmd[2]  24);
-   num = cmd[8] + (cmd[7]  8);
-   } else {/* WRITE (6) */
-   lba = cmd[3] + (cmd[2]  8) +
-   ((cmd[1]  0x1f)  16);
-   num = (0 == cmd[4]) ? 256 : cmd[4];
-   }
+   

Re: [PATCH 2/3] scsi_debug: add bidi data transfer support

2008-01-22 Thread Douglas Gilbert

FUJITA Tomonori wrote:

This enables fill_from_dev_buffer and fetch_to_dev_buffer to handle
bidi commands.

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_debug.c |   21 ++---
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 31f7378..d810aa7 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -594,18 +594,18 @@ static int fill_from_dev_buffer(struct scsi_cmnd * scp, 
unsigned char * arr,
int k, req_len, act_len, len, active;
void * kaddr;
void * kaddr_off;
-   struct scatterlist * sg;
+   struct scatterlist *sg;
+   struct scsi_data_buffer *sdb = scsi_in(scp);
 
-	if (0 == scsi_bufflen(scp))

+   if (!sdb-length)
return 0;
-   if (NULL == scsi_sglist(scp))
+   if (!sdb-table.sgl)
return (DID_ERROR  16);
-   if (! ((scp-sc_data_direction == DMA_BIDIRECTIONAL) ||
- (scp-sc_data_direction == DMA_FROM_DEVICE)))
+   if (!(scsi_bidi_cmnd(scp) || scp-sc_data_direction == DMA_FROM_DEVICE))
return (DID_ERROR  16);
active = 1;
req_len = act_len = 0;
-   scsi_for_each_sg(scp, sg, scsi_sg_count(scp), k) {
+   for_each_sg(sdb-table.sgl, sg, sdb-table.nents, k) {
if (active) {
kaddr = (unsigned char *)
kmap_atomic(sg_page(sg), KM_USER0);
@@ -623,10 +623,10 @@ static int fill_from_dev_buffer(struct scsi_cmnd * scp, 
unsigned char * arr,
}
req_len += sg-length;
}
-   if (scsi_get_resid(scp))
-   scsi_set_resid(scp, scsi_get_resid(scp) - act_len);
+   if (sdb-resid)
+   sdb-resid -= act_len;
else
-   scsi_set_resid(scp, req_len - act_len);
+   sdb-resid = req_len - act_len;
return 0;
 }
 
@@ -643,8 +643,7 @@ static int fetch_to_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,

return 0;
if (NULL == scsi_sglist(scp))
return -1;
-   if (! ((scp-sc_data_direction == DMA_BIDIRECTIONAL) ||
- (scp-sc_data_direction == DMA_TO_DEVICE)))
+   if (!(scsi_bidi_cmnd(scp) || scp-sc_data_direction == DMA_TO_DEVICE))
return -1;
req_len = fin = 0;
scsi_for_each_sg(scp, sg, scsi_sg_count(scp), k) {


Signed-off-by: Douglas Gilbert [EMAIL PROTECTED]

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


Re: [PATCH 3/3] scsi_debug: add XDWRITEREAD_10 support

2008-01-22 Thread Douglas Gilbert

FUJITA Tomonori wrote:

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_debug.c |   70 +
 include/scsi/scsi.h   |1 +
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d810aa7..1541c17 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -280,6 +280,8 @@ static int resp_write(struct scsi_cmnd * SCpnt, unsigned 
long long lba,
  unsigned int num, struct sdebug_dev_info * devip);
 static int resp_report_luns(struct scsi_cmnd * SCpnt,
struct sdebug_dev_info * devip);
+static int resp_xdwriteread(struct scsi_cmnd *scp, unsigned long long lba,
+   unsigned int num, struct sdebug_dev_info *devip);
 static int fill_from_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,
 int arr_len);
 static int fetch_to_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,
@@ -334,6 +336,7 @@ static void get_data_transfer_info(unsigned char *cmd,
break;
case WRITE_10:
case READ_10:
+   case XDWRITEREAD_10:
*lba = cmd[5] + (cmd[4]  8) +   (cmd[3]  16) + (cmd[2]  
24);
*num = cmd[8] + (cmd[7]  8);
break;
@@ -542,6 +545,28 @@ int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, 
done_funct_t done)
case WRITE_BUFFER:
errsts = check_readiness(SCpnt, 1, devip);
break;
+   case XDWRITEREAD_10:
+   if (!scsi_bidi_cmnd(SCpnt)) {
+   mk_sense_buffer(devip, ILLEGAL_REQUEST,
+   INVALID_FIELD_IN_CDB, 0);
+   errsts = check_condition_result;
+   break;
+   }
+
+   errsts = check_readiness(SCpnt, 0, devip);
+   if (errsts)
+   break;
+   if (scsi_debug_fake_rw)
+   break;
+   get_data_transfer_info(cmd, lba, num);
+   errsts = resp_read(SCpnt, lba, num, devip);
+   if (errsts)
+   break;
+   errsts = resp_write(SCpnt, lba, num, devip);
+   if (errsts)
+   break;
+   errsts = resp_xdwriteread(SCpnt, lba, num, devip);
+   break;
default:
if (SCSI_DEBUG_OPT_NOISE  scsi_debug_opts)
printk(KERN_INFO scsi_debug: Opcode: 0x%x not 
@@ -1948,6 +1973,50 @@ static int resp_report_luns(struct scsi_cmnd * scp,
min((int)alloc_len, SDEBUG_RLUN_ARR_SZ));
 }
 
+static int resp_xdwriteread(struct scsi_cmnd *scp, unsigned long long lba,

+   unsigned int num, struct sdebug_dev_info *devip)
+{
+   int i, j, ret = -1;
+   unsigned char *kaddr, *buf;
+   unsigned int offset;
+   struct scatterlist *sg;
+   struct scsi_data_buffer *sdb = scsi_in(scp);
+
+   /* better not to use temporary buffer. */
+   buf = kmalloc(scsi_bufflen(scp), GFP_ATOMIC);
+   if (!buf)
+   return ret;
+
+   offset = 0;
+   scsi_for_each_sg(scp, sg, scsi_sg_count(scp), i) {
+   kaddr = (unsigned char *)kmap_atomic(sg_page(sg), KM_USER0);
+   if (!kaddr)
+   goto out;
+
+   memcpy(buf + offset, kaddr + sg-offset, sg-length);
+   offset += sg-length;
+   kunmap_atomic(kaddr, KM_USER0);
+   }
+
+   offset = 0;
+   for_each_sg(sdb-table.sgl, sg, sdb-table.nents, i) {
+   kaddr = (unsigned char *)kmap_atomic(sg_page(sg), KM_USER0);
+   if (!kaddr)
+   goto out;
+
+   for (j = 0; j  sg-length; j++)
+   *(kaddr + sg-offset + j) ^= *(buf + offset + j);
+
+   offset += sg-length;
+   kunmap_atomic(kaddr, KM_USER0);
+   }
+   ret = 0;
+out:
+   kfree(buf);
+
+   return ret;
+}
+
 /* When timer goes off this function is called. */
 static void timer_intr_handler(unsigned long indx)
 {
@@ -1981,6 +2050,7 @@ static int scsi_debug_slave_alloc(struct scsi_device * 
sdp)
if (SCSI_DEBUG_OPT_NOISE  scsi_debug_opts)
printk(KERN_INFO scsi_debug: slave_alloc %u %u %u %u\n,
   sdp-host-host_no, sdp-channel, sdp-id, sdp-lun);
+   set_bit(QUEUE_FLAG_BIDI, sdp-request_queue-queue_flags);
return 0;
 }
 
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h

index 056c5af..19ca9e9 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -102,6 +102,7 @@ extern const unsigned char scsi_command_size[8];
 #define READ_TOC  0x43
 #define LOG_SELECT0x4c
 #define LOG_SENSE 0x4d
+#define XDWRITEREAD_100x53
 #define MODE_SELECT_10

Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread James Bottomley

On Tue, 2008-01-22 at 22:59 +, Hugh Dickins wrote:
 On Tue, 22 Jan 2008, James Bottomley wrote:
  
  libsas looks to be OK because it specifically kmallocs a 512 byte buffer
  which should (for off slab data) be 512 byte aligned.
 
 I don't remember the various SLAB and SLOB and SLUB rules offhand:
 I'm not sure it's safe to rely on such alignment on all of them 
 
  libata actually
  has an issue because the usual location for IDENTIFY_DEVICE data is
  inside a struct ata_device, which is highly unlikely to be correctly
  aligned.  Fortunately, I think we can only get the bug if we actually
  cross a page boundary for non contiguous pages in the identify data,
  which a kernel allocation will never do, so libata should be safe as
  well.
 
  but this would trump it: yes, we don't need 512-byte alignment
 for this, and it is okay to cross a page boundary, just so long as
 the start of the next page really belongs to our buffer not somebody
 else's.
 
 There doesn't seem much likelihood of anyone vmalloc'ing the buffer
 in which that IDENTIFY_DEVICE gets done.  Though this discussion
 does make me wonder whether ata_pio_sector ought to have a BUG_ON
 (and yes, a BUG_ON rather than a WARN_ON) against the possibility.

Oh ... never say never.  One of the things the shift we did in SCSI to
the block submission api and elimination of the single map path actually
enables you to send kernel vmalloc areas straight into the storage APIs
(which was previously absolutely forbidden) ... fortunately no-one has
yet.

However, I think you're right we could do with a check in the PIO path
to make sure each sector is physically contiguous ... unfortunately, an
easy alignment check would trip on the IDENTIFY and other kernel
commands, sigh.

James


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


RE: Open-FCoE on linux-scsi

2008-01-22 Thread Love, Robert W


-Original Message-
From: James Smart [mailto:[EMAIL PROTECTED]
Sent: Tuesday, January 15, 2008 2:19 PM
To: Love, Robert W
Cc: Stefan Richter; Dev, Vasu; FUJITA Tomonori; [EMAIL PROTECTED]; Zou, Yi;
Leech, Christopher; linux-scsi@vger.kernel.org; James Smart
Subject: Re: Open-FCoE on linux-scsi

Love, Robert W wrote:
 The interconnect layer could be split further:
 SCSI command set layer -- SCSI core -- SCSI transport layer (FCP) --
 Fibre Channel core -- Fibre Channel card drivers, FCoE drivers.

 This is how I see the comparison. ('/' indicates 'or')

 You suggest  Open-FCoE
 SCSI-ml  SCSI-ml
 scsi_transport_fc.h  scsi_tranport_fc.h
 scsi_transport_fc.c (FC core) / HBA  openfc / HBA
 fcoe / HBA   fcoe / HBA

From what I can see the layering is roughly the same with the main
 difference being that we should be using more of (and putting more
into)
 scsi_transport_fc.h. Also we should make the FCP implementation
(openfc)
 fit in a bit nicer as scsi_transport_fc.c. We're going to look into
 making better use of scsi_transport_fc.h as a first step.

I don't know what the distinction is between scsi_transport_fc.h vs
scsi_transport_fc.c is. They're all one and the same - the fc
transport.
One contains the data structures and api between LLD and transport,
the other (the .c) contains the code to implement the api, transport
objects
and sysfs handlers.

 From my point of view, the fc transport is an assist library for the
FC
LLDDs.
Currently, it interacts with the midlayer only around some scan and
block/unblock
functions. Excepting a small helper function used by the LLDD, it does
not
get
involved in the i/o path.

So my view of the layering for a normal FC driver is:
SCSI-ml
LLDD - FC transport
bus code (e.g. pci)

Right now, the assists provided in the FC transport are:
- Presentation of transport objects into the sysfs tree, and thus sysfs
   attribute handling around those objects. This effectively is the FC
   management interface.
- Remote Port Object mgmt - interaction with the midlayer.
Specifically:
   - Manages the SCSI target id bindings for the remote port
   - Knows when the rport is present or not.
 On new connectivity:
   Kicks off scsi scans, restarts blocked i/o.
 On connectivity loss:
   Insulates midlayer from temporary disconnects by block of
 the target/luns, and manages the timer for the allowed period
of
 disconnect.
   Assists in knowing when/how to terminate pending i/o after a
 connectivity loss (fast fail, or wait).
   - Provides consistent error codes for i/o path and error handlers
via
 helpers that are used by LLDD.

Note that the above does not contain the FC login state machine, etc.
We have discussed this in the past. Given the 4 FC LLDDs we had, there
was
a wide difference on who did what where. LSI did all login and FC ELS
handling in their firmware. Qlogic did the initiation of the login in
the
driver, but the ELS handling in the firmware. Emulex did the ELS
handling
in the driver. IBM/zfcp runs a hybrid of login/ELS handling over it's
pseudo
hba interface. Knowing how much time we spend constantly debugging
login/ELS
handling and the fact that we have to interject adapter resource
allocation
steps into the statemachine, I didn't want to go to a common library
until
there was a very clear and similar LLDD.  Well, you can't get much
clearer
than a full software-based login/ELS state machine that FCOE needs. It
makes
sense to at least try to library-ize the login/ELS handling if
possible.

Here's what I have in mind for FCOE layering. Keep in mind, that one of
the
goals here is to support a lot of different implementations which may
range
from s/w layers on a simple Ethernet packet pusher, to more and more
levels
of offload on an FCOE adapter. The goal is to create the s/w layers
such
that
different LLDD's can pick and choose the layer(s) (or level) they want
to
integrate into. At a minimum, they should/must integrate with the base
mgmt
objects.

For FC transport, we'd have the following layers or api sections :
Layer 0: rport and vport objects   (current functionality)
Layer 1: Port login and ELS handling
Layer 2: Fabric login, PT2PT login, CT handling, and discovery/RSCN
Layer 3: FCP I/O Assist
Layer 4: FC2 - Exchange and Sequence handling
Layer 5: FCOE encap/decap
Layer 6: FCOE FLOGI handler

Layer 1 would work with an api to the LLDD based on a send/receive ELS
interface
   coupled with a login/logout to address interface. The code within
layer
1
   would make calls to layer 0 to instantiate the different objects. If
layer 1
   needs to track additional rport data, it should specify dd_data on
the
   rport_add call. (Note: all of the LLDDs today have their own node
structure
   that is independent from the rport struct. I 

Re: [PATCH rc8-mm1] hotfix libata-scsi corruption

2008-01-22 Thread Matt Mackall

On Tue, 2008-01-22 at 22:59 +, Hugh Dickins wrote:
 On Tue, 22 Jan 2008, James Bottomley wrote:
  
  libsas looks to be OK because it specifically kmallocs a 512 byte buffer
  which should (for off slab data) be 512 byte aligned.
 
 I don't remember the various SLAB and SLOB and SLUB rules offhand:
 I'm not sure it's safe to rely on such alignment on all of them 

It doesn't work that way with SLOB kmalloc (nor did it in pre-slabified
kmalloc). One shouldn't be surprised if a SLAB/SLUB debugging feature
breaks that alignment either.

-- 
Mathematics is the supreme nostalgia of our time.

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


AACRAID driver broken in 2.6.22.x (and beyond?) [WAS: Re: 2.6.22.16 MD raid1 doesn't mark removed disk faulty, MD thread goes UN]

2008-01-22 Thread Mike Snitzer
On Jan 22, 2008 12:29 AM, Mike Snitzer [EMAIL PROTECTED] wrote:
 cc'ing Tanaka-san given his recent raid1 BUG report:
 http://lkml.org/lkml/2008/1/14/515


 On Jan 21, 2008 6:04 PM, Mike Snitzer [EMAIL PROTECTED] wrote:
  Under 2.6.22.16, I physically pulled a SATA disk (/dev/sdac, connected to
  an aacraid controller) that was acting as the local raid1 member of
  /dev/md30.
 
  Linux MD didn't see an /dev/sdac1 error until I tried forcing the issue by
  doing a read (with dd) from /dev/md30:

 The raid1d thread is locked at line 720 in raid1.c (raid1d+2437); aka
 freeze_array:

 (gdb) l *0x2539
 0x2539 is in raid1d (drivers/md/raid1.c:720).
 715  * wait until barrier+nr_pending match nr_queued+2
 716  */
 717 spin_lock_irq(conf-resync_lock);
 718 conf-barrier++;
 719 conf-nr_waiting++;
 720 wait_event_lock_irq(conf-wait_barrier,
 721 conf-barrier+conf-nr_pending ==
 conf-nr_queued+2,
 722 conf-resync_lock,
 723 raid1_unplug(conf-mddev-queue));
 724 spin_unlock_irq(conf-resync_lock);

 Given Tanaka-san's report against 2.6.23 and me hitting what seems to
 be the same deadlock in 2.6.22.16; it stands to reason this affects
 raid1 in 2.6.24-rcX too.

Turns out that the aacraid driver in 2.6.22.x is HORRIBLY BROKEN (when
you pull a drive); it responds to MD's write requests with uptodate=1
(in raid1_end_write_request) for the drive that was pulled!  I've not
looked to see if aacraid has been fixed in newer kernels... are others
aware of any crucial aacraid fixes in 2.6.23.x or 2.6.24?

After the drive was physically pulled, and small periodic writes
continued to the associated MD device, the raid1 MD driver did _NOT_
detect the pulled drive's writes as having failed (verified this with
systemtap).  MD happily thought the write completed to both members
(so MD had no reason to mark the pulled drive faulty; or mark the
raid degraded).

Installing an Adaptec-provided 1.1-5[2451] driver enabled raid1 to
work as expected.

That said, I now have a recipe for hitting the raid1 deadlock that
Tanaka first reported over a week ago.  I'm still surprised that all
of this chatter about that BUG hasn't drawn interest/scrutiny from
others!?

regards,
Mike
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 01/10] [FCoE] Changing all ASSERT_NOTIMPL to WARN_ON(1)

2008-01-22 Thread Robert Love
Trigger the warning every time the block is encountered.

Signed-off-by: Robert Love [EMAIL PROTECTED]
---

 drivers/scsi/ofc/libfc/fc_exch.c   |   11 +--
 drivers/scsi/ofc/libfc/fc_frame.c  |2 +-
 drivers/scsi/ofc/libfc/fc_local_port.c |2 +-
 drivers/scsi/ofc/openfc/openfc_scsi.c  |4 ++--
 4 files changed, 5 insertions(+), 14 deletions(-)


diff --git a/drivers/scsi/ofc/libfc/fc_exch.c b/drivers/scsi/ofc/libfc/fc_exch.c
index 4a19936..9a5cd98 100644
--- a/drivers/scsi/ofc/libfc/fc_exch.c
+++ b/drivers/scsi/ofc/libfc/fc_exch.c
@@ -203,14 +203,6 @@ static int fc_exch_mgr_init(struct fc_exch_mgr *mp, enum 
fc_class class,
u_int pool_count;
u_int cpu;
 
-   /*
-* Check to make sure the declaration of ESB and SSB structures came out
-* with the right size and no unexpected padding.
-*/
-   ASSERT_NOTIMPL(sizeof(struct fc_esb) == FC_ESB_SIZE);
-   ASSERT_NOTIMPL(sizeof(struct fc_ssb) == FC_SSB_SIZE);
-   ASSERT_NOTIMPL(sizeof(struct fc_frame_header) == FC_FRAME_HEADER_LEN);
-
mp-em_class = class;
 
/*
@@ -562,7 +554,6 @@ static struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp)
 
spin_lock_bh(pp-emp_lock);
if (list_empty(pp-emp_exch_free)) {
-   ASSERT_NOTIMPL(ep); /* not generally handled */
atomic_inc(mp-em_stats.ems_error_no_free_exch);
spin_unlock_bh(pp-emp_lock);
} else {
@@ -1144,7 +1135,7 @@ static void fc_seq_send_ack(struct fc_seq *sp, const 
struct fc_frame *rx_fp)
 */
if (fc_sof_needs_ack(rx_fp-fr_sof)) {
fp = fc_frame_alloc(fc_seq_exch(sp)-ex_port, 0);
-   ASSERT_NOTIMPL(fp);
+   BUG_ON(!fp);
if (!fp)
return;
fc_seq_fill_hdr(sp, fp);
diff --git a/drivers/scsi/ofc/libfc/fc_frame.c 
b/drivers/scsi/ofc/libfc/fc_frame.c
index 056df3d..c609ebd 100644
--- a/drivers/scsi/ofc/libfc/fc_frame.c
+++ b/drivers/scsi/ofc/libfc/fc_frame.c
@@ -44,7 +44,7 @@ u_int32_t fc_frame_crc_check(struct fc_frame *fp)
const u_int8_t *bp;
u_int len;
 
-   ASSERT_NOTIMPL(fc_frame_is_linear(fp));
+   WARN_ON(!fc_frame_is_linear(fp));
fp-fr_flags = ~FCPHF_CRC_UNCHECKED;
len = (fp-fr_len + 3)  ~3;/* round up length to include fill */
bp = (const u_int8_t *)fp-fr_hdr;
diff --git a/drivers/scsi/ofc/libfc/fc_local_port.c 
b/drivers/scsi/ofc/libfc/fc_local_port.c
index 311b5e9..cd70c74 100644
--- a/drivers/scsi/ofc/libfc/fc_local_port.c
+++ b/drivers/scsi/ofc/libfc/fc_local_port.c
@@ -1329,7 +1329,7 @@ static void fc_local_port_rscn_req(struct fc_seq *sp, 
struct fc_frame *fp,
if (fc_local_port_debug)
OFC_DBG(RSCN received: rediscovering);
error = fc_disc_targ_restart(lp);
-   ASSERT_NOTIMPL(error == 0);
+   WARN_ON(error != 0);
} else if (fc_local_port_debug) {
OFC_DBG(RSCN received: not rediscovering. 
   redisc %d state %d disc_cb %p in_prog %d,
diff --git a/drivers/scsi/ofc/openfc/openfc_scsi.c 
b/drivers/scsi/ofc/openfc/openfc_scsi.c
index cd3afb7..9c7f9bb 100644
--- a/drivers/scsi/ofc/openfc/openfc_scsi.c
+++ b/drivers/scsi/ofc/openfc/openfc_scsi.c
@@ -309,7 +309,7 @@ static void openfc_scsi_send_data(struct fc_scsi_pkt *fsp, 
struct fc_seq *sp,
 tlen);
data = (void *)(fp-fr_hdr + 1);
}
-   ASSERT_NOTIMPL(fp != NULL); /* XXX */
+   BUG_ON(!fp);
fc_frame_setup(fp, FC_RCTL_DD_SOL_DATA, FC_TYPE_FCP);
fc_frame_set_offset(fp, frame_offset);
}
@@ -442,7 +442,7 @@ static void openfc_scsi_rcv(struct fc_seq *sp, struct 
fc_frame *fp, void *arg)
 */
ASSERT(!(fp-fr_flags  FCPHF_CRC_UNCHECKED));
dd = fc_frame_payload_get(fp, sizeof(*dd));
-   ASSERT_NOTIMPL(dd != NULL);
+   WARN_ON(!dd);
fsp-state = OPENFC_SRB_IN_DATA_TRANS;
 
/*

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


[PATCH 03/10] [FCoE] Remove DEBUG_ASSERTS statements

2008-01-22 Thread Robert Love

---

 drivers/scsi/ofc/include/fc_els.h |   17 -
 drivers/scsi/ofc/include/fc_fs.h  |   13 -
 drivers/scsi/ofc/include/fc_gs.h  |   12 
 drivers/scsi/ofc/libfc/fc_ils.h   |   33 -
 drivers/scsi/ofc/libfc/fc_sess.c  |   14 --
 5 files changed, 0 insertions(+), 89 deletions(-)


diff --git a/drivers/scsi/ofc/include/fc_els.h 
b/drivers/scsi/ofc/include/fc_els.h
index 8ef10c0..6bff73e 100644
--- a/drivers/scsi/ofc/include/fc_els.h
+++ b/drivers/scsi/ofc/include/fc_els.h
@@ -808,21 +808,4 @@ enum fc_els_clid_ic {
ELS_CLID_IC_LIP =   8,  /* receiving LIP */
 };
 
-#ifdef DEBUG_ASSERTS
-/*
- * Static checks for packet structure sizes.
- * These catch some obvious errors in structure definitions.
- * They should generate no code since they can be tested at compile time.
- */
-static inline void fc_els_size_checks(void)
-{
-   ASSERT_NOTIMPL(sizeof(struct fc_els_csp) == FC_ELS_CSP_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_els_cssp) == FC_ELS_CSSP_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_els_flogi) == FC_ELS_FLOGI_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_els_spp) == FC_ELS_SPP_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_els_prli) == FC_ELS_PRLI_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_els_logo) == FC_ELS_LOGO_LEN);
-}
-#endif /* DEBUG_ASSERTS */
-
 #endif /* _FC_ELS_H_ */
diff --git a/drivers/scsi/ofc/include/fc_fs.h b/drivers/scsi/ofc/include/fc_fs.h
index 150a711..fdbed64 100644
--- a/drivers/scsi/ofc/include/fc_fs.h
+++ b/drivers/scsi/ofc/include/fc_fs.h
@@ -324,17 +324,4 @@ struct fc_data_desc {
 
 #define FC_DATA_DESC_LEN12 /* expected length of structure */
 
-#ifdef DEBUG_ASSERTS
-/*
- * Static checks for packet structure sizes.
- * These catch some obvious errors in structure definitions.
- * This should generate no code.  The check should be true at compile time.
- */
-static inline void fc_fs_size_checks(void)
-{
-   ASSERT_NOTIMPL(sizeof(struct fc_frame_header) == FC_FRAME_HEADER_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_data_desc) == FC_DATA_DESC_LEN);
-}
-#endif /* DEBUG_ASSERTS */
-
 #endif /* _FC_FS_H_ */
diff --git a/drivers/scsi/ofc/include/fc_gs.h b/drivers/scsi/ofc/include/fc_gs.h
index 2ac3d24..6738535 100644
--- a/drivers/scsi/ofc/include/fc_gs.h
+++ b/drivers/scsi/ofc/include/fc_gs.h
@@ -90,16 +90,4 @@ enum fc_ct_explan {
/* definitions not complete */
 };
 
-#ifdef DEBUG_ASSERTS
-/*
- * Static checks for packet structure sizes.
- * These catch some obvious errors in structure definitions.
- * These should generate no code since they can be tested at compile time.
- */
-static inline void fc_gs_size_checks(void)
-{
-   ASSERT_NOTIMPL(sizeof(struct fc_ct_hdr) == FC_CT_HDR_LEN);
-}
-#endif /* DEBUG_ASSERTS */
-
 #endif /* _FC_GS_H_ */
diff --git a/drivers/scsi/ofc/libfc/fc_ils.h b/drivers/scsi/ofc/libfc/fc_ils.h
index 7225ac5..c421df7 100644
--- a/drivers/scsi/ofc/libfc/fc_ils.h
+++ b/drivers/scsi/ofc/libfc/fc_ils.h
@@ -521,37 +521,4 @@ struct fc_ils_mrra_resp {
 #defineFC_MRRR_AVAIL   1   /* resources available */
 #defineFC_MRRR_UNAV2   /* resources unavailable */
 
-#ifdef DEBUG_ASSERTS
-/*
- * Static checks for packet structure sizes.
- * These catch some obvious errors in structure definitions.
- * This should generate no code.  The check should be true at compile time.
- */
-static inline void fc_ils_size_checks(void)
-{
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_sw_rjt) == FC_ILS_SW_RJT_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_gnf) == FC_ILS_GNF_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_elp_f_params) ==
-  FC_ILS_ELP_F_PARAMS_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_elp) == FC_ILS_ELP_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_elp_r_rdy) == FC_ILS_ELP_R_RDY_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_elp_vc_rdy) ==
-  FC_ILS_ELP_VC_RDY_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_efp) == FC_ILS_EFP_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_efp_rec) == FC_ILS_EFP_REC_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_mcast_id_rec) ==
-  FC_ILS_MCAST_ID_REC_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_dia) == FC_ILS_DIA_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_rdi) == FC_ILS_RDI_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_fspf) == FC_ILS_FSPF_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_hlo) == FC_ILS_HLO_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_lsu) == FC_ILS_LSU_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_lsr) == FC_ILS_LSR_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_link_desc) == FC_LINK_DESC_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_esc) == FC_ILS_ESC_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_mrra) == FC_ILS_MRRA_LEN);
-   ASSERT_NOTIMPL(sizeof(struct fc_ils_mrra_resp) == FC_ILS_MRRA_RESP_LEN);
-}
-#endif /* DEBUG_ASSERTS */
-
 

[PATCH 02/10] [FCoE] Convert one ASSERT_NOTREACHED to WARN_ON(1)

2008-01-22 Thread Robert Love

---

 drivers/scsi/ofc/openfc/openfc_if.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/drivers/scsi/ofc/openfc/openfc_if.c 
b/drivers/scsi/ofc/openfc/openfc_if.c
index 8725845..0b0c782 100644
--- a/drivers/scsi/ofc/openfc/openfc_if.c
+++ b/drivers/scsi/ofc/openfc/openfc_if.c
@@ -1059,7 +1059,7 @@ out_host_rem:
scsi_remove_host(openfcp-host);
 out_host_put:
scsi_host_put(openfcp-host);
-   ASSERT_NOTREACHED;  /* XXX caller doesn't check return yet */
+   WARN_ON(1);
return -1;
 }
 EXPORT_SYMBOL(openfc_register);

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


[PATCH 04/10] [FCoE] Remove ASSERTs from libfc

2008-01-22 Thread Robert Love

---

 drivers/scsi/ofc/include/fc_frame.h |   17 +
 drivers/scsi/ofc/libfc/fc_disc_targ.c   |   25 ++--
 drivers/scsi/ofc/libfc/fc_exch.c|   89 ---
 drivers/scsi/ofc/libfc/fc_frame.c   |1 
 drivers/scsi/ofc/libfc/fc_local_port.c  |   60 --
 drivers/scsi/ofc/libfc/fc_local_port_impl.h |1 
 drivers/scsi/ofc/libfc/fc_port.c|9 +--
 drivers/scsi/ofc/libfc/fc_print.c   |1 
 drivers/scsi/ofc/libfc/fc_remote_port.c |   21 +++---
 drivers/scsi/ofc/libfc/fc_sess.c|   63 +++
 drivers/scsi/ofc/libfc/fc_virt_fab.c|1 
 drivers/scsi/ofc/libfc/fcs_attr.c   |1 
 drivers/scsi/ofc/libfc/fcs_cmd.c|7 +-
 drivers/scsi/ofc/libfc/fcs_event.c  |3 -
 drivers/scsi/ofc/libfc/fcs_state.c  |   25 +++-
 15 files changed, 103 insertions(+), 221 deletions(-)


diff --git a/drivers/scsi/ofc/include/fc_frame.h 
b/drivers/scsi/ofc/include/fc_frame.h
index 36e9dce..6f8e85e 100644
--- a/drivers/scsi/ofc/include/fc_frame.h
+++ b/drivers/scsi/ofc/include/fc_frame.h
@@ -109,7 +109,6 @@ static inline void fc_frame_init_static(struct fc_frame *fp)
  */
 static inline int fc_frame_freed_static(struct fc_frame *fp)
 {
-   ASSERT(fp-fr_flags  FCPHF_STATIC);
return (fp-fr_flags  FCPHF_FREED);
 }
 
@@ -127,8 +126,7 @@ struct fc_frame *fc_frame_alloc_fill(struct fc_port *, 
size_t payload_len);
 static inline struct fc_frame *fc_port_frame_alloc(struct fc_port *port,
size_t payload_len)
 {
-   ASSERT(port-np_frame_alloc);
-   ASSERT((payload_len % 4) == 0);
+   WARN_ON((payload_len % 4) != 0);
return (*port-np_frame_alloc)(payload_len);
 }
 
@@ -137,8 +135,6 @@ static inline struct fc_frame *fc_frame_alloc_inline(struct 
fc_port *port,
 {
struct fc_frame *fp;
 
-   ASSERT(port);
-
/*
 * Note: Since len will often be a constant multiple of 4,
 * this check will usually be evaluated and eliminated at compile time.
@@ -166,9 +162,6 @@ static inline struct fc_frame *fc_frame_alloc_inline(struct 
fc_port *port,
  */
 static inline void fc_frame_free(struct fc_frame *fp)
 {
-   ASSERT(fp);
-   ASSERT(fp-fr_free);
-   ASSERT(fp-fr_hdr);
FC_FRAME_STAMP(fp);
fp-fr_hdr = NULL;
(*fp-fr_free)(fp);
@@ -186,8 +179,7 @@ static inline int fc_frame_is_linear(struct fc_frame *fp)
 static inline struct fc_frame_header *fc_frame_header_get(const struct
  fc_frame *fp)
 {
-   ASSERT(fp != NULL);
-   ASSERT(fp-fr_len = sizeof(struct fc_frame_header));
+   WARN_ON(fp-fr_len  sizeof(struct fc_frame_header));
return fp-fr_hdr;
 }
 
@@ -206,7 +198,6 @@ static inline void *fc_frame_payload_get(const struct 
fc_frame *fp,
 {
void *pp = NULL;
 
-   ASSERT(fp != NULL);
if (fp-fr_len = sizeof(struct fc_frame_header) + len)
pp = fc_frame_header_get(fp) + 1;
return pp;
@@ -239,8 +230,7 @@ static inline void fc_frame_setup(struct fc_frame *fp, enum 
fc_rctl r_ctl,
struct fc_frame_header *fh;
 
fh = fc_frame_header_get(fp);
-   ASSERT(fh != NULL);
-   ASSERT(r_ctl != 0);
+   WARN_ON(r_ctl == 0);
fh-fh_r_ctl = r_ctl;
fh-fh_type = type;
net32_put(fh-fh_parm_offset, 0);
@@ -255,7 +245,6 @@ fc_frame_set_offset(struct fc_frame *fp, u_int32_t offset)
struct fc_frame_header *fh;
 
fh = fc_frame_header_get(fp);
-   ASSERT(fh != NULL);
net32_put(fh-fh_parm_offset, offset);
 }
 
diff --git a/drivers/scsi/ofc/libfc/fc_disc_targ.c 
b/drivers/scsi/ofc/libfc/fc_disc_targ.c
index 4906a2b..eb24e52 100644
--- a/drivers/scsi/ofc/libfc/fc_disc_targ.c
+++ b/drivers/scsi/ofc/libfc/fc_disc_targ.c
@@ -26,7 +26,6 @@
 #undef LIST_HEAD
 #include net_types.h
 
-#include sa_assert.h
 #include ofc_dbg.h
 #include sa_timer.h
 #include sa_event.h
@@ -204,7 +203,6 @@ static int fcdt_new_target(struct fc_local_port *lp,
int error = 0;
 
if (rp  wwpn) {
-   ASSERT(atomic_read(rp-rp_refcnt)  0);
if (rp-rp_port_wwn == 0) {
/*
 * Set WWN and fall through to notify of create.
@@ -258,7 +256,6 @@ static void fcdt_del_target(struct fc_local_port *lp, 
struct fc_remote_port *rp)
 {
struct fc_sess *sess;
 
-   ASSERT(atomic_read(rp-rp_refcnt)  0);/* caller holds rp */
sess = rp-rp_sess;
if (sess) {
rp-rp_sess_ready = 0;
@@ -310,7 +307,7 @@ static void fcdt_done(struct fc_local_port *lp)
else
held = NULL;
}
-   ASSERT(!held);
+   WARN_ON(held);
fc_virt_fab_unlock(vp);
(*lp-fl_disc_cb) (lp-fl_disc_cb_arg, NULL, FC_EV_NONE);
lp-fl_disc_in_prog = 0;
@@ -347,12 +344,10 @@ static void 

[PATCH 05/10] [FCoE] Remove ASSERTs from libsa.

2008-01-22 Thread Robert Love
Signed-off-by: Robert Love [EMAIL PROTECTED]
---

 drivers/scsi/ofc/libsa/sa_event.c |   24 
 drivers/scsi/ofc/libsa/sa_hash_kern.c |7 ---
 drivers/scsi/ofc/libsa/sa_timer.c |1 -
 3 files changed, 0 insertions(+), 32 deletions(-)


diff --git a/drivers/scsi/ofc/libsa/sa_event.c 
b/drivers/scsi/ofc/libsa/sa_event.c
index cf6596a..7d9850b 100644
--- a/drivers/scsi/ofc/libsa/sa_event.c
+++ b/drivers/scsi/ofc/libsa/sa_event.c
@@ -18,7 +18,6 @@
  */
 
 #include sa_kernel.h
-#include sa_assert.h
 #include ofc_dbg.h
 #include sa_event.h
 #include queue.h
@@ -66,7 +65,6 @@ struct sa_event_list *sa_event_list_alloc(void)
TAILQ_INIT(lp-se_head);
atomic_set(lp-se_refcnt, 1);
spin_lock_init(lp-se_lock);
-   ASSERT(sa_event_list_initialized(lp));
}
return lp;
 }
@@ -77,7 +75,6 @@ void sa_event_list_free(struct sa_event_list *lp)
struct sa_event *next;
unsigned long flags;
 
-   ASSERT(sa_event_list_initialized(lp));
spin_lock_irqsave(lp-se_lock, flags);
TAILQ_FOREACH_SAFE(ev, lp-se_head, se_list, next) {
if ((ev-se_flags  SEV_MARK) == 0) {
@@ -91,19 +88,16 @@ void sa_event_list_free(struct sa_event_list *lp)
 
 static void sa_event_list_free_int(struct sa_event_list *lp)
 {
-   ASSERT(!atomic_read(lp-se_refcnt));
sa_free(lp);
 }
 
 static void sa_event_list_hold(struct sa_event_list *lp)
 {
atomic_inc(lp-se_refcnt);
-   ASSERT(atomic_read(lp-se_refcnt));
 }
 
 static void sa_event_list_release(struct sa_event_list *lp)
 {
-   ASSERT(atomic_read(lp-se_refcnt));
if (atomic_dec_and_test(lp-se_refcnt))
sa_event_list_free_int(lp);
 }
@@ -120,8 +114,6 @@ struct sa_event *sa_event_enq(struct sa_event_list *lp,
struct sa_event_head *hp;
unsigned long flags;
 
-   ASSERT(sa_event_list_initialized(lp));
-   ASSERT(handler != NULL);
spin_lock_irqsave(lp-se_lock, flags);
hp = lp-se_head;
TAILQ_FOREACH(ev, lp-se_head, se_list) {
@@ -147,8 +139,6 @@ void sa_event_deq_ev(struct sa_event_list *lp, struct 
sa_event *ev)
unsigned long flags;
 
spin_lock_irqsave(lp-se_lock, flags);
-   ASSERT(sa_event_list_initialized(lp));
-   ASSERT((ev-se_flags  SEV_MARK) == 0);
TAILQ_REMOVE(lp-se_head, ev, se_list);
spin_unlock_irqrestore(lp-se_lock, flags);
sa_free(ev);
@@ -162,7 +152,6 @@ sa_event_deq(struct sa_event_list *lp, void (*handler) 
(int, void *), void *arg)
unsigned long flags;
 
spin_lock_irqsave(lp-se_lock, flags);
-   ASSERT(sa_event_list_initialized(lp));
 
TAILQ_FOREACH_SAFE(ev, lp-se_head, se_list, next) {
if (ev-se_handler == handler  ev-se_arg == arg) {
@@ -187,7 +176,6 @@ void sa_event_call(struct sa_event_list *lp, int rc)
unsigned long flags;
 
spin_lock_irqsave(lp-se_lock, flags);
-   ASSERT(sa_event_list_initialized(lp));
memset(mark, 0, sizeof(mark));
mark.se_flags = SEV_MARK;
 
@@ -202,7 +190,6 @@ void sa_event_call(struct sa_event_list *lp, int rc)
TAILQ_INSERT_AFTER(lp-se_head, ev, mark, se_list);
spin_unlock_irqrestore(lp-se_lock, flags);
 
-   ASSERT(ev-se_handler != NULL);
(*ev-se_handler) (rc, ev-se_arg);
 
spin_lock_irqsave(lp-se_lock, flags);
@@ -218,9 +205,6 @@ void sa_event_call(struct sa_event_list *lp, int rc)
  */
 void sa_event_call_defer(struct sa_event_list *lp, int event)
 {
-   ASSERT(sa_event_list_initialized(lp));
-   ASSERT(event = 0);
-   ASSERT(event  sizeof(lp-se_pending_events) * 8);
atomic_set_mask(1  event, lp-se_pending_events);
 }
 
@@ -229,9 +213,6 @@ void sa_event_call_defer(struct sa_event_list *lp, int 
event)
  */
 void sa_event_call_cancel(struct sa_event_list *lp, int event)
 {
-   ASSERT(sa_event_list_initialized(lp));
-   ASSERT(event = 0);
-   ASSERT(event  sizeof(lp-se_pending_events) * 8);
atomic_clear_mask(1  event, lp-se_pending_events);
 }
 
@@ -243,13 +224,8 @@ void sa_event_send_deferred(struct sa_event_list *lp)
u_int32_t mask;
u_int event;
 
-   ASSERT(sa_event_list_initialized(lp));
-   ASSERT(sizeof(mask) == sizeof(lp-se_pending_events));
-
while ((mask = atomic_read(lp-se_pending_events)) != 0) {
event = ffs(mask) - 1;
-   ASSERT(event = 0);
-   ASSERT(event  sizeof(lp-se_pending_events) * 8);
atomic_clear_mask(1  event, lp-se_pending_events);
sa_event_call(lp, event);
}
diff --git a/drivers/scsi/ofc/libsa/sa_hash_kern.c 
b/drivers/scsi/ofc/libsa/sa_hash_kern.c
index 6046758..f1103a5 100644
--- a/drivers/scsi/ofc/libsa/sa_hash_kern.c
+++ b/drivers/scsi/ofc/libsa/sa_hash_kern.c
@@ -18,7 +18,6 @@
  */
 
 #include sa_kernel.h
-#include sa_assert.h
 #include 

[PATCH 06/10] [FCoE] Remove ASSERTs from openfc

2008-01-22 Thread Robert Love

---

 drivers/scsi/ofc/openfc/openfc_attr.c  |1 -
 drivers/scsi/ofc/openfc/openfc_if.c|3 ---
 drivers/scsi/ofc/openfc/openfc_ioctl.c |1 -
 drivers/scsi/ofc/openfc/openfc_pkt.c   |4 
 drivers/scsi/ofc/openfc/openfc_scsi.c  |   24 +++-
 5 files changed, 11 insertions(+), 22 deletions(-)


diff --git a/drivers/scsi/ofc/openfc/openfc_attr.c 
b/drivers/scsi/ofc/openfc/openfc_attr.c
index 12b00c3..d2a3b17 100644
--- a/drivers/scsi/ofc/openfc/openfc_attr.c
+++ b/drivers/scsi/ofc/openfc/openfc_attr.c
@@ -33,7 +33,6 @@
 #include scsi/scsi_transport_fc.h
 
 #include fc_types.h
-#include sa_assert.h
 #include fc_port.h
 #include fc_remote_port.h
 #include fcdev.h
diff --git a/drivers/scsi/ofc/openfc/openfc_if.c 
b/drivers/scsi/ofc/openfc/openfc_if.c
index 0b0c782..179f750 100644
--- a/drivers/scsi/ofc/openfc/openfc_if.c
+++ b/drivers/scsi/ofc/openfc/openfc_if.c
@@ -34,7 +34,6 @@
 #include scsi/scsi_tcq.h
 
 #include fc_types.h
-#include sa_assert.h
 #include fc_port.h
 #include fc_event.h
 #include fc_remote_port.h
@@ -277,8 +276,6 @@ static int openfc_queuecommand(struct scsi_cmnd *sc_cmd,
int rc = 0;
struct fcoe_dev_stats *stats;
 
-   ASSERT(done != NULL);
-
openfcp = (struct openfc_softc *)sc_cmd-device-host-hostdata;
 
if (openfcp-state != OPENFC_RUNNING) {
diff --git a/drivers/scsi/ofc/openfc/openfc_ioctl.c 
b/drivers/scsi/ofc/openfc/openfc_ioctl.c
index d04164b..60662d9 100644
--- a/drivers/scsi/ofc/openfc/openfc_ioctl.c
+++ b/drivers/scsi/ofc/openfc/openfc_ioctl.c
@@ -28,7 +28,6 @@
 #include scsi/scsi_transport_fc.h
 #include scsi/scsi_tcq.h
 
-#include sa_assert.h
 #include fc_types.h
 #include fc_encaps.h
 #include fc_frame.h
diff --git a/drivers/scsi/ofc/openfc/openfc_pkt.c 
b/drivers/scsi/ofc/openfc/openfc_pkt.c
index baaa77f..ecb1aae 100644
--- a/drivers/scsi/ofc/openfc/openfc_pkt.c
+++ b/drivers/scsi/ofc/openfc/openfc_pkt.c
@@ -32,7 +32,6 @@
  */
 #include sa_kernel.h
 #include fc_types.h
-#include sa_assert.h
 #include fc_event.h
 #include fc_port.h
 #include fc_remote_port.h
@@ -69,8 +68,6 @@ struct fc_scsi_pkt *openfc_alloc_scsi_pkt(struct openfc_softc 
*openfcp)
 {
struct fc_scsi_pkt *sp;
 
-   ASSERT(openfcp != NULL);
-
sp = kmem_cache_alloc(openfcp-openfc_scsi_pkt_cachep,
  openfcp-alloc_flags);
if (sp) {
@@ -110,7 +107,6 @@ int openfc_free_scsi_pkt(struct fc_scsi_pkt *sp)
 void openfc_scsi_pkt_hold(struct fc_scsi_pkt *sp)
 {
atomic_inc(sp-ref_cnt);
-   ASSERT(atomic_read(sp-ref_cnt) != 0);
 }
 
 void openfc_scsi_pkt_release(struct fc_scsi_pkt *sp)
diff --git a/drivers/scsi/ofc/openfc/openfc_scsi.c 
b/drivers/scsi/ofc/openfc/openfc_scsi.c
index 9c7f9bb..f4c742f 100644
--- a/drivers/scsi/ofc/openfc/openfc_scsi.c
+++ b/drivers/scsi/ofc/openfc/openfc_scsi.c
@@ -34,7 +34,6 @@
  * non linux dot h files
  */
 #include fc_types.h
-#include sa_assert.h
 #include fc_event.h
 #include fc_port.h
 #include fc_remote_port.h
@@ -268,12 +267,12 @@ static void openfc_scsi_send_data(struct fc_scsi_pkt 
*fsp, struct fc_seq *sp,
}
}
mfs = fc_seq_mfs(sp);
-   ASSERT(mfs = FC_MIN_MAX_PAYLOAD);
-   ASSERT(mfs = FC_MAX_PAYLOAD);
+   WARN_ON(mfs  FC_MAX_PAYLOAD);
+   WARN_ON(mfs  FC_MIN_MAX_PAYLOAD);
if (mfs  512)
mfs = ~(512 - 1);  /* round down to block size */
-   ASSERT(mfs = FC_MIN_MAX_PAYLOAD);  /* won't go below 256 */
-   ASSERT(len  0);
+   WARN_ON(mfs  FC_MIN_MAX_PAYLOAD);  /* won't go below 256 */
+   WARN_ON(len = 0);
sc = fsp-cmd;
if (sc-use_sg) {
sg = (struct scatterlist *)sc-request_buffer;
@@ -288,7 +287,7 @@ static void openfc_scsi_send_data(struct fc_scsi_pkt *fsp, 
struct fc_seq *sp,
frame_offset = offset;
tlen = 0;
sp = fc_seq_start_next_fctl(sp, FC_FC_REL_OFF);
-   ASSERT(sp);
+   WARN_ON(!sp);
 
if (page_count(sg_page(sg)) == 0)
using_sg = 0;
@@ -320,7 +319,7 @@ static void openfc_scsi_send_data(struct fc_scsi_pkt *fsp, 
struct fc_seq *sp,
 * because we can unpin the memory when the
 * status of this I/O is received
 */
-   ASSERT(fp-fr_sg_len  FC_FRAME_SG_LEN);
+   WARN_ON(fp-fr_sg_len  FC_FRAME_SG_LEN);
fsg = fp-fr_sg[fp-fr_sg_len++];
sg_set_page(fsg, sg_page(sg),
sg_bytes, sg-offset + offset);
@@ -355,11 +354,11 @@ static void openfc_scsi_send_data(struct fc_scsi_pkt 
*fsp, struct fc_seq *sp,
}
fp = NULL;
 
-   ASSERT(!error);
if (error) {
/*
 * we need to handle this case -XXX
 */
+   WARN_ON(1);

[PATCH 07/10] [FCoE] Remove ASSERTs from fcoe

2008-01-22 Thread Robert Love
Signed-off-by: Robert Love [EMAIL PROTECTED]
---

 drivers/scsi/ofc/fcoe/fcoe_def.h  |1 -
 drivers/scsi/ofc/fcoe/fcoe_dev.c  |   16 ++--
 drivers/scsi/ofc/fcoe/fcoe_if.c   |4 
 drivers/scsi/ofc/fcoe/fcoeinit.c  |1 -
 drivers/scsi/ofc/fcoe/fcoeioctl.c |1 -
 5 files changed, 6 insertions(+), 17 deletions(-)


diff --git a/drivers/scsi/ofc/fcoe/fcoe_def.h b/drivers/scsi/ofc/fcoe/fcoe_def.h
index 22c5934..867ae55 100644
--- a/drivers/scsi/ofc/fcoe/fcoe_def.h
+++ b/drivers/scsi/ofc/fcoe/fcoe_def.h
@@ -93,7 +93,6 @@ struct fcoe_rcv_info {
  */
 static inline struct fcoe_rcv_info *fcoe_dev_from_skb(struct sk_buff *skb)
 {
-ASSERT(sizeof(struct fcoe_rcv_info) = sizeof(skb-cb));
 return ((struct fcoe_rcv_info *) skb-cb);
 }
 
diff --git a/drivers/scsi/ofc/fcoe/fcoe_dev.c b/drivers/scsi/ofc/fcoe/fcoe_dev.c
index f37bd2c..0beb78e 100644
--- a/drivers/scsi/ofc/fcoe/fcoe_dev.c
+++ b/drivers/scsi/ofc/fcoe/fcoe_dev.c
@@ -36,7 +36,6 @@
 #include scsi/scsi_transport_fc.h
 #include net/rtnetlink.h
 
-#include sa_assert.h
 #include fc_types.h
 #include fc_frame.h
 #include fc_print.h
@@ -178,7 +177,7 @@ static void fcoe_skb_destroy(struct sk_buff *skb)
struct fcdev *fdev = (struct fcdev *)fp-fr_dev;
struct fcoe_softc *fc = NULL;
 
-   ASSERT(fp-fr_free_priv == skb);
+   WARN_ON(fp-fr_free_priv != skb);
 
if (fdev)
fc = (struct fcoe_softc *)fdev-drv_priv;
@@ -201,7 +200,7 @@ struct fc_frame *fcoe_frame_alloc(size_t len)
struct fc_frame *fp;
struct sk_buff *skb;
 
-   ASSERT((len % sizeof(uint32_t)) == 0);
+   WARN_ON((len % sizeof(uint32_t)) != 0);
len += sizeof(struct fc_frame_header);
skb = dev_alloc_skb(len + sizeof(*fp) +
FC_FRAME_HEADROOM + FC_FRAME_TAILROOM);
@@ -245,7 +244,7 @@ int fcoe_xmit(struct fcdev *fc_dev, struct fc_frame *fp)
if (unlikely(debug_fcoe))
fc_print_frame_hdr(fcoe_xmit, fp);
 
-   ASSERT((fp-fr_len % sizeof(uint32_t)) == 0);
+   WARN_ON((fp-fr_len % sizeof(uint32_t)) != 0);
 
fc = (struct fcoe_softc *)fc_dev-drv_priv;
/*
@@ -272,9 +271,9 @@ int fcoe_xmit(struct fcdev *fc_dev, struct fc_frame *fp)
}
}
 
-   ASSERT(fp-fr_free == fcoe_frame_free); /* allocated by us */
+   WARN_ON(fp-fr_free != fcoe_frame_free);/* allocated by us */
skb = (struct sk_buff *)fp-fr_free_priv;
-   ASSERT(skb-data == (unsigned char *)fp-fr_hdr);
+   WARN_ON(skb-data != (unsigned char *)fp-fr_hdr);
 
sof = fp-fr_sof;
eof = fp-fr_eof;
@@ -394,7 +393,6 @@ int fcoe_xmit(struct fcdev *fc_dev, struct fc_frame *fp)
} else {
struct fcoe_hdr_old *ohp;
 
-   ASSERT(hlen == sizeof(*ohp));
ohp = (struct fcoe_hdr_old *)(eh + 1);
net16_put(ohp-fcoe_plen, FC_FCOE_ENCAPS_LEN_SOF(wlen, sof));
}
@@ -502,7 +500,6 @@ int fcoe_percpu_receive_thread(void *arg)
struct fcoe_hdr_old *fchp;
u_int len;
 
-   ASSERT(hlen == sizeof(*fchp));
fchp = (struct fcoe_hdr_old *)skb-data;
len = net16_get(fchp-fcoe_plen);
skb_pull(skb, sizeof(*fchp));
@@ -548,7 +545,6 @@ int fcoe_percpu_receive_thread(void *arg)
 * and it'll be more cache-efficient.
 */
fh = fc_frame_header_get(fp);
-   ASSERT(fh);
if (fh-fh_r_ctl == FC_RCTL_DD_SOL_DATA 
fh-fh_type == FC_TYPE_FCP) {
fp-fr_flags |= FCPHF_CRC_UNCHECKED;
@@ -568,7 +564,7 @@ int fcoe_percpu_receive_thread(void *arg)
stats-ErrorFrames++;
fc_frame_free(fp);
}
-   ASSERT(fc_frame_freed_static(fp));
+   WARN_ON(!fc_frame_freed_static(fp));
kfree_skb(skb);
}
return 0;
diff --git a/drivers/scsi/ofc/fcoe/fcoe_if.c b/drivers/scsi/ofc/fcoe/fcoe_if.c
index 9963593..f3e323d 100644
--- a/drivers/scsi/ofc/fcoe/fcoe_if.c
+++ b/drivers/scsi/ofc/fcoe/fcoe_if.c
@@ -40,7 +40,6 @@
 /*
  * Non Linux header definitions
  */
-#include sa_assert.h
 #include fc_types.h
 #include fc_frame.h
 #include fc_print.h
@@ -102,8 +101,6 @@ int fcoe_destroy_interface(struct fcdev *fd)
net8_t flogi_maddr[ETH_ALEN];
 #endif
 
-   ASSERT(fd != NULL);
-
openfc_unregister(fd);
 
fc = (struct fcoe_softc *)fd-drv_priv;
@@ -222,7 +219,6 @@ int fcoe_create_interface(struct fcoe_info *fci, void *ptr)
net8_t flogi_maddr[ETH_ALEN];
 #endif
 
-   ASSERT(fci != NULL);
fdev = openfc_alloc_dev(fcoe_port_ops, sizeof(struct fcoe_softc));
if (unlikely(!fdev)) {
OFC_DBG(couldn't allocate space for hba struct);
diff --git a/drivers/scsi/ofc/fcoe/fcoeinit.c 

[PATCH 08/10] [FCoE] Remove ASSERTs from frame headers.

2008-01-22 Thread Robert Love
Signed-off-by: Robert Love [EMAIL PROTECTED]
---

 drivers/scsi/ofc/include/fc_frame.h |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)


diff --git a/drivers/scsi/ofc/include/fc_frame.h 
b/drivers/scsi/ofc/include/fc_frame.h
index 6f8e85e..3fce700 100644
--- a/drivers/scsi/ofc/include/fc_frame.h
+++ b/drivers/scsi/ofc/include/fc_frame.h
@@ -126,7 +126,6 @@ struct fc_frame *fc_frame_alloc_fill(struct fc_port *, 
size_t payload_len);
 static inline struct fc_frame *fc_port_frame_alloc(struct fc_port *port,
size_t payload_len)
 {
-   WARN_ON((payload_len % 4) != 0);
return (*port-np_frame_alloc)(payload_len);
 }
 

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


[PATCH 09/10] [FCoE] Remove sa_assert files

2008-01-22 Thread Robert Love
Signed-off-by: Robert Love [EMAIL PROTECTED]
---

 drivers/scsi/ofc/include/sa_assert.h |   85 --
 drivers/scsi/ofc/libsa/Makefile  |1 
 drivers/scsi/ofc/libsa/sa_assert.c   |   50 
 3 files changed, 0 insertions(+), 136 deletions(-)
 delete mode 100644 drivers/scsi/ofc/include/sa_assert.h
 delete mode 100644 drivers/scsi/ofc/libsa/sa_assert.c


diff --git a/drivers/scsi/ofc/include/sa_assert.h 
b/drivers/scsi/ofc/include/sa_assert.h
deleted file mode 100644
index 16224d7..000
--- a/drivers/scsi/ofc/include/sa_assert.h
+++ /dev/null
@@ -1,85 +0,0 @@
-/*
- * Copyright(c) 2007 Intel Corporation. All rights reserved.
- * 
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- * 
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- * 
- * You should have received a copy of the GNU General Public License along with
- * this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
- * 
- * Maintained at www.Open-FCoE.org
- */
-
-#ifndef _LIBSA_ASSERT_H_
-#define _LIBSA_ASSERT_H_
-
-extern void assert_failed(const char *s, ...)
-__attribute__ ((format(printf, 1, 2)));
-
-#ifndef UNLIKELY
-#define UNLIKELY(_x) (_x)
-#endif /* UNLIKELY */
-
-/*
- * ASSERT macros
- *
- * ASSERT(expr) - this calls assert_failed() if expr is false.  This variant
- * is not present in production code or if DEBUG_ASSERTS is not defined.
- * Be careful not to rely on expr being evaluated.
- */
-#if defined(DEBUG_ASSERTS)
-#define ASSERT(_x) do { \
-if (UNLIKELY(!(_x))) {  \
-assert_failed(ASSERT FAILED (%s) @ %s:%d\n,   \
-   #_x, __FILE__, __LINE__);  \
-}   \
-} while (0)
-#else
-#define ASSERT(_x)
-#endif /* DEBUG_ASSERTS */
-
-/*
- * ASSERT_NOTIMPL(expr) - this calls assert_failed() if expr is false.
- * The implication is that the condition is not handled by the current
- * implementation, and work should be done eventually to handle this.
- */
-#define ASSERT_NOTIMPL(_x) do { \
-if (UNLIKELY(!(_x))) {  \
-assert_failed(ASSERT (NOT IMPL) (%s) @ %s:%d\n, \
-   #_x, __FILE__, __LINE__);  \
-}   \
-} while (0)
-
-/*
- * ASSERT_NOTREACHED - this is the same as ASSERT_NOTIMPL(0).
- */
-#define ASSERT_NOTREACHED do {  \
-assert_failed(ASSERT (NOT REACHED) @ %s:%d\n, \
-__FILE__, __LINE__);\
-} while (0)
-
-/*
- * ASSERT_BUG(bugno, expr).  This variant is used when a bug number has
- * been assigned to any one of the other assertion failures.  It is always
- * present in code.  It gives the bug number which helps locate
- * documentation and helps prevent duplicate bug filings.
- */
-#define ASSERT_BUG(_bugNr, _x) do { \
-if (UNLIKELY(!(_x))) {  \
-assert_failed(ASSERT (BUG %d) (%s) @ %s:%d\n, \
-(_bugNr), #_x, __FILE__, __LINE__); \
-}   \
-} while (0)
-
-#ifndef LIBSA_USE_DANGEROUS_ROUTINES
-#define gets   DONT_USE_gets
-#endif /* LIBSA_USE_DANGEROUS_ROUTINES */
-
-#endif /* _LIBSA_ASSERT_H_ */
diff --git a/drivers/scsi/ofc/libsa/Makefile b/drivers/scsi/ofc/libsa/Makefile
index 42b8733..6206b78 100644
--- a/drivers/scsi/ofc/libsa/Makefile
+++ b/drivers/scsi/ofc/libsa/Makefile
@@ -5,7 +5,6 @@ EXTRA_CFLAGS += -I$(OFC_DIR)/include
 obj-y += libsa.o
 
 libsa-y := \
-   sa_assert.o \
sa_event.o \
sa_hash_kern.o \
sa_timer.o
diff --git a/drivers/scsi/ofc/libsa/sa_assert.c 
b/drivers/scsi/ofc/libsa/sa_assert.c
deleted file mode 100644
index 0824bb6..000
--- a/drivers/scsi/ofc/libsa/sa_assert.c
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Copyright(c) 2007 Intel Corporation. All rights reserved.
- * 
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- * 
- * This program is distributed in the hope it 

[PATCH 10/10] [FCoE] Removing unused defines

2008-01-22 Thread Robert Love
These defines were used with the DEBUG_ASSERT code that has
been removed.

Removed defines are,

FC_ELS_CSP_LEN
FC_ELS_CSSP_LEN
FC_ELS_FLOGI_LEN
FC_ELS_SPP_LEN
FC_ELS_RRQ_LEN
FC_ELS_PRLI_LEN
FC_ELS_LOGO_LEN

Signed-off-by: Robert Love [EMAIL PROTECTED]
---

 drivers/scsi/ofc/include/fc_els.h |   14 --
 1 files changed, 0 insertions(+), 14 deletions(-)


diff --git a/drivers/scsi/ofc/include/fc_els.h 
b/drivers/scsi/ofc/include/fc_els.h
index 6bff73e..502f0c5 100644
--- a/drivers/scsi/ofc/include/fc_els.h
+++ b/drivers/scsi/ofc/include/fc_els.h
@@ -232,8 +232,6 @@ struct fc_els_csp {
 #definesp_rel_off  sp_u.sp_plogi._sp_rel_off
 #definesp_r_a_tov  sp_u.sp_flogi_acc._sp_r_a_tov
 
-#defineFC_ELS_CSP_LEN  16  /* expected size of struct */
-
 #defineFC_SP_BB_DATA_MASK 0xfff /* mask for data field size in 
sp_bb_data */
 
 /*
@@ -278,8 +276,6 @@ struct fc_els_cssp {
u_int8_t_cp_resv2[2];   /* reserved */
 };
 
-#defineFC_ELS_CSSP_LEN 16  /* expected size of struct */
-
 /*
  * cp_class flags.
  */
@@ -314,8 +310,6 @@ struct fc_els_flogi {
net8_t  fl_vend[16];/* vendor version level */
 };
 
-#defineFC_ELS_FLOGI_LEN (7 * 16 + 4)   /* expected size of flogi 
struct */
-
 /*
  * Process login service parameter page.
  */
@@ -329,8 +323,6 @@ struct fc_els_spp {
net32_t spp_params; /* service parameters */
 };
 
-#defineFC_ELS_SPP_LEN  16  /* expected length of struct */
-
 /*
  * spp_flags.
  */
@@ -365,8 +357,6 @@ struct fc_els_rrq {
net16_t rrq_rx_id;  /* responders exchange ID */
 };
 
-#defineFC_ELS_RRQ_LEN  12  /* expected length of struct */
-
 /*
  * ELS_REC - Read exchange concise.
  */
@@ -405,8 +395,6 @@ struct fc_els_prli {
/* service parameter pages follow */
 };
 
-#defineFC_ELS_PRLI_LEN 4   /* expected length of struct */
-
 /*
  * ELS_LOGO - process or fabric logout.
  */
@@ -418,8 +406,6 @@ struct fc_els_logo {
net64_t fl_n_port_wwn;  /* port name */
 };
 
-#defineFC_ELS_LOGO_LEN 16  /* expected length of struct */
-
 /*
  * ELS_RTV - read timeout value.
  */

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


Re: [PATCH] Marvell 6440 SAS/SATA driver

2008-01-22 Thread Jeff Garzik


Comments inline, mostly minor stuff cleaning up the source.

Major problem though:  your mailer converted tabs to spaces, so our 
automated patch tools won't work on your submission.  It usually takes a 
few attempts to get your email setup working, such that all the 
automated tools used in the Linux community work.



Ke Wei wrote:

+#define MVS_QUEUE_SIZE (30)


to be consistent with the rest of the driver, make this an enum



+#define MVS_PRINTK(_x_, ...) \
+   printk(KERN_NOTICE DRV_NAME :  _x_ , ## __VA_ARGS__)

 #define mr32(reg)  readl(regs + MVS_##reg)
 #define mw32(reg,val)  writel((val), regs + MVS_##reg)
@@ -47,6 +53,65 @@
   readl(regs + MVS_##reg);\
   } while (0)

+#define MVS_BIT(x) (1L  (x))
+
+#define PORT_TYPE_SATA MVS_BIT(0)
+#define PORT_TYPE_SAS  MVS_BIT(1)


to be consistent with the rest of the driver, just open-code 1  n. 
This also makes it easier to get the C type correct.




+#define MVS_ID_NOT_MAPPED  0xff
+#define MVS_CHIP_SLOT_SZ   (1U  mvi-chip-slot_width)
+
+/* offset for D2H FIS in the Received FIS List Structure */
+#define SATA_RECEIVED_D2H_FIS(reg_set) \
+   (mvi-rx_fis + 0x400 + 0x100 * reg_set + 0x40)
+#define SATA_RECEIVED_PIO_FIS(reg_set) \
+   (mvi-rx_fis + 0x400 + 0x100 * reg_set + 0x20)
+#define UNASSOC_D2H_FIS(id) \
+   (mvi-rx_fis + 0x100 * id)
+
+
+#define READ_PORT_CONFIG_DATA(i) \
+   ((i  3)?mr32(P4_CFG_DATA + (i - 4) * 8):mr32(P0_CFG_DATA + i * 8))
+#define WRITE_PORT_CONFIG_DATA(i,tmp) \
+   {if (i  3)mw32(P4_CFG_DATA + (i - 4) * 8, tmp); \
+   else \
+   mw32(P0_CFG_DATA + i * 8, tmp); }
+#define WRITE_PORT_CONFIG_ADDR(i,tmp) \
+   {if (i  3)mw32(P4_CFG_ADDR + (i - 4) * 8, tmp); \
+   else \
+   mw32(P0_CFG_ADDR + i * 8, tmp); }
+
+#define READ_PORT_PHY_CONTROL(i) \
+   ((i  3)?mr32(P4_SER_CTLSTAT + (i - 4) * 4):mr32(P0_SER_CTLSTAT+i * 4))
+#define WRITE_PORT_PHY_CONTROL(i,tmp) \
+   {if (i  3)mw32(P4_SER_CTLSTAT + (i - 4) * 4, tmp); \
+   else \
+   mw32(P0_SER_CTLSTAT + i * 4, tmp); }
+
+#define READ_PORT_VSR_DATA(i) \
+   ((i  3)?mr32(P4_VSR_DATA + (i - 4) * 8):mr32(P0_VSR_DATA+i*8))
+#define WRITE_PORT_VSR_DATA(i,tmp) \
+   {if (i  3)mw32(P4_VSR_DATA + (i - 4) * 8, tmp); \
+   else \
+   mw32(P0_VSR_DATA + i*8, tmp); }
+#define WRITE_PORT_VSR_ADDR(i,tmp) \
+   {if (i  3)mw32(P4_VSR_ADDR + (i - 4) * 8, tmp); \
+   else \
+   mw32(P0_VSR_ADDR + i * 8, tmp); }
+
+#define READ_PORT_IRQ_STAT(i) \
+   ((i  3)?mr32(P4_INT_STAT + (i - 4) * 8):mr32(P0_INT_STAT + i * 8))
+#define WRITE_PORT_IRQ_STAT(i,tmp) \
+   {if (i  3)mw32(P4_INT_STAT + (i-4) * 8, tmp); \
+   else \
+   mw32(P0_INT_STAT + i * 8, tmp); }
+#define READ_PORT_IRQ_MASK(i) \
+   ((i  3)?mr32(P4_INT_MASK + (i-4) * 8):mr32(P0_INT_MASK+i*8))
+#define WRITE_PORT_IRQ_MASK(i,tmp) \
+   {if (i  3)mw32(P4_INT_MASK + (i-4) * 8, tmp); \
+   else \
+   mw32(P0_INT_MASK + i * 8, tmp); }



make these macros readable, by breaking each C statement into a separate 
line





@@ -260,13 +368,33 @@ enum hw_register_bits {
   PHYEV_RDY_CH= (1U  0),/* phy ready changed state */

   /* MVS_PCS */
+   PCS_EN_SATA_REG = (16), /* Enable SATA Register Set*/
+   PCS_EN_PORT_XMT_START   = (12), /* Enable Port Transmit*/
+   PCS_EN_PORT_XMT_START2  = (8),  /* For 6480*/
   PCS_SATA_RETRY  = (1U  8),/* retry ctl FIS on R_ERR */
   PCS_RSP_RX_EN   = (1U  7),/* raw response rx */
   PCS_SELF_CLEAR  = (1U  5),/* self-clearing int mode */
   PCS_FIS_RX_EN   = (1U  4),/* FIS rx enable */
   PCS_CMD_STOP_ERR= (1U  3),/* cmd stop-on-err enable */
-   PCS_CMD_RST = (1U  2),/* reset cmd issue */
+   PCS_CMD_RST = (1U  1),/* reset cmd issue */
   PCS_CMD_EN  = (1U  0),/* enable cmd issue */
+
+   /*Port n Attached Device Info*/
+   PORT_DEV_SSP_TRGT   = (1U  19),
+   PORT_DEV_SMP_TRGT   = (1U  18),
+   PORT_DEV_STP_TRGT   = (1U  17),
+   PORT_DEV_SSP_INIT   = (1U  11),
+   PORT_DEV_SMP_INIT   = (1U  10),
+   PORT_DEV_STP_INIT   = (1U  9),
+   PORT_PHY_ID_MASK= (0xFFU  24),
+   PORT_DEV_TRGT_MASK  = (0x7U  17),
+   PORT_DEV_INIT_MASK  = (0x7U  9),
+   PORT_DEV_TYPE_MASK  = (0x7U  0),
+
+   /*Port n PHY Status*/
+   PHY_RDY = (1U  2),
+   PHY_DW_SYNC = (1U  1),
+   PHY_OOB_DTCTD   = (1U  0),


to be consistent, add spaces after /* and before */




 struct mvs_port {
   struct asd_sas_port sas_port;
+   u8  taskfileset;
 };

 struct mvs_phy {
   struct mvs_port *port;
   struct asd_sas_phy  sas_phy;
+  

Re: [linux-usb-devel] [PATCH] [2.6.23.13] udev hangs USB-storage (HP r707 camera)

2008-01-22 Thread Phil Dibowitz
Grant Grundler wrote:
 On Sat, Jan 19, 2008 at 10:11:05PM -0600, James Bottomley wrote:
 ...
 Add the device to drivers/usb/storage/unusual_devs.h with
 US_FL_FIX_CAPACITY.  You'll need to know it's USB ids as well for this
 file.
 
 James,
 Thanks! Patch below (for Alan) works for me.

Patch looks fine to me. Let me get my tree in order (sorry, been really
swamped lately so I haven't updated in a while), make sure this diff's
nicely against the latest rc's, and I'll pass it on up. .24 is already
(essentially) done, so I'll get it to Greg in the next day or so and it
should make it into .25.

Sorry for the delay, and thanks Alan for pinging me on this.

As for Grant - more information on Protocol vs Subclass can be found on
my unusual_devs page here:

  http://www.phildev.net/linux/usb-unusualdevs-notes.html

-- 
Phil Dibowitz [EMAIL PROTECTED]
Open Source software and tech docsInsanity Palace of Metallica
http://www.phildev.net/   http://www.ipom.com/

Never write it in C if you can do it in 'awk';
 Never do it in 'awk' if 'sed' can handle it;
 Never use 'sed' when 'tr' can do the job;
 Never invoke 'tr' when 'cat' is sufficient;
 Avoid using 'cat' whenever possible -- Taylor's Laws of Programming




signature.asc
Description: OpenPGP digital signature


Re: [linux-usb-devel] [PATCH] [2.6.23.13] udev hangs USB-storage (HP r707 camera)

2008-01-22 Thread Grant Grundler
On Tue, Jan 22, 2008 at 09:27:09PM -0800, Phil Dibowitz wrote:
...
 Patch looks fine to me. Let me get my tree in order (sorry, been really
 swamped lately so I haven't updated in a while), make sure this diff's
 nicely against the latest rc's,

It's not. It's against 2.6.23. But I expect it to apply with fuzz
at the worst case.

  and I'll pass it on up. .24 is already
 (essentially) done, so I'll get it to Greg in the next day or so and it
 should make it into .25.

thanks!

 
 Sorry for the delay, and thanks Alan for pinging me on this.
 
 As for Grant - more information on Protocol vs Subclass can be found on
 my unusual_devs page here:
 
   http://www.phildev.net/linux/usb-unusualdevs-notes.html

In general, a very helpful document! Thanks!

useProtocol - These are listed in protocols.h with explinations. One example:

* US_SC_DEVICE - use whatever the device claims
* see drivers/usb/storage/protocol.h for a full list and comments 

Some issues/comments:
o I don't see any sort of list in protocol.h. And the comments in
  protocol.h don't discuss useProtocol or which protocols.
  Maybe the list moved to include/linux/usb_usual.h?

o US_SC_* and US_PR_* constants both seem like good candidates for enum
 (to make sure the right type get used with the right field.)

cheers,
grant
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html