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 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=en&lnk=gst&q=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. -- Jens Axboe - 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=en&lnk=gst&q=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: fix data buffer accessors conversion bug
ACK Wonderful work Tom and Fujita-san! Sincerely -- Mark Salyzyn > -Original Message- > From: FUJITA Tomonori [mailto:[EMAIL PROTECTED] > Sent: Tuesday, February 19, 2008 3:02 AM > To: linux-scsi@vger.kernel.org > Cc: [EMAIL PROTECTED]; Salyzyn, Mark; > [EMAIL PROTECTED]; [EMAIL PROTECTED] > Subject: [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: [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 Sun, 17 Feb 2008 23:36:59 -0700 Matthew Wilcox <[EMAIL PROTECTED]> wrote: > > This is the first release of a driver we've been using internally at Intel > for a few weeks which is as low-latency as possible. It's designed to > let us find latency issues elsewhere in the storage stack (eg filesystem, > block layer, scsi layer) > > There are a few different options, controlled through module parameters. > The sector size and disc capacity are load-time parameters, but the > parameters affecting performance are tweakable at runtime. > > Arguably, this driver should be merged into scsi_debug. I didn't want to > trip over Doug's toes while developing this driver, and the two drivers > do have very different objectives. scsi_debug is obviously a lot more > fully-featured and implements lots of bits of the spec I simply haven't > bothered with. Like MODE SENSE ;-) 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. If we clean up scsi_debug driver, can scsi_debug work as a nice scsi ram 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: 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: gdth new set of patches for 2.6.24 stable
On Mon, Feb 18, 2008 at 12:27:42PM -0800, David Brownell wrote: > > Actually I think David might have been wrong about mips. afaict its > > dma_free_coherent() is callable under local_irq_disable(), so ARM SMP is > > the sole exception? > > All I recall at this point was getting some arch-specific patches in that > area; I thought it was MIPS, maybe it was PPC. The arch code may have > changed since then too. Mostly to avoid code duplication on the MIPS side but the core of the implementation has always been similar to what it is currently. Ralf - 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=en&lnk=gst&q=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: aic94xx: failing on high load (another data point)
On Mon, 2008-02-18 at 22:26 +0800, Keith Hopkins wrote: > Well, that made life interesting > but didn't seem to fix anything. > > The behavior is about the same as before, but with more verbose > errors. I failed one member of the raid and had it rebuild as a > test...which hangs for a while and the drive falls off-line. > > Please grab the dmesg output in all its gory glory from here: > http://wiki.hopnet.net/dokuwiki/lib/exe/fetch.php?media=myit:sas:dmesg-20080218-wpatch-fail.txt.gz I had a look through this. Amazingly, in spite of the message spew, up to here: > sas: Enter sas_scsi_recover_host > sas: trying to find task 0x81033c3d3d80 > sas: sas_scsi_find_task: aborting task 0x81033c3d3d80 > aic94xx: tmf timed out > aic94xx: tmf came back Everything is going normally (the REQ_TASK_ABORT are properly aborted an retried). At this point (around L3449 in the trace) the aborts start failing. 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. 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: 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 +#include +#include #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] = (h<<4) | 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] = (h<<4) | 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 #include #include +#include #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: + mpt-fusion-dont-oops-if-numphys==0.patch added to -mm tree
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. What's the topology this thing is connected to? 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, 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: [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 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.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 > > Ah, so the !scsi_status is wrong it was supposed to be scsi_status != > 0 ... and even then it can just be dropped. My guess is that the check should have been written as: ... if (sts_entry->iscsiFlags &ISCSI_FLAG_RESIDUAL_UNDER) scsi_set_resid(cmd, residual); if ((scsi_bufflen(cmd) - residual) < cmd->underflow) { ... It looks to be a logic-error while porting from qla2xxx, where scsi_status during CS_COMPLETE is the full 16-bit status (high-byte is transport, low-byte SCSI status) from from the FCP_RSP frame (not so in iSCSI, where it's just the SCSI-status) and the residual check in qla_isr.c::qla2x00_status_entry() looks like: if (!lscsi_status && ((unsigned)(scsi_bufflen(cp) - resid) < cp->underflow)) { ... I'll defer to Dave S. for verification. - 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)); -
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