Re: ips.c broken since 2.6.23 on x86_64?
ips did scsi_add_host(sh, NULL) so scsi_dma_map uses shost_gendev.parent that isn't initialized properly, then the kernel crashes. 2.6.23 and 2.6.24 have this bug. We can fix this by calling scsi_add_host with pdev-dev, in the standard way (like the following way) but this bug was fixed in the current Linus tree by: commit 2551a13e61d3c3df6c2da6de5a3ece78e6d67111 Author: Jeff Garzik [EMAIL PROTECTED] Date: Thu Dec 13 16:14:10 2007 -0800 [SCSI] ips: handle scsi_add_host() failure, and other err cleanups James, the legitimate way to fix stable trees is sending this commit (not sending a patch that was not committed upstream)? On Mon, 18 Feb 2008 22:32:46 +0900 FUJITA Tomonori [EMAIL PROTECTED] wrote: On Sun, 17 Feb 2008 15:37:02 -0800 Tim Pepper [EMAIL PROTECTED] wrote: On Mon 19 Feb at 07:31:56 +0900 [EMAIL PROTECTED] said: Can you apply the 0001 and 0002 against 2.6.24 and see how it works? If it works well, then please apply the 0001, 0002 and 0003. Fujita-san, I've started through the patches in order, cumulatively and after applying 0005 things break. I wont be able to test anything else until tomorrow when I can phycisally reset the machine... Great, thanks a lot! Can you apply this patch after the 0005 patch and see how it works? If it works, then please continue to test 0006, 0007 ... diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c index 05bb6ea..39cdd68 100644 --- a/drivers/scsi/ips.c +++ b/drivers/scsi/ips.c @@ -6906,7 +6906,7 @@ ips_register_scsi(int index) sh-max_channel = ha-nbus - 1; sh-can_queue = ha-max_cmds - 1; - scsi_add_host(sh, NULL); + scsi_add_host(sh, ha-pcidev-dev); scsi_scan_host(sh); return 0; -- 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 - 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 update] firewire: fw-sbp2: fix NULL pointer deref in scsi_remove_device
Fix a kernel bug when unplugging an SBP-2 device after having its
scsi_device already removed via the delete sysfs attribute.
Signed-off-by: Stefan Richter [EMAIL PROTECTED]
---
Update: A _put was missing in a failure path.
drivers/firewire/fw-sbp2.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
Index: linux/drivers/firewire/fw-sbp2.c
===
--- linux.orig/drivers/firewire/fw-sbp2.c
+++ linux/drivers/firewire/fw-sbp2.c
@@ -762,9 +762,10 @@ static void sbp2_release_target(struct k
sbp2_unblock(tgt);
list_for_each_entry_safe(lu, next, tgt-lu_list, link) {
- if (lu-sdev)
+ if (lu-sdev) {
scsi_remove_device(lu-sdev);
-
+ scsi_device_put(lu-sdev);
+ }
sbp2_send_management_orb(lu, tgt-node_id, lu-generation,
SBP2_LOGOUT_REQUEST, lu-login_id, NULL);
@@ -886,12 +887,11 @@ static void sbp2_login(struct work_struc
if (IS_ERR(sdev))
goto out_logout_login;
- scsi_device_put(sdev);
-
/* Unreported error during __scsi_add_device() */
smp_rmb(); /* get current card generation */
if (generation != device-card-generation) {
scsi_remove_device(sdev);
+ scsi_device_put(sdev);
goto out_logout_login;
}
--
Stefan Richter
-=-==--- --=- =--==
http://arcgraph.de/sr/
-
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] ips: fix data buffer accessors conversion bug
There is one more bug in ips. I think that this needs to go to
scsi-rc-fixes, 2.6.24-stable, and 2.6.23-stable though we might rarely
hit this bug.
=
From: FUJITA Tomonori [EMAIL PROTECTED]
Date: Tue, 19 Feb 2008 16:03:47 +0900
Subject: [PATCH] ips: fix data buffer accessors conversion bug
This fixes a bug that can't handle a passthru command with more than
two sg entries.
Big thanks to Tim Pepper for debugging the problem.
Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
Cc: Tim Pepper [EMAIL PROTECTED]
Cc: Salyzyn, Mark [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/ips.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index bb152fb..7ed568f 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -1576,7 +1576,7 @@ ips_make_passthru(ips_ha_t *ha, struct scsi_cmnd *SC,
ips_scb_t *scb, int intr)
METHOD_TRACE(ips_make_passthru, 1);
scsi_for_each_sg(SC, sg, scsi_sg_count(SC), i)
-length += sg[i].length;
+ length += sg-length;
if (length sizeof (ips_passthru_t)) {
/* wrong size */
--
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: ips.c broken since 2.6.23 on x86_64?
On Mon, 18 Feb 2008 19:48:49 -0800 Tim Pepper [EMAIL PROTECTED] wrote: On Feb 18, 2008 4:11 PM, FUJITA Tomonori [EMAIL PROTECTED] wrote: Can you please help me just once more? 2.6.25-rc2 fixed this bug in a bit different way by chance. Please test 2.6.25-rc2 with the attached patch to make sure that ips in 2.6.25 works well. Confirmed...the patch below against 2.6.25-rc2 also works for me. Thanks a lot! I'll make sure that the ips driver in 2.6.23-stable, 2.6.24-stable (and 2.6.25) will work well. - 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] ips: sg chaining support to the path to non I/O commands
Here is another ips patch, but not a bug fix.
=
From: FUJITA Tomonori [EMAIL PROTECTED]
Subject: [PATCH] ips: sg chaining support to the path to non I/O commands
I overlooked ips_scmd_buf_write and ips_scmd_buf_read when I converted
ips to use the data buffer accessors.
ips is unlikely to use sg chaining (especially in this path) since a)
this path is used only for non I/O commands (with little data
transfer), b) ips's sg_tablesize is set to just 17.
Thanks to Tim Pepper for testing this patch.
Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
Cc: Salyzyn, Mark [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/ips.c | 18 ++
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 7ed568f..e5467a4 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -3510,15 +3510,16 @@ ips_scmd_buf_write(struct scsi_cmnd *scmd, void *data,
unsigned int count)
struct scatterlist *sg = scsi_sglist(scmd);
for (i = 0, xfer_cnt = 0;
- (i scsi_sg_count(scmd)) (xfer_cnt count); i++) {
-min_cnt = min(count - xfer_cnt, sg[i].length);
+ (i scsi_sg_count(scmd)) (xfer_cnt count);
+ i++, sg = sg_next(sg)) {
+min_cnt = min(count - xfer_cnt, sg-length);
/* kmap_atomic() ensures addressability of the data buffer.*/
/* local_irq_save() protects the KM_IRQ0 address slot. */
local_irq_save(flags);
-buffer = kmap_atomic(sg_page(sg[i]), KM_IRQ0) + sg[i].offset;
+ buffer = kmap_atomic(sg_page(sg), KM_IRQ0) + sg-offset;
memcpy(buffer, cdata[xfer_cnt], min_cnt);
-kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
+ kunmap_atomic(buffer - sg-offset, KM_IRQ0);
local_irq_restore(flags);
xfer_cnt += min_cnt;
@@ -3543,15 +3544,16 @@ ips_scmd_buf_read(struct scsi_cmnd *scmd, void *data,
unsigned int count)
struct scatterlist *sg = scsi_sglist(scmd);
for (i = 0, xfer_cnt = 0;
- (i scsi_sg_count(scmd)) (xfer_cnt count); i++) {
-min_cnt = min(count - xfer_cnt, sg[i].length);
+ (i scsi_sg_count(scmd)) (xfer_cnt count);
+ i++, sg = sg_next(sg)) {
+ min_cnt = min(count - xfer_cnt, sg-length);
/* kmap_atomic() ensures addressability of the data buffer.*/
/* local_irq_save() protects the KM_IRQ0 address slot. */
local_irq_save(flags);
-buffer = kmap_atomic(sg_page(sg[i]), KM_IRQ0) + sg[i].offset;
+ buffer = kmap_atomic(sg_page(sg), KM_IRQ0) + sg-offset;
memcpy(cdata[xfer_cnt], buffer, min_cnt);
-kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
+ kunmap_atomic(buffer - sg-offset, KM_IRQ0);
local_irq_restore(flags);
xfer_cnt += min_cnt;
--
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: [PATCH 1/1] cciss: procfs updates to display info about many volumes
On Tue, 19 Feb 2008 11:48:18 +0100 Jens Axboe [EMAIL PROTECTED] wrote: On Mon, Feb 11 2008, Mike Miller wrote: Patch 1 of 1 This patch allows us to display information about all of the logical volumes configured on a particular without stepping on memory even when there are many volumes (128 or more) configured. This patch replaces the one submitted on 20071214. See http://groups.google.com/group/linux.kernel/browse_thread/thread/49a50244b19f8855/ba3dc95b23391521?hl=enlnk=gstq=cciss#ba3dc95b23391521 which has not been merged. That patch displayed information about only the first logical volume on each controller and had negative side effects for some installers. Please consider this for inclusion. It looks ok, but has some flaws. Try to disable cciss scsi and tape support: In file included from drivers/block/cciss.c:231: drivers/block/cciss_scsi.c:1498:38: error: macro parameters must be comma-separated drivers/block/cciss.c: In function 'cciss_seq_show_header': drivers/block/cciss.c:272: error: implicit declaration of function 'cciss_seq_tape_report' drivers/block/cciss.c: In function 'cciss_proc_write': drivers/block/cciss.c:393: error: implicit declaration of function 'cciss_engage_scsi' You macro definition of cciss_seq_tape_report() is totally busted. Either write is as a macro OR as a function. Fix these up and resubmit, then I'll take it. It also need to be updated to use the non-racy proc_create(), please, as per Alexey's comments. - 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] ips: sg chaining support to the path to non I/O commands
ACK
Other RAID drivers (eg: aacraid) makes the assumption that commands in these
paths (INQUIRY, READ CAPACITY, MODE SENSE etc spoofing) are single scatter
gather elements and have yet to be bitten. I agree with Fujita-san about the
practical unlikelihood. The fix does not incur any change in code overhead, so
it is worth hardening!
Can one create a request via /dev/sg for a buffer smaller than 256 and manage
to create a multi-element scatter gather?
Sincerely -- Mark Salyzyn
-Original Message-
From: FUJITA Tomonori [mailto:[EMAIL PROTECTED]
Sent: Tuesday, February 19, 2008 4:42 AM
To: [EMAIL PROTECTED]
Cc: linux-scsi@vger.kernel.org; Salyzyn, Mark;
[EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: [PATCH] ips: sg chaining support to the path to non
I/O commands
Here is another ips patch, but not a bug fix.
=
From: FUJITA Tomonori [EMAIL PROTECTED]
Subject: [PATCH] ips: sg chaining support to the path to non
I/O commands
I overlooked ips_scmd_buf_write and ips_scmd_buf_read when I converted
ips to use the data buffer accessors.
ips is unlikely to use sg chaining (especially in this path) since a)
this path is used only for non I/O commands (with little data
transfer), b) ips's sg_tablesize is set to just 17.
Thanks to Tim Pepper for testing this patch.
Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
Cc: Salyzyn, Mark [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/ips.c | 18 ++
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 7ed568f..e5467a4 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -3510,15 +3510,16 @@ ips_scmd_buf_write(struct scsi_cmnd
*scmd, void *data, unsigned int count)
struct scatterlist *sg = scsi_sglist(scmd);
for (i = 0, xfer_cnt = 0;
- (i scsi_sg_count(scmd)) (xfer_cnt count); i++) {
-min_cnt = min(count - xfer_cnt, sg[i].length);
+ (i scsi_sg_count(scmd)) (xfer_cnt count);
+ i++, sg = sg_next(sg)) {
+min_cnt = min(count - xfer_cnt, sg-length);
/* kmap_atomic() ensures addressability of
the data buffer.*/
/* local_irq_save() protects the KM_IRQ0
address slot. */
local_irq_save(flags);
-buffer = kmap_atomic(sg_page(sg[i]),
KM_IRQ0) + sg[i].offset;
+ buffer = kmap_atomic(sg_page(sg), KM_IRQ0) +
sg-offset;
memcpy(buffer, cdata[xfer_cnt], min_cnt);
-kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
+ kunmap_atomic(buffer - sg-offset, KM_IRQ0);
local_irq_restore(flags);
xfer_cnt += min_cnt;
@@ -3543,15 +3544,16 @@ ips_scmd_buf_read(struct scsi_cmnd
*scmd, void *data, unsigned int count)
struct scatterlist *sg = scsi_sglist(scmd);
for (i = 0, xfer_cnt = 0;
- (i scsi_sg_count(scmd)) (xfer_cnt count); i++) {
-min_cnt = min(count - xfer_cnt, sg[i].length);
+ (i scsi_sg_count(scmd)) (xfer_cnt count);
+ i++, sg = sg_next(sg)) {
+ min_cnt = min(count - xfer_cnt, sg-length);
/* kmap_atomic() ensures addressability of
the data buffer.*/
/* local_irq_save() protects the KM_IRQ0
address slot. */
local_irq_save(flags);
-buffer = kmap_atomic(sg_page(sg[i]),
KM_IRQ0) + sg[i].offset;
+ buffer = kmap_atomic(sg_page(sg), KM_IRQ0) +
sg-offset;
memcpy(cdata[xfer_cnt], buffer, min_cnt);
-kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
+ kunmap_atomic(buffer - sg-offset, KM_IRQ0);
local_irq_restore(flags);
xfer_cnt += min_cnt;
-
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: SCSI RAM driver
On Tue, Feb 19, 2008 at 10:14:53PM +0900, FUJITA Tomonori wrote: I see that two drivers have very different objectives but if we add use_thread option to scsi_debug (we can do easily), it seems that scsi_debug can provide all the features that scsi_ram does. It's not just use_thread. It's also discard_read/discard_write. And scsi_ram has a different data storage model from scsi_debug -- scsi_debug simulates an arbitrarily sized disc by wrapping around some small (virtually) contiguous allocation of pages; scsi_ram actually allocates the amount of ram that it's told to. This can be solved with another module parameter, of course. I'm in no way opposed to merging the two; it's a question of whether Doug will mind me doing some surgery on his driver. -- 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: SCSI RAM driver
On Tue, 19 Feb 2008 06:31:20 -0700 Matthew Wilcox [EMAIL PROTECTED] wrote: On Tue, Feb 19, 2008 at 10:14:53PM +0900, FUJITA Tomonori wrote: I see that two drivers have very different objectives but if we add use_thread option to scsi_debug (we can do easily), it seems that scsi_debug can provide all the features that scsi_ram does. It's not just use_thread. It's also discard_read/discard_write. scsi_debug has a similar option, fake_rw, which discards both read and write data. And scsi_ram has a different data storage model from scsi_debug -- scsi_debug simulates an arbitrarily sized disc by wrapping around some small (virtually) contiguous allocation of pages; scsi_ram actually allocates the amount of ram that it's told to. This can be solved with another module parameter, of course. IIRC, if virtual_gb option is set to zero, scsi_debug allocates the amount of ram that it's told to. I'm in no way opposed to merging the two; it's a question of whether Doug will mind me doing some surgery on his driver. - 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/1] cciss: procfs updates to display info about many volumes
-Original Message- From: Andrew Morton [mailto:[EMAIL PROTECTED] Sent: Tuesday, February 19, 2008 6:00 AM To: Jens Axboe Cc: Miller, Mike (OS Dev); LKML; LKML-scsi Subject: Re: [PATCH 1/1] cciss: procfs updates to display info about many volumes On Tue, 19 Feb 2008 11:48:18 +0100 Jens Axboe [EMAIL PROTECTED] wrote: On Mon, Feb 11 2008, Mike Miller wrote: Patch 1 of 1 This patch allows us to display information about all of the logical volumes configured on a particular without stepping on memory even when there are many volumes (128 or more) configured. This patch replaces the one submitted on 20071214. See http://groups.google.com/group/linux.kernel/browse_thread/thread/49a 50244b19f8855/ba3dc95b23391521?hl=enlnk=gstq=cciss#ba3dc95b2339152 1 which has not been merged. That patch displayed information about only the first logical volume on each controller and had negative side effects for some installers. Please consider this for inclusion. It looks ok, but has some flaws. Try to disable cciss scsi and tape support: In file included from drivers/block/cciss.c:231: drivers/block/cciss_scsi.c:1498:38: error: macro parameters must be comma-separated drivers/block/cciss.c: In function 'cciss_seq_show_header': drivers/block/cciss.c:272: error: implicit declaration of function 'cciss_seq_tape_report' drivers/block/cciss.c: In function 'cciss_proc_write': drivers/block/cciss.c:393: error: implicit declaration of function 'cciss_engage_scsi' You macro definition of cciss_seq_tape_report() is totally busted. Either write is as a macro OR as a function. Fix these up and resubmit, then I'll take it. It also need to be updated to use the non-racy proc_create(), please, as per Alexey's comments. Thanks all, I'll fix and resubmit. -- mikem - 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: ips.c broken since 2.6.23 on x86_64?
On Tue, 2008-02-19 at 17:02 +0900, FUJITA Tomonori wrote: ips did scsi_add_host(sh, NULL) so scsi_dma_map uses shost_gendev.parent that isn't initialized properly, then the kernel crashes. 2.6.23 and 2.6.24 have this bug. We can fix this by calling scsi_add_host with pdev-dev, in the standard way (like the following way) but this bug was fixed in the current Linus tree by: commit 2551a13e61d3c3df6c2da6de5a3ece78e6d67111 Author: Jeff Garzik [EMAIL PROTECTED] Date: Thu Dec 13 16:14:10 2007 -0800 [SCSI] ips: handle scsi_add_host() failure, and other err cleanups James, the legitimate way to fix stable trees is sending this commit (not sending a patch that was not committed upstream)? Well, the upstream patch doesn't look so bad as a stable candidate to my eye. Just because it's an unintended bugfix doesn't automatically invalidate it. The reason stable likes backports of existing upstream patches is because they've supposedly been well tested in upstream. Although that doesn't apply in this case because the other bug rather prevented testing, the principle is still sound. So, would there be any problems simply backporting this? 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
[PATCH 1/2] libsas: Provide a transport-level facility to request SAS addrs
Provide a facility to use the request_firmware() interface to get a SAS
address from userspace. This can be used by SAS LLDDs that cannot
obtain the address from the host adapter.
Resend of 8 Oct. 2007 patch, now based off 2.6.25-rc2 + scsi_misc.
Signed-off-by: Darrick J. Wong [EMAIL PROTECTED]
---
drivers/scsi/libsas/sas_scsi_host.c | 41 +++
include/scsi/libsas.h |2 ++
2 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/libsas/sas_scsi_host.c
b/drivers/scsi/libsas/sas_scsi_host.c
index f869fba..583d249 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -24,6 +24,8 @@
*/
#include linux/kthread.h
+#include linux/firmware.h
+#include linux/ctype.h
#include sas_internal.h
@@ -1050,6 +1052,45 @@ void sas_target_destroy(struct scsi_target *starget)
return;
}
+static void sas_parse_addr(u8 *sas_addr, const char *p)
+{
+ int i;
+ for (i = 0; i SAS_ADDR_SIZE; i++) {
+ u8 h, l;
+ if (!*p)
+ break;
+ h = isdigit(*p) ? *p-'0' : toupper(*p)-'A'+10;
+ p++;
+ l = isdigit(*p) ? *p-'0' : toupper(*p)-'A'+10;
+ p++;
+ sas_addr[i] = (h4) | l;
+ }
+}
+
+#define SAS_STRING_ADDR_SIZE 16
+
+int sas_request_addr(struct Scsi_Host *shost, u8 *addr)
+{
+ int res;
+ const struct firmware *fw;
+
+ res = request_firmware(fw, sas_addr, shost-shost_gendev);
+ if (res)
+ return res;
+
+ if (fw-size SAS_STRING_ADDR_SIZE) {
+ res = -ENODEV;
+ goto out;
+ }
+
+ sas_parse_addr(addr, fw-data);
+
+out:
+ release_firmware(fw);
+ return res;
+}
+EXPORT_SYMBOL_GPL(sas_request_addr);
+
EXPORT_SYMBOL_GPL(sas_queuecommand);
EXPORT_SYMBOL_GPL(sas_target_alloc);
EXPORT_SYMBOL_GPL(sas_slave_configure);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 3ffd6b5..5f183de 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -676,4 +676,6 @@ extern int sas_smp_handler(struct Scsi_Host *shost, struct
sas_rphy *rphy,
extern void sas_ssp_task_response(struct device *dev, struct sas_task *task,
struct ssp_response_iu *iu);
+int sas_request_addr(struct Scsi_Host *shost, u8 *addr);
+
#endif /* _SASLIB_H_ */
-
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] aic94xx: Don't free ABORT_TASK SCBs that are timed out (Was: Re: aic94xx: failing on high load)
If we send an ABORT_TASK ascb that doesn't return within the timeout period, we should not free that ascb because the sequencer is still holding onto it. Hopefully it will fix what James Bottomley describes below: On Tue, Feb 19, 2008 at 10:22:20AM -0600, James Bottomley wrote: Unfortunately, there's a bug in TMF timeout handling in the driver, it leaves the sequencer entry pending, but frees the ascb. If the sequencer ever picks this up it will get very confused, as it does a while down in the trace: aic94xx: BUG:sequencer:dl:no ascb?! aic94xx: BUG:sequencer:dl:no ascb?! That's where the sequencer adds an ascb to the done list that we've already freed. From this point on confusion reigns and the error handler eventually offlines the device. I'll see if I can come up with patches to fix this ... or at least mitigate the problems it causes. Signed-off-by: Darrick J. Wong [EMAIL PROTECTED] --- drivers/scsi/aic94xx/aic94xx_tmf.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c b/drivers/scsi/aic94xx/aic94xx_tmf.c index b52124f..4b24bd3 100644 --- a/drivers/scsi/aic94xx/aic94xx_tmf.c +++ b/drivers/scsi/aic94xx/aic94xx_tmf.c @@ -463,7 +463,7 @@ int asd_abort_task(struct sas_task *task) AIC94XX_SCB_TIMEOUT); spin_lock_irqsave(task-task_state_lock, flags); if (leftover 1) - res = TMF_RESP_FUNC_FAILED; + goto out_not_reported; if (task-task_state_flags SAS_TASK_STATE_DONE) res = TMF_RESP_FUNC_COMPLETE; spin_unlock_irqrestore(task-task_state_lock, flags); @@ -487,6 +487,11 @@ out: asd_ascb_free(ascb); ASD_DPRINTK(task 0x%p aborted, res: 0x%x\n, task, res); return res; + +out_not_reported: + spin_unlock_irqrestore(task-task_state_lock, flags); + ASD_DPRINTK(task 0x%p aborted? but not reported.\n, task); + return res; } /** - 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] enclosure: add support for enclosure services
On Wed 2008-02-13 09:45:02, Kristen Carlson Accardi wrote: On Tue, 12 Feb 2008 13:28:15 -0600 James Bottomley [EMAIL PROTECTED] wrote: On Tue, 2008-02-12 at 11:07 -0800, Kristen Carlson Accardi wrote: I understand what you are trying to do - I guess I just doubt the value you've added by doing this. I think that there's going to be so much customization that system vendors will want to add, that they are going to wind up adding a custom library regardless, so standardising those few things won't buy us anything. It depends ... if you actually have a use for the customisations, yes. If you just want the basics of who (what's in the enclousure), what (activity) and where (locate) then I think it solves your problem almost entirely. So, entirely as a straw horse, tell me what else your enclosures provide that I haven't listed in the four points. The SES standards too provide a huge range of things that no-one ever seems to implement (temperature, power, fan speeds etc). I think the users of enclosures fall int these categories 85% just want to know where their device actually is (i.e. that sdc is in enclosure slot 5) 50% like watching the activity lights 30% want to be able to have a visual locate function 20% want a visual failure indication (the other 80% rely on some OS notification instead) When you add up the overlapping needs, you get about 90% of people happy with the basics that the enclosure services provide. Could there be more ... sure; should there be more ... I don't think so ... that's what value add the user libraries can provide. James I don't think I'm arguing whether or not your solution may work, what I am arguing is really a more philosophical point. Not can we do it this way, but should we do it way. I am of the opinion that Hw abstraction is still kernel's job. That's why we have leds exported in sysfs... let vendors have their libraries, but lets put the 'everyone does these' stuff in kernel. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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
[PATCH 2/2] aic94xx: Use sas_request_addr() to provide SAS WWN if the adapter lacks one
If the aic94xx chip doesn't have a SAS address in the chip's flash memory,
make libsas get one for us.
Resend of 8 Oct 2007 patch, now based off 2.6.25-rc2 + scsi_misc.
Signed-off-by: Darrick J. Wong [EMAIL PROTECTED]
---
drivers/scsi/aic94xx/aic94xx.h | 16
drivers/scsi/aic94xx/aic94xx_hwi.c | 20 +---
drivers/scsi/aic94xx/aic94xx_init.c |2 --
3 files changed, 9 insertions(+), 29 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx.h b/drivers/scsi/aic94xx/aic94xx.h
index 32f513b..aee235f 100644
--- a/drivers/scsi/aic94xx/aic94xx.h
+++ b/drivers/scsi/aic94xx/aic94xx.h
@@ -58,7 +58,6 @@
extern struct kmem_cache *asd_dma_token_cache;
extern struct kmem_cache *asd_ascb_cache;
-extern char sas_addr_str[2*SAS_ADDR_SIZE + 1];
static inline void asd_stringify_sas_addr(char *p, const u8 *sas_addr)
{
@@ -68,21 +67,6 @@ static inline void asd_stringify_sas_addr(char *p, const u8
*sas_addr)
*p = '\0';
}
-static inline void asd_destringify_sas_addr(u8 *sas_addr, const char *p)
-{
- int i;
- for (i = 0; i SAS_ADDR_SIZE; i++) {
- u8 h, l;
- if (!*p)
- break;
- h = isdigit(*p) ? *p-'0' : *p-'A'+10;
- p++;
- l = isdigit(*p) ? *p-'0' : *p-'A'+10;
- p++;
- sas_addr[i] = (h4) | l;
- }
-}
-
struct asd_ha_struct;
struct asd_ascb;
diff --git a/drivers/scsi/aic94xx/aic94xx_hwi.c
b/drivers/scsi/aic94xx/aic94xx_hwi.c
index 098b5f3..940a207 100644
--- a/drivers/scsi/aic94xx/aic94xx_hwi.c
+++ b/drivers/scsi/aic94xx/aic94xx_hwi.c
@@ -27,6 +27,7 @@
#include linux/pci.h
#include linux/delay.h
#include linux/module.h
+#include linux/firmware.h
#include aic94xx.h
#include aic94xx_reg.h
@@ -38,16 +39,14 @@ u32 MBAR0_SWB_SIZE;
/* -- Initialization -- */
-static void asd_get_user_sas_addr(struct asd_ha_struct *asd_ha)
+static int asd_get_user_sas_addr(struct asd_ha_struct *asd_ha)
{
- extern char sas_addr_str[];
- /* If the user has specified a WWN it overrides other settings
-*/
- if (sas_addr_str[0] != '\0')
- asd_destringify_sas_addr(asd_ha-hw_prof.sas_addr,
-sas_addr_str);
- else if (asd_ha-hw_prof.sas_addr[0] != 0)
- asd_stringify_sas_addr(sas_addr_str, asd_ha-hw_prof.sas_addr);
+ /* adapter came with a sas address */
+ if (asd_ha-hw_prof.sas_addr[0])
+ return 0;
+
+ return sas_request_addr(asd_ha-sas_ha.core.shost,
+ asd_ha-hw_prof.sas_addr);
}
static void asd_propagate_sas_addr(struct asd_ha_struct *asd_ha)
@@ -657,8 +656,7 @@ int asd_init_hw(struct asd_ha_struct *asd_ha)
asd_init_ctxmem(asd_ha);
- asd_get_user_sas_addr(asd_ha);
- if (!asd_ha-hw_prof.sas_addr[0]) {
+ if (asd_get_user_sas_addr(asd_ha)) {
asd_printk(No SAS Address provided for %s\n,
pci_name(asd_ha-pcidev));
err = -ENODEV;
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c
b/drivers/scsi/aic94xx/aic94xx_init.c
index 5d761eb..1824b0b 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -56,8 +56,6 @@ MODULE_PARM_DESC(collector, \n
\tThe aic94xx SAS LLDD supports both modes.\n
\tDefault: 0 (Direct Mode).\n);
-char sas_addr_str[2*SAS_ADDR_SIZE + 1] = ;
-
static struct scsi_transport_template *aic94xx_transport_template;
static int asd_scan_finished(struct Scsi_Host *, unsigned long);
static void asd_scan_start(struct Scsi_Host *);
-
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] aic94xx: Don't free ABORT_TASK SCBs that are timed out (Was: Re: aic94xx: failing on high load)
On Tue, 2008-02-19 at 10:44 -0800, Darrick J. Wong wrote: If we send an ABORT_TASK ascb that doesn't return within the timeout period, we should not free that ascb because the sequencer is still holding onto it. Hopefully it will fix what James Bottomley describes below: On Tue, Feb 19, 2008 at 10:22:20AM -0600, James Bottomley wrote: Unfortunately, there's a bug in TMF timeout handling in the driver, it leaves the sequencer entry pending, but frees the ascb. If the sequencer ever picks this up it will get very confused, as it does a while down in the trace: aic94xx: BUG:sequencer:dl:no ascb?! aic94xx: BUG:sequencer:dl:no ascb?! That's where the sequencer adds an ascb to the done list that we've already freed. From this point on confusion reigns and the error handler eventually offlines the device. I'll see if I can come up with patches to fix this ... or at least mitigate the problems it causes. Signed-off-by: Darrick J. Wong [EMAIL PROTECTED] Actually, unfortunately, this is only a tiny part of it. The message that triggered all of this is sas: sas_scsi_find_task: aborting task 0x81033c3d3d80 aic94xx: tmf timed out aic94xx: tmf came back That's caused by a timeout at asd_enqueue_internal() further up in the code base. 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
[2.6 patch] scsi/qla4xxx/ql4_isr.c: remove dead code
This patch removes dead code spotted by the Coverity checker.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
drivers/scsi/qla4xxx/ql4_isr.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
--- linux-2.6/drivers/scsi/qla4xxx/ql4_isr.c.old2008-02-19
20:29:16.0 +0200
+++ linux-2.6/drivers/scsi/qla4xxx/ql4_isr.c2008-02-19 20:30:37.0
+0200
@@ -91,38 +91,22 @@ static void qla4xxx_status_entry(struct
if (scsi_status == 0) {
cmd-result = DID_OK 16;
break;
}
if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_OVER) {
cmd-result = DID_ERROR 16;
break;
}
- if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_UNDER) {
+ if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_UNDER)
scsi_set_resid(cmd, residual);
- if (!scsi_status ((scsi_bufflen(cmd) - residual)
- cmd-underflow)) {
-
- cmd-result = DID_ERROR 16;
-
- DEBUG2(printk(scsi%ld:%d:%d:%d: %s:
- Mid-layer Data underrun0,
- xferlen = 0x%x,
- residual = 0x%x\n, ha-host_no,
- cmd-device-channel,
- cmd-device-id,
- cmd-device-lun, __func__,
- scsi_bufflen(cmd), residual));
- break;
- }
- }
cmd-result = DID_OK 16 | scsi_status;
if (scsi_status != SCSI_CHECK_CONDITION)
break;
/* Copy Sense Data into sense buffer. */
memset(cmd-sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
sensebytecnt = le16_to_cpu(sts_entry-senseDataByteCnt);
-
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/2] libsas: Provide a transport-level facility to request SAS addrs
Darrick J. Wong wrote: Provide a facility to use the request_firmware() interface to get a SAS address from userspace. This can be used by SAS LLDDs that cannot obtain the address from the host adapter. Resend of 8 Oct. 2007 patch, now based off 2.6.25-rc2 + scsi_misc. Signed-off-by: Darrick J. Wong [EMAIL PROTECTED] --- drivers/scsi/libsas/sas_scsi_host.c | 41 +++ include/scsi/libsas.h |2 ++ 2 files changed, 43 insertions(+), 0 deletions(-) ACK patches 1-2 - 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: ips.c broken since 2.6.23 on x86_64?
On Tue, 19 Feb 2008 10:06:39 -0600 James Bottomley [EMAIL PROTECTED] wrote: On Tue, 2008-02-19 at 17:02 +0900, FUJITA Tomonori wrote: ips did scsi_add_host(sh, NULL) so scsi_dma_map uses shost_gendev.parent that isn't initialized properly, then the kernel crashes. 2.6.23 and 2.6.24 have this bug. We can fix this by calling scsi_add_host with pdev-dev, in the standard way (like the following way) but this bug was fixed in the current Linus tree by: commit 2551a13e61d3c3df6c2da6de5a3ece78e6d67111 Author: Jeff Garzik [EMAIL PROTECTED] Date: Thu Dec 13 16:14:10 2007 -0800 [SCSI] ips: handle scsi_add_host() failure, and other err cleanups James, the legitimate way to fix stable trees is sending this commit (not sending a patch that was not committed upstream)? Well, the upstream patch doesn't look so bad as a stable candidate to my eye. Just because it's an unintended bugfix doesn't automatically invalidate it. The reason stable likes backports of existing upstream patches is because they've supposedly been well tested in upstream. Although that doesn't apply in this case because the other bug rather prevented testing, the principle is still sound. So, would there be any problems simply backporting this? Thanks, I see. There is no problem. Please backport this. - 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: USB regression (and other failures) in 2.6.2[45]* - mostly resolved
On 2/16/2008 10:35 PM, Alan Stern wrote: On Sat, 16 Feb 2008, Andrew Buehler wrote: Until this thread, I was not even aware that ACPI was related to USB; I had largely conflated it with a similar acronym which I think is related to power management and which I can suddenly not even find in my kernel config. I will, however, look into linux-acpi. ACPI isn't directly related to USB; rather it has to do with transferring information between the OS and the BIOS/vendor-specific-hardware. Power management is example where such a transfer is needed. In your case, the relevant information is which IRQ is connected to which motherboard device. If you don't have ACPI enabled in your configuration, then perhaps that's the problem -- try enabling it. Apparently it was the problem; enabling ACPI has fixed not only the USB problem but also the network problem (somewhat miraculously, since I'm quite certain that I had ACPI enabled in a 2.6.23.x kernel where the network did not work despite an apparently matching driver). I feel somewhat foolish for having reported a regression over what turns out to have been a simple misconfiguration, but I still do think it's somewhat misleading at best for something so potentially important to completely non-power-related things to be buried under the heading of power management... I would suggest moving it somewhere else in the config and the dependencies, except that I have neither a suggestion for a possible place nor any idea of how much actual work that would involve. With those two problems out of the way, what is left is the hard-drive issue, and that is also halfway fixed by enabling ACPI. Specifically, it is fixed in that the kernel sees the hard drive and I can mount it, but it is not fixed in that the program we need to use in this environment does not see the drive. I have a config from a boot disc running 2.6.5 (that's not a typo) under which the program in question *does* see the drive, but there are massive differences between that config and the one I am using now, and narrowing the critical difference down is likely to be somewhat difficult - particularly since some of the differences are merely renamed config symbols (i.e. the CONFIG_SCSI_SATA_*-CONFIG_SATA_* switchover), and I have limited ability to tell which without intensive investigation. Are there any established techniques for simplifying this kind of comparison? -- Andrew Buehler - 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: arcmsr + archttp64 calls dma_free_coherent() with irqs disabled - dmesg filled with warnings
I have a test machine running:
0001-arcmsr-fix-message-allocation.patch
arcmsr-pci-alloc-consistent.patch
2.6.24-gentoo-r2
The arcmsr driver is loaded and archttp64 is running. I've been access
archttp64 and there are no warnings in dmesg. Without the patches this
would have generated 10s of MB of warning messages. I haven't really
exercised the areca volumes yet but this looks to be both a fix for the
warnings and James' coding issues. I'd vote for re-diffing and kicking
the patch upstream at this point.
-J
--
On Sat, Feb 16, 2008 at 11:37:41PM +, Daniel Drake wrote:
Daniel Drake wrote:
Here is a patch to address your comments.
Joshua, would you mind testing this before I submit it properly? It will
apply cleanly to 2.6.24 on top of the previous patch you tested. I have
compile-tested it.
It would help to include the patch.
From 0a9cd6133fe4f0c3a8906d6be1b9d1ef083345dc Mon Sep 17 00:00:00 2001
From: Daniel Drake [EMAIL PROTECTED]
Date: Sat, 16 Feb 2008 23:25:02 +
Subject: [PATCH] arcmsr: fix message allocation
---
drivers/scsi/arcmsr/arcmsr_hba.c | 26 +++---
1 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c
b/drivers/scsi/arcmsr/arcmsr_hba.c
index 4f9ff32..f91f79c 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -1387,18 +1387,16 @@ static int arcmsr_iop_message_xfer(struct
AdapterControlBlock *acb, \
switch(controlcode) {
case ARCMSR_MESSAGE_READ_RQBUFFER: {
- unsigned long *ver_addr;
+ unsigned char *ver_addr;
uint8_t *pQbuffer, *ptmpQbuffer;
int32_t allxfer_len = 0;
- void *tmp;
- tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);
- ver_addr = (unsigned long *)tmp;
- if (!tmp) {
+ ver_addr = kmalloc(1032, GFP_ATOMIC);
+ if (!ver_addr) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
- ptmpQbuffer = (uint8_t *) ver_addr;
+ ptmpQbuffer = ver_addr;
while ((acb-rqbuf_firstindex != acb-rqbuf_lastindex)
(allxfer_len 1031)) {
pQbuffer = acb-rqbuffer[acb-rqbuf_firstindex];
@@ -1427,26 +1425,24 @@ static int arcmsr_iop_message_xfer(struct
AdapterControlBlock *acb, \
}
arcmsr_iop_message_read(acb);
}
- memcpy(pcmdmessagefld-messagedatabuffer, (uint8_t *)ver_addr,
allxfer_len);
+ memcpy(pcmdmessagefld-messagedatabuffer, ver_addr,
allxfer_len);
pcmdmessagefld-cmdmessage.Length = allxfer_len;
pcmdmessagefld-cmdmessage.ReturnCode =
ARCMSR_MESSAGE_RETURNCODE_OK;
- kfree(tmp);
+ kfree(ver_addr);
}
break;
case ARCMSR_MESSAGE_WRITE_WQBUFFER: {
- unsigned long *ver_addr;
+ unsigned char *ver_addr;
int32_t my_empty_len, user_len, wqbuf_firstindex,
wqbuf_lastindex;
uint8_t *pQbuffer, *ptmpuserbuffer;
- void *tmp;
- tmp = kmalloc(1032, GFP_KERNEL|GFP_DMA);
- ver_addr = (unsigned long *)tmp;
- if (!tmp) {
+ ver_addr = kmalloc(1032, GFP_ATOMIC);
+ if (!ver_addr) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
}
- ptmpuserbuffer = (uint8_t *)ver_addr;
+ ptmpuserbuffer = ver_addr;
user_len = pcmdmessagefld-cmdmessage.Length;
memcpy(ptmpuserbuffer, pcmdmessagefld-messagedatabuffer,
user_len);
wqbuf_lastindex = acb-wqbuf_lastindex;
@@ -1492,7 +1488,7 @@ static int arcmsr_iop_message_xfer(struct
AdapterControlBlock *acb, \
retvalue = ARCMSR_MESSAGE_FAIL;
}
}
- kfree(tmp);
+ kfree(ver_addr);
}
break;
--
1.5.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: no INQUIRY from userspace please
Stefan Richter wrote: James Bottomley wrote: It's all very well to say this, but I think if you look at what udev does, you'll find that it uses scsi_id to send a VPD inquiry to the device so it can populate /dev/disk/by-id, so the point is already conceded PS: Alas we don't have a practicable way to know how many of the - doesn't work with Linux but works to some degree with Windows, - doesn't work with a 2.6 based Linux distro but did work with a 2.4 based distro kinds of devices are those with this INQUIRY bug or similar bugs. While non-udev distros slowly went out of fashion on the desktop, there was a certain frequency of reports of the latter kind of FireWire devices, but this was before I became aware of that kind of firmware bug, therefore I don't have any data whether it played a role for these cases. Just a small point here. To my knowledge all devices that understand the SCSI command set will accept a 36 byte INQUIRY (i.e. an INQUIRY with an allocation length of 36). That is a practical consideration since they would crash on Windows otherwise. Now scsi_id (and probably other programs of that ilk (including some of mine)) get excited when they get a response to a 36 byte INQUIRY and go onto ask for VPD page 0x80 (serial number) or page 0x83 (device id) via an INQUIRY command with the EVPD bit set. Now if they were clever they would make sure that the allocation length of the _first_ such VPD inquiry was 36! That way they could have a close look at the response and if it wasn't valid for a VPD page (e.g. requested_VPD_page_num != response[1]) then they would leave the damn device alone **. Special case: removable media Since response[1] of a standard INQUIRY has RMB (removable medium bit) set in its top bit and the rest of the byte reserved, 0x80 is a possibility. That could give a false positive when the serial number VPD page is requested. So try looking for VPD page 0x83 (with alloc_len=36) first. ** The logic here is that a brain dead SCSI target interpreter doesn't look at the EVPD bit and can only handle an allocation length of 36. So if it receives an INQUIRY cdb with the EVPD bit set it will just respond with its standard INQUIRY response. Doug Gilbert - 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
[Bug 10050] New: message/fusion/mptbase.c: fix use-after-free's
http://bugzilla.kernel.org/show_bug.cgi?id=10050 Summary: message/fusion/mptbase.c: fix use-after-free's Product: SCSI Drivers Version: 2.5 KernelVersion: 2.6.25-rc2-git Platform: All OS/Version: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Other AssignedTo: [EMAIL PROTECTED] ReportedBy: [EMAIL PROTECTED] OtherBugsDependingO 9832 nThis: Regression: 1 Subject : message/fusion/mptbase.c: fix use-after-free's Submitter : Adrian Bunk [EMAIL PROTECTED] References : http://lkml.org/lkml/2008/2/19/562 Caused-By : Sathya Prakash [EMAIL PROTECTED] commit e78d5b8f1e73ab82f3fd041d05824cfee7d83a2c Handled-By : Adrian Bunk [EMAIL PROTECTED] Patch : http://lkml.org/lkml/2008/2/19/562 -- Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee. - 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
[Fwd: [2.6 patch] message/fusion/mptbase.c: fix use-after-free's]
Forwarded Message
From: Adrian Bunk [EMAIL PROTECTED]
To: Sathya Prakash [EMAIL PROTECTED], James Bottomley
[EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Subject: [2.6 patch] message/fusion/mptbase.c: fix use-after-free's
Date: Wed, 20 Feb 2008 02:39:43 +0200
This patch fixes two use-after-free's introduced by
commit e78d5b8f1e73ab82f3fd041d05824cfee7d83a2c and spotted by the
Coverity checker.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
drivers/message/fusion/mptbase.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
901baeee449285d231921d0812b64b0f14d794b8 diff --git
a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index bfda731..0c303c8 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1481,15 +1481,15 @@ mpt_attach(struct pci_dev *pdev, const struct
pci_device_id *id)
ioc-bars = pci_select_bars(pdev, IORESOURCE_MEM);
if (pci_enable_device_mem(pdev)) {
- kfree(ioc);
printk(MYIOC_s_ERR_FMT pci_enable_device_mem()
failed\n, ioc-name);
+ kfree(ioc);
return r;
}
if (pci_request_selected_regions(pdev, ioc-bars, mpt)) {
- kfree(ioc);
printk(MYIOC_s_ERR_FMT pci_request_selected_regions() with
MEM failed\n, ioc-name);
+ kfree(ioc);
return r;
}
-
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: [2.6 patch] scsi/qla4xxx/ql4_isr.c: remove dead code
On Tue, 2008-02-19 at 21:29 +0200, Adrian Bunk wrote:
This patch removes dead code spotted by the Coverity checker.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
drivers/scsi/qla4xxx/ql4_isr.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
--- linux-2.6/drivers/scsi/qla4xxx/ql4_isr.c.old 2008-02-19
20:29:16.0 +0200
+++ linux-2.6/drivers/scsi/qla4xxx/ql4_isr.c 2008-02-19 20:30:37.0
+0200
@@ -91,38 +91,22 @@ static void qla4xxx_status_entry(struct
if (scsi_status == 0) {
cmd-result = DID_OK 16;
break;
}
if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_OVER) {
cmd-result = DID_ERROR 16;
break;
}
- if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_UNDER) {
+ if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_UNDER)
scsi_set_resid(cmd, residual);
- if (!scsi_status ((scsi_bufflen(cmd) - residual)
- cmd-underflow)) {
-
- cmd-result = DID_ERROR 16;
-
- DEBUG2(printk(scsi%ld:%d:%d:%d: %s:
- Mid-layer Data underrun0,
- xferlen = 0x%x,
- residual = 0x%x\n, ha-host_no,
- cmd-device-channel,
- cmd-device-id,
- cmd-device-lun, __func__,
- scsi_bufflen(cmd), residual));
- break;
- }
- }
This code doesn't look dead to me, it looks to be enforcing
cmd-underrun if set ... what makes the coverity checker think it can
never be executed?
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: [2.6 patch] scsi/qla4xxx/ql4_isr.c: remove dead code
On Tue, 19 Feb 2008, James Bottomley wrote:
On Tue, 2008-02-19 at 21:29 +0200, Adrian Bunk wrote:
This patch removes dead code spotted by the Coverity checker.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
drivers/scsi/qla4xxx/ql4_isr.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
--- linux-2.6/drivers/scsi/qla4xxx/ql4_isr.c.old2008-02-19
20:29:16.0 +0200
+++ linux-2.6/drivers/scsi/qla4xxx/ql4_isr.c2008-02-19
20:30:37.0 +0200
@@ -91,38 +91,22 @@ static void qla4xxx_status_entry(struct
if (scsi_status == 0) {
cmd-result = DID_OK 16;
break;
}
if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_OVER) {
cmd-result = DID_ERROR 16;
break;
}
- if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_UNDER) {
+ if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_UNDER)
scsi_set_resid(cmd, residual);
- if (!scsi_status ((scsi_bufflen(cmd) - residual)
- cmd-underflow)) {
-
- cmd-result = DID_ERROR 16;
-
- DEBUG2(printk(scsi%ld:%d:%d:%d: %s:
- Mid-layer Data underrun0,
- xferlen = 0x%x,
- residual = 0x%x\n, ha-host_no,
- cmd-device-channel,
- cmd-device-id,
- cmd-device-lun, __func__,
- scsi_bufflen(cmd), residual));
- break;
- }
- }
This code doesn't look dead to me, it looks to be enforcing
cmd-underrun if set ... what makes the coverity checker think it can
never be executed?
Hmm, guess it's the earlier 'if (scsi_status == 0)' check a few lines
up... Dave S., can you take a look at this... Thanks, av
-
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: [2.6 patch] scsi/qla4xxx/ql4_isr.c: remove dead code
On Tue, 2008-02-19 at 18:35 -0800, Andrew Vasquez wrote:
On Tue, 19 Feb 2008, James Bottomley wrote:
On Tue, 2008-02-19 at 21:29 +0200, Adrian Bunk wrote:
This patch removes dead code spotted by the Coverity checker.
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
drivers/scsi/qla4xxx/ql4_isr.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
--- linux-2.6/drivers/scsi/qla4xxx/ql4_isr.c.old 2008-02-19
20:29:16.0 +0200
+++ linux-2.6/drivers/scsi/qla4xxx/ql4_isr.c 2008-02-19
20:30:37.0 +0200
@@ -91,38 +91,22 @@ static void qla4xxx_status_entry(struct
if (scsi_status == 0) {
cmd-result = DID_OK 16;
break;
}
if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_OVER) {
cmd-result = DID_ERROR 16;
break;
}
- if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_UNDER) {
+ if (sts_entry-iscsiFlags ISCSI_FLAG_RESIDUAL_UNDER)
scsi_set_resid(cmd, residual);
- if (!scsi_status ((scsi_bufflen(cmd) - residual)
- cmd-underflow)) {
-
- cmd-result = DID_ERROR 16;
-
- DEBUG2(printk(scsi%ld:%d:%d:%d: %s:
- Mid-layer Data underrun0,
- xferlen = 0x%x,
- residual = 0x%x\n, ha-host_no,
- cmd-device-channel,
- cmd-device-id,
- cmd-device-lun, __func__,
- scsi_bufflen(cmd), residual));
- break;
- }
- }
This code doesn't look dead to me, it looks to be enforcing
cmd-underrun if set ... what makes the coverity checker think it can
never be executed?
Hmm, guess it's the earlier 'if (scsi_status == 0)' check a few lines
up... Dave S., can you take a look at this... Thanks, av
Ah, so the !scsi_status is wrong it was supposed to be scsi_status !=
0 ... and even then it can just be dropped.
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: + mpt-fusion-dont-oops-if-numphys==0.patch added to -mm tree
On Tue, 19 Feb 2008, James Bottomley wrote:
On Tue, 2008-02-19 at 15:34 -0800, [EMAIL PROTECTED] wrote:
Subject: mpt fusion: don't oops if NumPhys==0
From: Krzysztof Oledzki [EMAIL PROTECTED]
Don't oops if NumPhys==0, instead return -ENODEV.
This patch fixes http://bugzilla.kernel.org/show_bug.cgi?id=9909
Signed-off-by: Krzysztof Piotr Oledzki [EMAIL PROTECTED]
Cc: FUJITA Tomonori [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
Cc: Eric Moore [EMAIL PROTECTED]
Cc: Sathya Prakash [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---
drivers/message/fusion/mptsas.c |5 +
1 file changed, 5 insertions(+)
diff -puN drivers/message/fusion/mptsas.c~mpt-fusion-dont-oops-if-numphys==0
drivers/message/fusion/mptsas.c
--- a/drivers/message/fusion/mptsas.c~mpt-fusion-dont-oops-if-numphys==0
+++ a/drivers/message/fusion/mptsas.c
@@ -1701,6 +1701,11 @@ mptsas_sas_expander_pg0(MPT_ADAPTER *ioc
if (error)
goto out_free_consistent;
+ if (!buffer-NumPhys) {
+ error = -ENODEV;
+ goto out_free_consistent;
+ }
+
/* save config data */
port_info-num_phys = buffer-NumPhys;
port_info-phy_info = kcalloc(port_info-num_phys,
_
This is a bit odd ... It looks like a work around for a fusion firmware
problem.
Workaround or not, the driver should not oops by allocating 0 size memory
block and dereferencing it. It used to work in 2.6.23 but not now.
What's the topology this thing is connected to?
It is a Dell-1950III server with two 400GB SAS disks (RAID1) installed in
a dwo disk SAS backplane.
Best regrds,
Krzysztof Olędzki
Re: + mpt-fusion-dont-oops-if-numphys==0.patch added to -mm tree
On Wed, 2008-02-20 at 04:17 +0100, Maximilian Wilhelm wrote: Am Wednesday, den 20 February hub Krzysztof Oledzki folgendes in die Tasten: What's the topology this thing is connected to? It is a Dell-1950III server with two 400GB SAS disks (RAID1) installed in a dwo disk SAS backplane. Here it's a Dell 1950 with two 73G SAS disks (RAID1) but having four SAS disk slots. If you need further information about the hardware, just let me know. Is that directly connected or via an internal expander? 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: aic94xx: failing on high load (another data point)
On Tue, 2008-02-19 at 10:22 -0600, James Bottomley wrote:
I'll see if I can come up with patches to fix this ... or at least
mitigate the problems it causes.
Darrick's working on the ascb sequencer use after free problem.
I looked into some of the error handling in libsas, and apparently
that's a bit of a huge screw up too. There are a number of places where
we won't complete a task that is being errored out and thus causes
timeout errors. This patch is actually for libsas to fix all of this.
I've managed to reproduce some of your problem by firing random resets
across a disk under load, and this recovers the protocol errors for me.
However, I can't reproduce the TMF timeout which caused the sequencer
screw up, so you still need to wait for Darrick's fix as well.
James
---
diff --git a/drivers/scsi/libsas/sas_scsi_host.c
b/drivers/scsi/libsas/sas_scsi_host.c
index f869fba..b656e29 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -51,8 +51,6 @@ static void sas_scsi_task_done(struct sas_task *task)
{
struct task_status_struct *ts = task-task_status;
struct scsi_cmnd *sc = task-uldd_task;
- struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(sc-device-host);
- unsigned ts_flags = task-task_state_flags;
int hs = 0, stat = 0;
if (unlikely(!sc)) {
@@ -120,11 +118,7 @@ static void sas_scsi_task_done(struct sas_task *task)
sc-result = (hs 16) | stat;
list_del_init(task-list);
sas_free_task(task);
- /* This is very ugly but this is how SCSI Core works. */
- if (ts_flags SAS_TASK_STATE_ABORTED)
- scsi_eh_finish_cmd(sc, sas_ha-eh_done_q);
- else
- sc-scsi_done(sc);
+ sc-scsi_done(sc);
}
static enum task_attribute sas_scsi_get_task_attr(struct scsi_cmnd *cmd)
@@ -255,13 +249,27 @@ out:
return res;
}
+static void sas_eh_finish_cmd(struct scsi_cmnd *cmd)
+{
+ struct sas_task *task = TO_SAS_TASK(cmd);
+ struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(cmd-device-host);
+
+ /* First off call task_done. However, task will
+* be free'd after this */
+ task-task_done(task);
+ /* now finish the command and move it on to the error
+* handler done list, this also takes it off the
+* error handler pending list */
+ scsi_eh_finish_cmd(cmd, sas_ha-eh_done_q);
+}
+
static void sas_scsi_clear_queue_lu(struct list_head *error_q, struct
scsi_cmnd *my_cmd)
{
struct scsi_cmnd *cmd, *n;
list_for_each_entry_safe(cmd, n, error_q, eh_entry) {
if (cmd == my_cmd)
- list_del_init(cmd-eh_entry);
+ sas_eh_finish_cmd(cmd);
}
}
@@ -274,7 +282,7 @@ static void sas_scsi_clear_queue_I_T(struct list_head
*error_q,
struct domain_device *x = cmd_to_domain_dev(cmd);
if (x == dev)
- list_del_init(cmd-eh_entry);
+ sas_eh_finish_cmd(cmd);
}
}
@@ -288,7 +296,7 @@ static void sas_scsi_clear_queue_port(struct list_head
*error_q,
struct asd_sas_port *x = dev-port;
if (x == port)
- list_del_init(cmd-eh_entry);
+ sas_eh_finish_cmd(cmd);
}
}
@@ -528,14 +536,14 @@ Again:
case TASK_IS_DONE:
SAS_DPRINTK(%s: task 0x%p is done\n, __FUNCTION__,
task);
- task-task_done(task);
+ sas_eh_finish_cmd(cmd);
if (need_reset)
try_to_reset_cmd_device(shost, cmd);
continue;
case TASK_IS_ABORTED:
SAS_DPRINTK(%s: task 0x%p is aborted\n,
__FUNCTION__, task);
- task-task_done(task);
+ sas_eh_finish_cmd(cmd);
if (need_reset)
try_to_reset_cmd_device(shost, cmd);
continue;
@@ -547,7 +555,7 @@ Again:
recovered\n,
SAS_ADDR(task-dev),
cmd-device-lun);
- task-task_done(task);
+ sas_eh_finish_cmd(cmd);
if (need_reset)
try_to_reset_cmd_device(shost, cmd);
sas_scsi_clear_queue_lu(work_q, cmd);
@@ -562,7 +570,7 @@ Again:
if (tmf_resp == TMF_RESP_FUNC_COMPLETE) {
SAS_DPRINTK(I_T %016llx recovered\n,
SAS_ADDR(task-dev-sas_addr));
- task-task_done(task);
+
Re: + mpt-fusion-dont-oops-if-numphys==0.patch added to -mm tree
Am Wednesday, den 20 February hub Krzysztof Oledzki folgendes in die Tasten:
Hi Krzysztof, *
Thanks for CC'ing me.
On Tue, 2008-02-19 at 15:34 -0800, [EMAIL PROTECTED] wrote:
Subject: mpt fusion: don't oops if NumPhys==0
From: Krzysztof Oledzki [EMAIL PROTECTED]
Don't oops if NumPhys==0, instead return -ENODEV.
This patch fixes http://bugzilla.kernel.org/show_bug.cgi?id=9909
Signed-off-by: Krzysztof Piotr Oledzki [EMAIL PROTECTED]
Cc: FUJITA Tomonori [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
Cc: Eric Moore [EMAIL PROTECTED]
Cc: Sathya Prakash [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---
drivers/message/fusion/mptsas.c |5 +
1 file changed, 5 insertions(+)
diff -puN
drivers/message/fusion/mptsas.c~mpt-fusion-dont-oops-if-numphys==0
drivers/message/fusion/mptsas.c
--- a/drivers/message/fusion/mptsas.c~mpt-fusion-dont-oops-if-numphys==0
+++ a/drivers/message/fusion/mptsas.c
@@ -1701,6 +1701,11 @@ mptsas_sas_expander_pg0(MPT_ADAPTER *ioc
if (error)
goto out_free_consistent;
+ if (!buffer-NumPhys) {
+ error = -ENODEV;
+ goto out_free_consistent;
+ }
+
/* save config data */
port_info-num_phys = buffer-NumPhys;
port_info-phy_info = kcalloc(port_info-num_phys,
_
This is a bit odd ... It looks like a work around for a fusion firmware
problem.
Workaround or not, the driver should not oops by allocating 0 size memory
block and dereferencing it. It used to work in 2.6.23 but not now.
ACK.
Nevertheless my server is working absolutely well with this
fix/workaround. No problems at all.
What's the topology this thing is connected to?
It is a Dell-1950III server with two 400GB SAS disks (RAID1) installed in
a dwo disk SAS backplane.
Here it's a Dell 1950 with two 73G SAS disks (RAID1) but having four
SAS disk slots.
If you need further information about the hardware, just let me know.
Ciao
Max
--
Follow the white penguin.
-
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: Integration of SCST in the mainstream Linux kernel
Bart Van Assche wrote: On Feb 18, 2008 10:43 AM, Erez Zilber [EMAIL PROTECTED] wrote: If you use a high value for FirstBurstLength, all (or most) of your data will be sent as unsolicited data-out PDUs. These PDUs don't use the RDMA engine, so you miss the advantage of IB. Hello Erez, Did you notice the e-mail Roland Dreier wrote on Februari 6, 2008 ? This is what Roland wrote: I think the confusion here is caused by a slight misuse of the term RDMA. It is true that all data is always transported over an InfiniBand connection when iSER is used, but not all such transfers are one-sided RDMA operations; some data can be transferred using send/receive operations. Yes, I saw that. I tried to give an explanation with more details. Or: data sent during the first burst is not transferred via one-sided remote memory reads or writes but via two-sided send/receive operations. At least on my setup, these operations are as fast as one-sided remote memory reads or writes. As an example, I obtained the following numbers on my setup (SDR 4x network); ib_write_bw: 933 MB/s. ib_read_bw: 905 MB/s. ib_send_bw: 931 MB/s. According to these numbers one can think that you don't need RDMA at all, just send iSCSI PDUs over IB. The benchmarks that you use are synthetic IB benchmarks that are not equivalent to iSCSI over iSER. They just send IB packets. I'm not surprised that you got more or less the same performance because, AFAIK, ib_send_bw doesn't copy data (unlike iSCSI that has to copy data that is sent/received without RDMA). When you use RDMA with iSCSI (i.e. iSER), you don't need to create iSCSI PDUs and process them. The CPU is not busy as it is with iSCSI over TCP because no data copies are required. Another advantage is that you don't need header/data digest because the IB HW does that. Erez - 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
