Re: [usb-storage] PATCH: usb-storage-set-last-sector-bug-flag.patch
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
-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
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]
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)
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
--- 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)
--- 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
--- 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.
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
--- 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
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.
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
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
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
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)
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)
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