Re: [SCSI] sun3x_esp: convert to esp_scsi
On vr, 2008-02-08 at 09:33 +0100, Geert Uytterhoeven wrote: On Fri, 8 Feb 2008, Linux Kernel Mailing List wrote: Gitweb: http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=0bb67f181834044db6e9b15c7d5cc3cce0489bfd Commit: 0bb67f181834044db6e9b15c7d5cc3cce0489bfd Parent: f8d9d654fcc7dd87f5d0b222e233eaab15d650c4 Author: Thomas Bogendoerfer [EMAIL PROTECTED] AuthorDate: Fri Feb 1 00:13:34 2008 +0100 Committer: James Bottomley [EMAIL PROTECTED] CommitDate: Thu Feb 7 18:02:33 2008 -0600 [SCSI] sun3x_esp: convert to esp_scsi Converted sun3x_esp driver to use esp_scsi.c Signed-off-by: Thomas Bogendoerfer [EMAIL PROTECTED] Signed-off-by: James Bottomley [EMAIL PROTECTED] --- drivers/scsi/Kconfig |1 + drivers/scsi/Makefile|2 +- drivers/scsi/sun3x_esp.c | 708 +- 3 files changed, 318 insertions(+), 393 deletions(-) But the first one is back! Thank you, Thomas! Thomas, can't you use ioreadxx() and friends instead of rolling your own memory mapped I/O handlers? readxx() and friends are only to be used on PCI-like buses. Unfortunately, a simple conversion like this didn't work for me on the Blizzard SCSI IV. The driver gets stuck when probing devices: esp: esp0, regs[80ea8000:80eb] irq[2] esp: esp0 is a 53CF94/96-2, 40 MHz (ccf=0), SCSI ID 7 scsi0 : esp ESP: tgt[0] lun[0] scsi_cmd [ 12 00 00 00 24 00 ] ESP: start data addr[78e76590] len[36] write(1) ESP: data done flgs[1] sent[36] ESP: Command done status[0] message[0] ESP: tgt[0] lun[0] scsi_cmd [ 12 00 00 00 86 00 ] ESP: start data addr[78e76590] len[134] write(1) ESP: data done flgs[1] sent[134] ESP: Command done status[0] message[0] scsi 0:0:0:0: Direct-Access QUANTUM FIREBALL_TM2110S 300N PQ: 0 ANSI: 2 target0:0:0: Beginning Domain Validation ESP: tgt[0] lun[0] scsi_cmd [ 12 00 00 00 86 00 ] ESP: Got msgin byte 4 ESP: Disconnecting tgt[0] tag[20:0] ESP: reconnect tag, IRQ(0:10:97), 3esp: esp0: Reconnect IRQ2 timeout ESP: esp_schedule_reset() from 00127ad2 target0:0:0: Domain Validation Initial Inquiry Failed target0:0:0: Ending Domain Validation ESP: tgt[1] lun[0] scsi_cmd [ 12 00 00 00 24 00 ] ESP: tgt[2] lun[0] scsi_cmd [ 12 00 00 00 24 00 ] ESP: Got msgin byte 7 ESP: Sending message [ 06 ] ... ESP: Got msgin byte 7 ESP: Sending message [ 06 ] ... The Reconnect IRQ2 timeout thing seemed wrong, so I modified it to never timeout there (perhaps the timer was a bit short on m68k). This causes it to get completely stuck, so the chip is apparently not in the mood to generate any interrupt ever. I wonder whether this is because a DMA operation is done with only 2 bytes... The Blizzard DMA engine is rather simplistic (no byte counters). It's also undocumented. The old driver used to use PIO for small transfers (like message in and SCSI commands). David, any ideas on this? Kind regards, Kars. - 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: Investigating potential flaw in scsi error handling
James Bottomley [EMAIL PROTECTED] wrote: On Sat, 2008-02-09 at 22:59 +0100, Elias Oltmanns wrote: Hi there, I'm experiencing system lockups with 2.6.24 which I believe to be related to scsi error handling. Actually, I have patched the mainline kernel with a disk shock protection patch [1] and in my case it is indeed the shock protection mechanism that triggers the lockups. However, some rather lengthy investigations have lead me to the conclusion that this additional patch is just the means to reproduce the error condition fairly reliably rather than the origin of the problem. The problem has only become apparent since Tejun's commit 31cc23b34913bc173680bdc87af79e551bf8cc0d. More precisely, libata now sets max_host_blocked and max_device_blocked to 1 for all ATA devices. Various tests I've conducted so far have lead me to the conclusion that a non zero return code from scsi_dispatch_command is sufficient to trigger the problem I'm seeing provided that max_host_blocked and max_device_blocked are set to 1. There's nothing inherently incorrect with setting max_device_blocked to 1 but it is suboptimal: it means that for a single queue device returning a wait causes an immediate reissue. Thanks for rubbing that in again. It should have been clear to me all along but I've only just realised the consequences and found the problem, I think. We are, in fact, faced with a situation where the -request_fn() is being called recursively forever. Consider this: The -request_fn() of a single queue device is called which in turn calls scsi_dispatch_cmd(). Assume that the device is either in SDEV_BLOCK state or -queuecommand() returns SCSI_MLQUEUE_DEVICE_BUSY for some reason. In either case scsi_queue_insert() will be called. Eventually, blk_run_queue() will be called with the same device queue not plugged yet. This way we directly reenter q-request_fn(). Now, remember that libata sets sdev-max_device_blocked to 1. Consequently, the function scsi_dev_queue_ready() will immediately give a positive response and we go ahead calling scsi_dispatch_cmd() again. Note that at this stage the lld will not have had a chance yet to clear the SDEV_BLOCK state or the condition that caused the SCSI_MLQUEUE_DEVICE_BUSY return code from -queuecommand(). Hence the infinite recursion. A similar recursion can also occur due to a SCSI_MLQUEUE_HOST_BUSY response from -queuecommand(). Unless I have overlooked some unwanted implications, please consider applying the patch that I'm going to send you as a follow up to this email. Regards, Elias - 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/arm/fas216.c compile error
On Sat, Feb 09 2008 at 2:04 +0200, Adrian Bunk [EMAIL PROTECTED] wrote: Commit 30b0c37b27485a9cb897bfe3824f6f517b8c80d6 causes the following compile error: -- snip -- ... CC drivers/scsi/arm/fas216.o /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c: In function 'fas216_std_done': /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2120: error: 'struct scsi_cmnd' has no member named 'request_bufflen' /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2122: error: 'struct scsi_cmnd' has no member named 'use_sg' make[4]: *** [drivers/scsi/arm/fas216.o] Error 1 -- snip -- cu Adrian It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310 [SCSI] arm: convert to accessors and !use_sg cleanup Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable to get a responsive maintainer to ACK on them. until the breakage cause went into mainline we finally managed a Tested-by:. I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're schedule. Boaz - 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] Make sure that scsi_request_fn() isn't called recursively forever
Currently, scsi_dev_queue_ready() and scsi_host_queue_ready() decrease the
device_blocked or host_blocked counter respectively *before* they determine
the right return value. If the device can't accept a request for some
reason and max_host_blocked or max_device_blocked has been set to 1, this
may lead to scsi_request_fn() being called recursively without giving the
low level driver a chance to unjam the device.
This patch applies to 2.6.24. Please include it in the stable updates as
well.
Signed-off-by: Elias Oltmanns [EMAIL PROTECTED]
---
drivers/scsi/scsi_lib.c | 20
1 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a9ac5b1..7513bed 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1357,14 +1357,12 @@ static inline int scsi_dev_queue_ready(s
/*
* unblock after device_blocked iterates to zero
*/
- if (--sdev-device_blocked == 0) {
+ if (sdev-device_blocked-- == 1)
SCSI_LOG_MLQUEUE(3,
sdev_printk(KERN_INFO, sdev,
- unblocking device at zero depth\n));
- } else {
- blk_plug_device(q);
- return 0;
- }
+ device will be unblocked next time\n));
+ blk_plug_device(q);
+ return 0;
}
if (sdev-device_blocked)
return 0;
@@ -1389,14 +1387,12 @@ static inline int scsi_host_queue_ready(
/*
* unblock after host_blocked iterates to zero
*/
- if (--shost-host_blocked == 0) {
+ if (shost-host_blocked-- == 1)
SCSI_LOG_MLQUEUE(3,
- printk(scsi%d unblocking host at zero depth\n,
+ printk(scsi%d host will be unblocked next
time\n,
shost-host_no));
- } else {
- blk_plug_device(q);
- return 0;
- }
+ blk_plug_device(q);
+ return 0;
}
if ((shost-can_queue 0 shost-host_busy = shost-can_queue) ||
shost-host_blocked || shost-host_self_blocked) {
-
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/arm/fas216.c compile error
On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote: On Sat, Feb 09 2008 at 2:04 +0200, Adrian Bunk [EMAIL PROTECTED] wrote: Commit 30b0c37b27485a9cb897bfe3824f6f517b8c80d6 causes the following compile error: -- snip -- ... CC drivers/scsi/arm/fas216.o /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c: In function 'fas216_std_done': /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2120: error: 'struct scsi_cmnd' has no member named 'request_bufflen' /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2122: error: 'struct scsi_cmnd' has no member named 'use_sg' make[4]: *** [drivers/scsi/arm/fas216.o] Error 1 -- snip -- cu Adrian It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310 [SCSI] arm: convert to accessors and !use_sg cleanup Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable to get a responsive maintainer to ACK on them. until the breakage cause went into mainline we finally managed a Tested-by:. I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're schedule. Oh, just 20 minutes in your opinion? Reality works at a different tick rate to your timing then. However, if you're seeing a bulid error, it means that the *WRONG* patch went in. No idea why I even bothered to test it if that's what happens. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - 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/arm/fas216.c compile error
On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote: On Sat, Feb 09 2008 at 2:04 +0200, Adrian Bunk [EMAIL PROTECTED] wrote: Commit 30b0c37b27485a9cb897bfe3824f6f517b8c80d6 causes the following compile error: -- snip -- ... CC drivers/scsi/arm/fas216.o /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c: In function 'fas216_std_done': /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2120: error: 'struct scsi_cmnd' has no member named 'request_bufflen' /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/scsi/arm/fas216.c:2122: error: 'struct scsi_cmnd' has no member named 'use_sg' make[4]: *** [drivers/scsi/arm/fas216.o] Error 1 -- snip -- cu Adrian It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310 [SCSI] arm: convert to accessors and !use_sg cleanup ... fas216.c != acornscsi.c Boaz cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - 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/arm/fas216.c compile error
On Sun, 2008-02-10 at 13:58 +, Russell King wrote:
On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
[SCSI] arm: convert to accessors and !use_sg cleanup
Thanks for checking. This patch was in scsi-pending tree since forever, And
we were unable
to get a responsive maintainer to ACK on them. until the breakage cause
went into mainline
we finally managed a Tested-by:.
I guess sometimes people are so busy, you need a bulldozer to shove 20
minutes into they're
schedule.
Oh, I was ill for most of December, particularly at the time that you
sent the patch, and by the time I recovered, it was buried in my mailbox.
Suggest you have some consideration for others who might not be able to
do your beg and call at the immediate moment that you want it, and
consider that their email management skills may not be as l33t as yours.
OK, sorry about this, it's a bit of a cockup all around. The patch that
fixes this problem is still in SCSI pending largely because it's patch
description:
[SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
Doesn't lead one to think it might be build critical, so I concentrated
on getting the other arm patch out.
Russell, could you give it a quick test, and I'll put it in with a
tested-by tag?
Thanks,
James
---
From: Boaz Harrosh [EMAIL PROTECTED]
Date: Mon, 10 Sep 2007 22:39:11 +0300
Subject: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
- Use new scsi_eh_prep/restor_cmnd() for synchronous
REQUEST_SENSE invocation.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
Cc: Russell King [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/arm/fas216.c | 16 +++-
drivers/scsi/arm/fas216.h |3 +++
2 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index fb5f202..a715632 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct
scsi_cmnd *SCpnt,
* the upper layers to process. This would have been set
* correctly by fas216_std_done.
*/
+ scsi_eh_restore_cmnd(SCpnt, info-ses);
SCpnt-scsi_done(SCpnt);
}
@@ -2103,23 +2104,12 @@ request_sense:
if (SCpnt-cmnd[0] == REQUEST_SENSE)
goto done;
+ scsi_eh_prep_cmnd(SCpnt, info-ses, NULL, 0, ~0);
fas216_log_target(info, LOG_CONNECT, SCpnt-device-id,
requesting sense);
- memset(SCpnt-cmnd, 0, sizeof (SCpnt-cmnd));
- SCpnt-cmnd[0] = REQUEST_SENSE;
- SCpnt-cmnd[1] = SCpnt-device-lun 5;
- SCpnt-cmnd[4] = sizeof(SCpnt-sense_buffer);
- SCpnt-cmd_len = COMMAND_SIZE(SCpnt-cmnd[0]);
- SCpnt-SCp.buffer = NULL;
- SCpnt-SCp.buffers_residual = 0;
- SCpnt-SCp.ptr = (char *)SCpnt-sense_buffer;
- SCpnt-SCp.this_residual = sizeof(SCpnt-sense_buffer);
- SCpnt-SCp.phase = sizeof(SCpnt-sense_buffer);
+ init_SCp(SCpnt);
SCpnt-SCp.Message = 0;
SCpnt-SCp.Status = 0;
- SCpnt-request_bufflen = sizeof(SCpnt-sense_buffer);
- SCpnt-sc_data_direction = DMA_FROM_DEVICE;
- SCpnt-use_sg = 0;
SCpnt-tag = 0;
SCpnt-host_scribble = (void *)fas216_rq_sns_done;
diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h
index 00e5f05..3e73e26 100644
--- a/drivers/scsi/arm/fas216.h
+++ b/drivers/scsi/arm/fas216.h
@@ -16,6 +16,8 @@
#define NO_IRQ 255
#endif
+#include scsi/scsi_eh.h
+
#include queue.h
#include msgqueue.h
@@ -311,6 +313,7 @@ typedef struct {
/* miscellaneous */
int internal_done; /* flag to indicate
request done */
+ struct scsi_eh_save *ses; /* holds request sense restore
info */
unsigned long magic_end;
} FAS216_Info;
--
1.5.3.8
-
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/arm/fas216.c compile error
On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote: It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310 [SCSI] arm: convert to accessors and !use_sg cleanup Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable to get a responsive maintainer to ACK on them. until the breakage cause went into mainline we finally managed a Tested-by:. I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're schedule. Oh, I was ill for most of December, particularly at the time that you sent the patch, and by the time I recovered, it was buried in my mailbox. Suggest you have some consideration for others who might not be able to do your beg and call at the immediate moment that you want it, and consider that their email management skills may not be as l33t as yours. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: - 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/arm/fas216.c compile error
On Sun, Feb 10 2008 at 15:58 +0200, Russell King [EMAIL PROTECTED] wrote: On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote: It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310 [SCSI] arm: convert to accessors and !use_sg cleanup Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable to get a responsive maintainer to ACK on them. until the breakage cause went into mainline we finally managed a Tested-by:. I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're schedule. Oh, I was ill for most of December, particularly at the time that you sent the patch, and by the time I recovered, it was buried in my mailbox. Suggest you have some consideration for others who might not be able to do your beg and call at the immediate moment that you want it, and consider that their email management skills may not be as l33t as yours. Dear Russell. You are right. I apologize. I was too trigger happy. I should have resend the request. The patches were in scsi-pending and in -mm. And I assumed it was all good. But it was not accepted into scsi-misc and was somewhat forgotten. I assure you, my email management skills are just as laking as yours. Just that my responsibility's are few. Boaz - 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: Investigating potential flaw in scsi error handling
On Sun, 2008-02-10 at 13:54 +0100, Elias Oltmanns wrote: James Bottomley [EMAIL PROTECTED] wrote: On Sat, 2008-02-09 at 22:59 +0100, Elias Oltmanns wrote: Hi there, I'm experiencing system lockups with 2.6.24 which I believe to be related to scsi error handling. Actually, I have patched the mainline kernel with a disk shock protection patch [1] and in my case it is indeed the shock protection mechanism that triggers the lockups. However, some rather lengthy investigations have lead me to the conclusion that this additional patch is just the means to reproduce the error condition fairly reliably rather than the origin of the problem. The problem has only become apparent since Tejun's commit 31cc23b34913bc173680bdc87af79e551bf8cc0d. More precisely, libata now sets max_host_blocked and max_device_blocked to 1 for all ATA devices. Various tests I've conducted so far have lead me to the conclusion that a non zero return code from scsi_dispatch_command is sufficient to trigger the problem I'm seeing provided that max_host_blocked and max_device_blocked are set to 1. There's nothing inherently incorrect with setting max_device_blocked to 1 but it is suboptimal: it means that for a single queue device returning a wait causes an immediate reissue. Thanks for rubbing that in again. It should have been clear to me all along but I've only just realised the consequences and found the problem, I think. We are, in fact, faced with a situation where the -request_fn() is being called recursively forever. This happens on a device_max_blocked of 1 if there's a programmatic defer return of non zero at zero outstanding commands. If you want to set a max blocked of one, you need to ensure that doesn't happen. Consider this: The -request_fn() of a single queue device is called which in turn calls scsi_dispatch_cmd(). Assume that the device is either in SDEV_BLOCK state or -queuecommand() returns SCSI_MLQUEUE_DEVICE_BUSY for some reason. In either case scsi_queue_insert() will be called. Eventually, blk_run_queue() will be called with the same device queue not plugged yet. This way we directly reenter q-request_fn(). Now, remember that libata sets sdev-max_device_blocked to 1. Consequently, the function scsi_dev_queue_ready() will immediately give a positive response and we go ahead calling scsi_dispatch_cmd() again. Note that at this stage the lld will not have had a chance yet to clear the SDEV_BLOCK state or the condition that caused the SCSI_MLQUEUE_DEVICE_BUSY return code from -queuecommand(). Hence the infinite recursion. A similar recursion can also occur due to a SCSI_MLQUEUE_HOST_BUSY response from -queuecommand(). Unless I have overlooked some unwanted implications, please consider applying the patch that I'm going to send you as a follow up to this email. No. We have a fix for this, it's called setting device_max_blocked to 2 or greater. All your patch does is make this seem to be the case, plus it eliminates the instant reissue case for drivers with queuecommands that do obey all the rules. If you can prove that IDE doesn't obey the rules (no defer returns) then ask them to revert their setting of device_max_blocked to 1; if they do, then you have to come up with an alternative for your patch. James 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: Current git -- kaboom [bisect] seems IDE related.
On Sun, Feb 10, 2008 at 02:38:46PM +0100, Bartlomiej Zolnierkiewicz wrote:
The OOPS is most likely (again) my fault - I was rushing out to push out
the fix and memset() line didn't get converted.
The new patch works fine for me.
I prepared the new patch, documented it and started looking into SCSI
build breakage... and I no longer feel comfortable with the hack :(
It seems that fixing IDE properly will be easier than auditing the whole
SCSI for all the weird assumptions on rq-cmd[] size (James?) so I'm back
to the code, in the meantime here's the updated patch:
Yeah, this is quite nasty. I'll attach the patch below which just
rejects a command in scsi_setup_blk_pc_cmnd if it's too large for
the scsi_cmnd cmnd array. This is probably enough but I haven't
audited all of the scsi code yet. But as James said this is
too much of a memory vastage to put it into the tree.
Long-term the Panasas folks have looked into killing the scsi_cmnd.cmnd
filed entirely and make the struct request.cmd field dynamically sized
which would solve your problem, but probably won't be ready for 2.6.25.
Index: linux-2.6/drivers/scsi/scsi_lib.c
===
--- linux-2.6.orig/drivers/scsi/scsi_lib.c 2008-02-10 07:49:50.0
+0100
+++ linux-2.6/drivers/scsi/scsi_lib.c 2008-02-10 15:19:42.0 +0100
@@ -1129,7 +1129,12 @@ int scsi_setup_blk_pc_cmnd(struct scsi_d
req-buffer = NULL;
}
- BUILD_BUG_ON(sizeof(req-cmd) sizeof(cmd-cmnd));
+ if (req-cmd_len sizeof(cmd-cmnd)) {
+ scsi_release_buffers(cmd);
+ scsi_put_command(cmd);
+ return BLKPREP_KILL;
+ }
+
memcpy(cmd-cmnd, req-cmd, sizeof(cmd-cmnd));
cmd-cmd_len = req-cmd_len;
if (!req-data_len)
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi/arm/fas216.c compile error
On Sun, Feb 10 2008 at 16:20 +0200, James Bottomley [EMAIL PROTECTED] wrote: On Sun, 2008-02-10 at 13:58 +, Russell King wrote: On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote: It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310 [SCSI] arm: convert to accessors and !use_sg cleanup Thanks for checking. This patch was in scsi-pending tree since forever, And we were unable to get a responsive maintainer to ACK on them. until the breakage cause went into mainline we finally managed a Tested-by:. I guess sometimes people are so busy, you need a bulldozer to shove 20 minutes into they're schedule. Oh, I was ill for most of December, particularly at the time that you sent the patch, and by the time I recovered, it was buried in my mailbox. Suggest you have some consideration for others who might not be able to do your beg and call at the immediate moment that you want it, and consider that their email management skills may not be as l33t as yours. OK, sorry about this, it's a bit of a cockup all around. The patch that fixes this problem is still in SCSI pending largely because it's patch description: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation Doesn't lead one to think it might be build critical, so I concentrated on getting the other arm patch out. All the patches I pushed are build critical. The complete Use scsi_eh API for REQUEST_SENSE and the error refactoring patches were in support for the scsi_data_buffer effort. Well that was the last one so all is well I guess. (With out these patches, code is still pushing none-use_sg requests, apart from the members rename of scsi_cmnd) Russell, could you give it a quick test, and I'll put it in with a tested-by tag? Thanks, James Thanks to all Boaz - 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: Current git -- kaboom [bisect] seems IDE related.
On Sun, Feb 10 2008 at 16:43 +0200, Christoph Hellwig [EMAIL PROTECTED] wrote: On Sun, Feb 10, 2008 at 02:38:46PM +0100, Bartlomiej Zolnierkiewicz wrote: The OOPS is most likely (again) my fault - I was rushing out to push out the fix and memset() line didn't get converted. The new patch works fine for me. I prepared the new patch, documented it and started looking into SCSI build breakage... and I no longer feel comfortable with the hack :( It seems that fixing IDE properly will be easier than auditing the whole SCSI for all the weird assumptions on rq-cmd[] size (James?) so I'm back to the code, in the meantime here's the updated patch: Yeah, this is quite nasty. I'll attach the patch below which just rejects a command in scsi_setup_blk_pc_cmnd if it's too large for the scsi_cmnd cmnd array. This is probably enough but I haven't audited all of the scsi code yet. But as James said this is too much of a memory vastage to put it into the tree. Long-term the Panasas folks have looked into killing the scsi_cmnd.cmnd filed entirely and make the struct request.cmd field dynamically sized which would solve your problem, but probably won't be ready for 2.6.25. snip As far as I'm concerned it is very ready, and I have sent a last version for inclusion into 2.6.25. - There is a very minor patch-ability problem between last patchset and scsi-misc I will resend the pachset as a reply to this mail. - Since I never got any comments from Jens or James, this code was never accepted into -mm. So it was not widely tested. Though I have thrown every test I can on these patches. But that is still, a very limited testing. If people have a bit of spare time, please review. For some of us it is very important Thanks Boaz - 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: Investigating potential flaw in scsi error handling
James Bottomley [EMAIL PROTECTED] wrote: On Sun, 2008-02-10 at 13:54 +0100, Elias Oltmanns wrote: [...] Consider this: The -request_fn() of a single queue device is called which in turn calls scsi_dispatch_cmd(). Assume that the device is either in SDEV_BLOCK state or -queuecommand() returns SCSI_MLQUEUE_DEVICE_BUSY for some reason. In either case scsi_queue_insert() will be called. Eventually, blk_run_queue() will be called with the same device queue not plugged yet. This way we directly reenter q-request_fn(). Now, remember that libata sets sdev-max_device_blocked to 1. Consequently, the function scsi_dev_queue_ready() will immediately give a positive response and we go ahead calling scsi_dispatch_cmd() again. Note that at this stage the lld will not have had a chance yet to clear the SDEV_BLOCK state or the condition that caused the SCSI_MLQUEUE_DEVICE_BUSY return code from -queuecommand(). Hence the infinite recursion. A similar recursion can also occur due to a SCSI_MLQUEUE_HOST_BUSY response from -queuecommand(). Unless I have overlooked some unwanted implications, please consider applying the patch that I'm going to send you as a follow up to this email. No. We have a fix for this, it's called setting device_max_blocked to 2 or greater. All your patch does is make this seem to be the case, plus it eliminates the instant reissue case for drivers with queuecommands that do obey all the rules. If you can prove that IDE doesn't obey the rules (no defer returns) In fact, I can prove that scsi midlayer itself doesn't exactly comply with this rule by design. The comment explaining the SDEV_BLOCK state in scsi_device.h suggests that the low level driver is supposed to control whether a device is switched to or from SDEV_BLOCK. However, with max_device_blocked set to 1 we have an infinite loop where the low level driver never gets even called since scsi_dispatch_cmd will requeue the request instantly. IDE doesn't obey the rule either but this can be fixed easily. So, what about SDEV_BLOCK? Regards, Elias - 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: Drivers not ready for sg-chaining
On Thu, 2008-01-17 at 18:51 +0200, Boaz Harrosh wrote: All below drivers are not sg-chain ready do to incomplete software. Once fixed they can move back to SG_ALL. For now they are stuck on SCSI_MAX_SG_SEGMENTS. Affected drivers/files: drivers/scsi/aha152x.c This seems to process an element at a time and should be fixed by sg_next() drivers/scsi/esp_scsi.[ch] As does this. drivers/scsi/imm.c And this. drivers/scsi/in2000.h And this (.c not .h) drivers/scsi/pcmcia/nsp_cs.c This uses a software table, so should be fine. drivers/scsi/ppa.c This does element at a time, so should be fixed by sg_next() drivers/scsi/tmscsim.c as does this. Given where we are in the cycle, it's probably best just to set SG_ALL to 128 so there's no possibility of problems. Individual drivers can make the tradeoff on how they handle larger lists going beyond this. James --- diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index d1299e9..530ff4c 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -6,6 +6,7 @@ #include linux/types.h #include linux/workqueue.h #include linux/mutex.h +#include scsi/scsi.h struct request_queue; struct block_device; @@ -25,12 +26,15 @@ struct blk_queue_tags; * NONE: Self evident. Host adapter is not capable of scatter-gather. * ALL: Means that the host adapter module can do scatter-gather, * and that there is no limit to the size of the table to which - * we scatter/gather data. + * we scatter/gather data. The value we set here is the maximum + * single element sglist. To use chained sglists, the adapter + * has to set a value beyond ALL (and correctly use the chain + * handling API. * Anything else: Indicates the maximum number of chains that can be * used in one scatter-gather request. */ #define SG_NONE 0 -#define SG_ALL 0xff +#define SG_ALL SCSI_MAX_SG_SEGMENTS #define MODE_UNKNOWN 0x00 #define MODE_INITIATOR 0x01 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: Investigating potential flaw in scsi error handling
On Sun, 2008-02-10 at 16:29 +0100, Elias Oltmanns wrote: James Bottomley [EMAIL PROTECTED] wrote: On Sun, 2008-02-10 at 13:54 +0100, Elias Oltmanns wrote: [...] Consider this: The -request_fn() of a single queue device is called which in turn calls scsi_dispatch_cmd(). Assume that the device is either in SDEV_BLOCK state or -queuecommand() returns SCSI_MLQUEUE_DEVICE_BUSY for some reason. In either case scsi_queue_insert() will be called. Eventually, blk_run_queue() will be called with the same device queue not plugged yet. This way we directly reenter q-request_fn(). Now, remember that libata sets sdev-max_device_blocked to 1. Consequently, the function scsi_dev_queue_ready() will immediately give a positive response and we go ahead calling scsi_dispatch_cmd() again. Note that at this stage the lld will not have had a chance yet to clear the SDEV_BLOCK state or the condition that caused the SCSI_MLQUEUE_DEVICE_BUSY return code from -queuecommand(). Hence the infinite recursion. A similar recursion can also occur due to a SCSI_MLQUEUE_HOST_BUSY response from -queuecommand(). Unless I have overlooked some unwanted implications, please consider applying the patch that I'm going to send you as a follow up to this email. No. We have a fix for this, it's called setting device_max_blocked to 2 or greater. All your patch does is make this seem to be the case, plus it eliminates the instant reissue case for drivers with queuecommands that do obey all the rules. If you can prove that IDE doesn't obey the rules (no defer returns) In fact, I can prove that scsi midlayer itself doesn't exactly comply with this rule by design. The comment explaining the SDEV_BLOCK state in scsi_device.h suggests that the low level driver is supposed to control whether a device is switched to or from SDEV_BLOCK. However, with max_device_blocked set to 1 we have an infinite loop where the low level driver never gets even called since scsi_dispatch_cmd will requeue the request instantly. IDE doesn't obey the rule either but this can be fixed easily. So, what about SDEV_BLOCK? Well, nothing. It's an API a HBA may not use (per previous rule) if it wants to set max_device_blocked to 1. 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: Investigating potential flaw in scsi error handling
James Bottomley [EMAIL PROTECTED] wrote: On Sun, 2008-02-10 at 16:29 +0100, Elias Oltmanns wrote: James Bottomley [EMAIL PROTECTED] wrote: [...] No. We have a fix for this, it's called setting device_max_blocked to 2 or greater. All your patch does is make this seem to be the case, plus it eliminates the instant reissue case for drivers with queuecommands that do obey all the rules. If you can prove that IDE doesn't obey the rules (no defer returns) In fact, I can prove that scsi midlayer itself doesn't exactly comply with this rule by design. The comment explaining the SDEV_BLOCK state in scsi_device.h suggests that the low level driver is supposed to control whether a device is switched to or from SDEV_BLOCK. However, with max_device_blocked set to 1 we have an infinite loop where the low level driver never gets even called since scsi_dispatch_cmd will requeue the request instantly. IDE doesn't obey the rule either but this can be fixed easily. So, what about SDEV_BLOCK? Well, nothing. It's an API a HBA may not use (per previous rule) if it wants to set max_device_blocked to 1. Right. Thanks for the clarification. Regards, Elias - 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: Drivers not ready for sg-chaining
On Sun, 2008-02-10 at 18:08 +0200, Boaz Harrosh wrote: My patches *do not* attempt to fix the sg_chaining support. They only make all the drivers that use SG_ALL to use SCSI_MAX_SG_SEGMENTS. One by One, and not globally as your suggestion. Yes, I know ... but it does need fixing for the listed drivers. This is for two reasons. 1. So drivers can be individually fixed and in the patch that fixes them they can go back to SG_ALL. No, it's so SG_ALL can mean use chaining ... I'm not sure that's desirable for the default value. Particularly for devices that key internal sglist arrays off SG_ALL 2. Those drivers that have been using SG_ALL correctly and were converted to support sg-chaining are not penalized because of bad/old drivers I don't see they're penalised this way either ... they just have to set a higher value in their host template. 3. Some drivers in this patchset are converted to use a real internal driver limit. That does not necessarily match SCSI_MAX_SG_SEGMENTS. In the event that SCSI_MAX_SG_SEGMENTS wants to change. Yes, I looked at those they're all either safe or currently (eventually) do the right thing. The bulk of the patchset is very much mechanical and is not dangerous and was ACKed by the more important maintainers. (That is where the changes are more then trivial). So I don't see why they cannot get a proper review and be accepted. Instead of doing the safe but the wrong thing, cross tree. What's wrong about 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
Re: scsi: Drivers not ready for sg-chaining
On Sun, Feb 10 2008 at 17:42 +0200, James Bottomley [EMAIL PROTECTED] wrote: On Thu, 2008-01-17 at 18:51 +0200, Boaz Harrosh wrote: All below drivers are not sg-chain ready do to incomplete software. Once fixed they can move back to SG_ALL. For now they are stuck on SCSI_MAX_SG_SEGMENTS. Affected drivers/files: drivers/scsi/aha152x.c This seems to process an element at a time and should be fixed by sg_next() drivers/scsi/esp_scsi.[ch] As does this. drivers/scsi/imm.c And this. drivers/scsi/in2000.h And this (.c not .h) drivers/scsi/pcmcia/nsp_cs.c This uses a software table, so should be fine. drivers/scsi/ppa.c This does element at a time, so should be fixed by sg_next() drivers/scsi/tmscsim.c as does this. Given where we are in the cycle, it's probably best just to set SG_ALL to 128 so there's no possibility of problems. Individual drivers can make the tradeoff on how they handle larger lists going beyond this. James --- diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index d1299e9..530ff4c 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -6,6 +6,7 @@ #include linux/types.h #include linux/workqueue.h #include linux/mutex.h +#include scsi/scsi.h struct request_queue; struct block_device; @@ -25,12 +26,15 @@ struct blk_queue_tags; * NONE: Self evident. Host adapter is not capable of scatter-gather. * ALL: Means that the host adapter module can do scatter-gather, *and that there is no limit to the size of the table to which - *we scatter/gather data. + *we scatter/gather data. The value we set here is the maximum + *single element sglist. To use chained sglists, the adapter + *has to set a value beyond ALL (and correctly use the chain + *handling API. * Anything else: Indicates the maximum number of chains that can be *used in one scatter-gather request. */ #define SG_NONE 0 -#define SG_ALL 0xff +#define SG_ALL SCSI_MAX_SG_SEGMENTS #define MODE_UNKNOWN 0x00 #define MODE_INITIATOR 0x01 James My patches *do not* attempt to fix the sg_chaining support. They only make all the drivers that use SG_ALL to use SCSI_MAX_SG_SEGMENTS. One by One, and not globally as your suggestion. This is for two reasons. 1. So drivers can be individually fixed and in the patch that fixes them they can go back to SG_ALL. 2. Those drivers that have been using SG_ALL correctly and were converted to support sg-chaining are not penalized because of bad/old drivers 3. Some drivers in this patchset are converted to use a real internal driver limit. That does not necessarily match SCSI_MAX_SG_SEGMENTS. In the event that SCSI_MAX_SG_SEGMENTS wants to change. The bulk of the patchset is very much mechanical and is not dangerous and was ACKed by the more important maintainers. (That is where the changes are more then trivial). So I don't see why they cannot get a proper review and be accepted. Instead of doing the safe but the wrong thing, cross tree. Boaz - 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: Drivers not ready for sg-chaining
On Sun, Feb 10 2008 at 18:16 +0200, James Bottomley [EMAIL PROTECTED] wrote: On Sun, 2008-02-10 at 18:08 +0200, Boaz Harrosh wrote: My patches *do not* attempt to fix the sg_chaining support. They only make all the drivers that use SG_ALL to use SCSI_MAX_SG_SEGMENTS. One by One, and not globally as your suggestion. Yes, I know ... but it does need fixing for the listed drivers. That is another patchset that its time as not yet come. Once every thing settles I will send these too. That's why I put the list as a reminder for me to check upon later. This is for two reasons. 1. So drivers can be individually fixed and in the patch that fixes them they can go back to SG_ALL. No, it's so SG_ALL can mean use chaining ... I'm not sure that's desirable for the default value. Particularly for devices that key internal sglist arrays off SG_ALL I have fixed *all* these drivers that are based off SG_ALL for table sizes, and all other reasons. exactly so the SG_ALL will continue to mean what it means in English. (And this was triggered by your request) 2. Those drivers that have been using SG_ALL correctly and were converted to support sg-chaining are not penalized because of bad/old drivers I don't see they're penalised this way either ... they just have to set a higher value in their host template. It was you who wanted that to be SG_ALL. I wanted just an hard coded = ~0. 3. Some drivers in this patchset are converted to use a real internal driver limit. That does not necessarily match SCSI_MAX_SG_SEGMENTS. In the event that SCSI_MAX_SG_SEGMENTS wants to change. Yes, I looked at those they're all either safe or currently (eventually) do the right thing. What ?? The bulk of the patchset is very much mechanical and is not dangerous and was ACKed by the more important maintainers. (That is where the changes are more then trivial). So I don't see why they cannot get a proper review and be accepted. Instead of doing the safe but the wrong thing, cross tree. What's wrong about this? I don't want to repeat myself. If it's fine with you, I trust your final judgment. You are welcome to submit a patch that fixes all the good drivers that are regressed by your suggestion. James Boaz - 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: Drivers not ready for sg-chaining
On Sun, 2008-02-10 at 18:36 +0200, Boaz Harrosh wrote: 2. Those drivers that have been using SG_ALL correctly and were converted to support sg-chaining are not penalized because of bad/old drivers I don't see they're penalised this way either ... they just have to set a higher value in their host template. It was you who wanted that to be SG_ALL. I wanted just an hard coded = ~0. Yes, I've changed my mind. I think the best value for SG_ALL is the max single table and we'll do a dynamic increase for drivers that really have considered all the chaining implications. That doesn't mean we don't make the others chain ready ... we do ... we just don't force it on them. I don't want to repeat myself. If it's fine with you, I trust your final judgment. You are welcome to submit a patch that fixes all the good drivers that are regressed by your suggestion. Lowering the figure from current 255 to 128 isn't really going to cause any regressions. It will actually save a few unnecessary allocations for some drivers. 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: Kernel Panic in MPT SAS on 2.6.24 (and 2.6.23.14, 2.6.23.9) (fwd)
Hello,
Eric Moore is on vacation, adding some CCs and TOs.
-- Forwarded message --
Date: Sun, 10 Feb 2008 18:25:39 +0100 (CET)
From: Krzysztof Oledzki [EMAIL PROTECTED]
To: Maximilian Wilhelm [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED], [EMAIL PROTECTED]
Subject: Re: Kernel Panic in MPT SAS on 2.6.24 (and 2.6.23.14, 2.6.23.9)
On Sun, 10 Feb 2008, Maximilian Wilhelm wrote:
Am Friday, den 8 February hub Maximilian Wilhelm folgendes in die Tasten:
Just noticed that Eric's address was wrong, so resend with corrected Cc.
Eric, my intial report was http://lkml.org/lkml/2008/2/6/300
Am Thursday, den 7 February hub Krzysztof Oledzki folgendes in die Tasten:
Hi!
While installing my new firewall I got the following kernel panic in
the MPT SAS driver which I need for the disks.
The first kernel I bootet was 2.6.23.14 which did panic so I tried a
2.6.24 which panics, too. Our usual FAI kernel (2.6.23.9) is also
affected.
Could you please try 2.6.22-stable?
Yes it works :-/
I've put some things which on the web which might be helpful:
dmesg http://files.rfc2324.org/mptsas_panic/2.6.22-dmesg
lspci -vhttp://files.rfc2324.org/mptsas_panic/2.6.22-lspci-v
.config http://files.rfc2324.org/mptsas_panic/2.6.22-config
I'll search for the last working kernel and try to break it down to a
commit tommorow when I can get a serial console or direct access.
The Java driven console redirection is everything else than fulfilling :-(
It looks *very* similar to my problem:
http://bugzilla.kernel.org/show_bug.cgi?id=9909
It seems to be the same controller:
01:00.0 SCSI storage controller: LSI Logic / Symbios Logic SAS1068E
PCI-Express Fusion-MPT SAS (rev 08)
Subsystem: Dell Unknown device 1f10
Flags: bus master, fast devsel, latency 0, IRQ 16
I/O ports at ec00 [size=256]
Memory at fc8fc000 (64-bit, non-prefetchable) [size=16K]
Memory at fc8e (64-bit, non-prefetchable) [size=64K]
Expansion ROM at fc90 [disabled] [size=1M]
Capabilities: [50] Power Management version 2
Capabilities: [68] Express Endpoint IRQ 0
Capabilities: [98] Message Signalled Interrupts: Mask- 64bit+ Queue=0/0
Enable-
Capabilities: [b0] MSI-X: Enable- Mask- TabSize=1
I did a git bisect between v2.6.22 v2.6.23 and it seems that
6cb8f91320d3e720351c21741da795fed580b21b
introduced some badness.
Thanks! This was *really* useful!
Now, how about attached patch? Should work with both 2.6.23 and 2.6.24.
Best regards,
Krzysztof Olędzki[SCSI] mpt fusion: Don't oops if NumPhys==0
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]
diff -Nur a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
--- a/drivers/message/fusion/mptsas.c 2007-10-09 22:31:38.0 +0200
+++ b/drivers/message/fusion/mptsas.c 2008-02-10 17:38:51.0 +0100
@@ -1772,6 +1772,11 @@
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,
Re: [PATCH 11/9 update] firewire: fw-sbp2: enforce a retry of __scsi_add_device if bus generation changed
On Friday 08 February 2008 04:33:29 pm Stefan Richter wrote: fw-sbp2 is unable to reconnect while performing __scsi_add_device because there is only a single workqueue thread context available for both at the moment. This should be fixed eventually. An actual failure of __scsi_add_device is easy to handle, but an incomplete execution of __scsi_add_device with an sdev returned would remain undetected and leave the SBP-2 target unusable. Therefore we use a workaround: If there was a bus reset during __scsi_add_device (i.e. during the SCSI probe), we remove the new sdev immediately, log out, and attempt login and SCSI probe again. Signed-off-by: Stefan Richter [EMAIL PROTECTED] Now *this* does the trick. I get the 'READ CAPACITY failed' as before, then 'firewire_sbp2: fw1.0: error status: 0:4', followed by a new login and SCSI probe, both of which are successful this time, disk is available for use and all that good stuff. Signed-off-by: Jarod Wilson [EMAIL PROTECTED] -- Jarod Wilson [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] scsi: varlen extended and vendor-specific cdbs
Add support for variable-length, extended, and vendor specific
CDBs to scsi-ml. It is now possible for initiators and ULD's
to issue these types of commands. LLDs need not change much.
All they need is to raise the .max_cmd_len to the longest command
they support (see iscsi patches).
- clean-up some code paths that did not expect commands to be
larger than 16, and change cmd_len members' type to short as
char is not enough.
- Add support for varlen_cdb in scsi_execute.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
Signed-off-by: Benny Halevy [EMAIL PROTECTED]
---
block/scsi_ioctl.c |4 ++--
drivers/scsi/constants.c | 10 +++---
drivers/scsi/scsi.c | 22 +++---
drivers/scsi/scsi_lib.c | 27 ++-
include/scsi/scsi.h | 40 +---
include/scsi/scsi_cmnd.h |2 +-
include/scsi/scsi_host.h |8 +++-
7 files changed, 75 insertions(+), 38 deletions(-)
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 9675b34..a1d7070 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -33,13 +33,13 @@
#include scsi/scsi_cmnd.h
/* Command group 3 is reserved and should never be used. */
-const unsigned char scsi_command_size[8] =
+const unsigned char scsi_command_size_tbl[8] =
{
6, 10, 10, 12,
16, 12, 10, 10
};
-EXPORT_SYMBOL(scsi_command_size);
+EXPORT_SYMBOL(scsi_command_size_tbl);
#include scsi/sg.h
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 403a7f2..9785d73 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -28,7 +28,6 @@
#define SERVICE_ACTION_OUT_12 0xa9
#define SERVICE_ACTION_IN_16 0x9e
#define SERVICE_ACTION_OUT_16 0x9f
-#define VARIABLE_LENGTH_CMD 0x7f
@@ -210,7 +209,7 @@ static void print_opcode_name(unsigned char * cdbp, int
cdb_len)
cdb0 = cdbp[0];
switch(cdb0) {
case VARIABLE_LENGTH_CMD:
- len = cdbp[7] + 8;
+ len = scsi_varlen_cdb_length(cdbp);
if (len 10) {
printk(short variable length command,
len=%d ext_len=%d, len, cdb_len);
@@ -300,7 +299,7 @@ static void print_opcode_name(unsigned char * cdbp, int
cdb_len)
cdb0 = cdbp[0];
switch(cdb0) {
case VARIABLE_LENGTH_CMD:
- len = cdbp[7] + 8;
+ len = scsi_varlen_cdb_length(cdbp);
if (len 10) {
printk(short opcode=0x%x command, len=%d
ext_len=%d, cdb0, len, cdb_len);
@@ -335,10 +334,7 @@ void __scsi_print_command(unsigned char *cdb)
int k, len;
print_opcode_name(cdb, 0);
- if (VARIABLE_LENGTH_CMD == cdb[0])
- len = cdb[7] + 8;
- else
- len = COMMAND_SIZE(cdb[0]);
+ len = scsi_command_size(cdb);
/* print out all bytes in cdb */
for (k = 0; k len; ++k)
printk( %02x, cdb[k]);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index fecba05..944fafa 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -79,15 +79,6 @@ static void scsi_done(struct scsi_cmnd *cmd);
#define MIN_RESET_PERIOD (15*HZ)
/*
- * Macro to determine the size of SCSI command. This macro takes vendor
- * unique commands into account. SCSI commands in groups 6 and 7 are
- * vendor unique and we will depend upon the command length being
- * supplied correctly in cmd_len.
- */
-#define CDB_SIZE(cmd) (cmd)-cmnd[0] 5) 7) 6) ? \
- COMMAND_SIZE((cmd)-cmnd[0]) : (cmd)-cmd_len)
-
-/*
* Note - the initial logging level can be set here to log events at boot time.
* After the system is up, you may enable logging via the /proc interface.
*/
@@ -525,6 +516,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
unsigned long flags = 0;
unsigned long timeout;
int rtn = 0;
+ unsigned cmd_len;
/* check if the device is still usable */
if (unlikely(cmd-device-sdev_state == SDEV_DEL)) {
@@ -606,9 +598,17 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
* Before we queue this command, check if the command
* length exceeds what the host adapter can handle.
*/
- if (CDB_SIZE(cmd) cmd-device-host-max_cmd_len) {
+ cmd_len = cmd-cmd_len;
+ if (!cmd_len) {
+ BUG_ON(cmd-cmnd[0] == VARIABLE_LENGTH_CMD);
+ cmd_len = COMMAND_SIZE((cmd)-cmnd[0]);
+ }
+
+ if (cmd_len cmd-device-host-max_cmd_len) {
SCSI_LOG_MLQUEUE(3,
- printk(queuecommand : command too long.\n));
+ printk(queuecommand : command too long.
+ cdb_size=%d host-max_cmd_len=%d\n,
+ cmd-cmd_len, cmd-device-host-max_cmd_len));
cmd-result = (DID_ABORT 16);
[PATCH 2/3] block layer varlen-cdb
- add varlen_cdb and varlen_cdb_len to hold a large user cdb
if needed. They start as empty. Allocation of buffer must
be done by user and held until request execution is done.
- Since there can be either a fix_length command up to 16 bytes
or a variable_length, larger then 16 bytes, commands but never
both, we hold the two types in a union to save space. The
presence of varlen_cdb_len and cmd_len==0 signals a varlen_cdb
mode.
- Please use added rq_{set,get}_varlen_cdb() to set every thing
up in a way that will not confuse drivers that do not support
varlen_cdb's.
- Note that this patch does not add any size to struct request
since the unsigned cmd_len is split here to 2 ushorts, which
is more then enough.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
---
block/blk-core.c |2 ++
include/linux/blkdev.h | 27 +--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 4afb39c..1c5cfa7 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -124,6 +124,8 @@ void rq_init(struct request_queue *q, struct request *rq)
rq-end_io_data = NULL;
rq-completion_data = NULL;
rq-next_rq = NULL;
+ rq-varlen_cdb_len = 0;
+ rq-varlen_cdb = NULL;
}
static void req_bio_endio(struct request *rq, struct bio *bio,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 90392a9..a8a6c20 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -211,8 +211,15 @@ struct request {
/*
* when request is used as a packet command carrier
*/
- unsigned int cmd_len;
- unsigned char cmd[BLK_MAX_CDB];
+ unsigned short cmd_len;
+ unsigned short varlen_cdb_len; /* length of varlen_cdb buffer */
+ union {
+ unsigned char cmd[BLK_MAX_CDB];
+ unsigned char *varlen_cdb;/* an optional variable-length cdb.
+ * points to a user buffer that must
+ * be valid until end of request
+ */
+ };
unsigned int data_len;
unsigned int sense_len;
@@ -478,6 +485,22 @@ enum {
#define rq_is_sync(rq) (rq_data_dir((rq)) == READ || (rq)-cmd_flags
REQ_RW_SYNC)
#define rq_is_meta(rq) ((rq)-cmd_flags REQ_RW_META)
+static inline void rq_set_varlen_cdb(struct request *rq, u8 *cdb, short
cdb_len)
+{
+ rq-cmd_len = 0; /* Make sure legacy drivers don't get confused */
+ rq-varlen_cdb_len = cdb_len;
+ rq-varlen_cdb = cdb;
+}
+
+/* If ! a varlen_cdb than return NULL */
+static inline u8 *rq_get_varlen_cdb(struct request *rq)
+{
+ if (!rq-cmd_len rq-varlen_cdb_len)
+ return rq-varlen_cdb;
+ else
+ return NULL;
+}
+
static inline int blk_queue_full(struct request_queue *q, int rw)
{
if (rw == READ)
--
1.5.3.3
-
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
[PATCHSET 0/3] varlen extended and vendor-specific cdbs
Submitted is a patchset for adding support for variable-length, extended, and vendor specific CDBs. It should now cover the entire range of the SCSI standard. (and/or any other use of command packets in block devices) They are based on scsi-misc. Difference from last time, is at struct request. I did a smallish hack to save the extra pointer space. The pointer now shares it's space with the request-cmd, as they are mutual exclusive. The flag to switch between them is when cmd_len == 0 and varlen_cdb_len != 0 I've added 2 accessors to hide the mess. I think this approach should be safe. I can easily go back to the separate pointer, but I figured a little hack is better then a bloat. This is on top of the size shrink to struct scsi_cmnd gained in the first patch. We save upto 12 bytes on 32 bit ARCHs So over all, this cleans things up, and add fixtures without any extra cost. [1/3] Let scsi_cmnd-cmnd use request-cmd buffer Here I let scsi_cmnd-cmnd point to the space allocated by request-cmd, instead of copying the data. The scsi_cmnd-cmd_len is guaranteed to contain the right length of the command. I have tried to go over every single place in the kernel that uses scsi_cmnd-cmnd and make sure it looks sane. Surprisingly to me, that was not at all bad. I hope I did not miss anything. I've tested on an x86_64 machine booting from a sata disk and ran the iscsi regression tests as well as my bidi and varlen tests on top of the complete patchset and all tests passed. [2/3] block layer varlen-cd Here I added an option to use a user supplied buffer for an arbitrary large command. Buffer must be kept valid until execution of request ends. The pointer to the buffer shares it's space with the fixed length command, so the size of struct request does not change. [3/3] scsi: varlen extended and vendor-specific cdbs Adds support for variable length, extended, and vendor-specific cdbs at the scsi mid-layer. Bart: If you need this infrastructure see second patch for an easy to use inline accessors on struct request. If you then need to use them in a scsi LLD, thats easy same as before only cmd_len will be bigger then 16. If you need to use them in a block LLD. You need to use the accessors and an extra if() statement. See 3rd patch on how scsi_lib.c uses it. Boaz - 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
Subject: [PATCH 1/3] Let scsi_cmnd-cmnd use request-cmd buffer
- struct scsi_cmnd had a 16 bytes command buffer of its own.
This is an unnecessary duplication and copy of request's
cmd. It is probably left overs from the time that scsi_cmnd
could function without a request attached. So clean that up.
- Once above is done, few places, apart from scsi-ml, needed
adjustments due to changing the data type of scsi_cmnd-cmnd.
- Lots of drivers still use MAX_COMMAND_SIZE. So I have left
that #define but equate it to BLK_MAX_CDB. The way I see it
and is reflected in the patch below is.
MAX_COMMAND_SIZE - means: The longest fixed-length (*) SCSI CDB
as per the SCSI standard and is not related
to the implementation.
BLK_MAX_CDB. - The allocated space at the request level
(*)fixed-length here means commands that their size can be determined
by their opcode and the CDB does not carry a length specifier, like
the VARIABLE_LENGTH_CMD(0x7f) command. This is actually not exactly
true and the SCSI standard also defines extended commands and
vendor specific commands that can be bigger than 16 bytes. The kernel
will support these using the same infrastructure used for VARLEN CDB's.
So in effect MAX_COMMAND_SIZE means the maximum size command
scsi-ml supports without specifying a cmd_len by ULD's
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
---
drivers/firewire/fw-sbp2.c |2 +-
drivers/s390/scsi/zfcp_dbf.c |2 +-
drivers/s390/scsi/zfcp_fsf.c |2 +-
drivers/scsi/53c700.c|6 +++---
drivers/scsi/a100u2w.c |2 +-
drivers/scsi/hptiop.c|6 +++---
drivers/scsi/ibmvscsi/ibmvscsi.c |2 +-
drivers/scsi/initio.c|2 +-
drivers/scsi/qla1280.c |4 ++--
drivers/scsi/scsi_error.c| 14 --
drivers/scsi/scsi_lib.c |5 +++--
drivers/scsi/scsi_tgt_lib.c |2 ++
drivers/usb/storage/isd200.c |2 ++
include/scsi/scsi_cmnd.h |9 +++--
include/scsi/scsi_eh.h |4 ++--
15 files changed, 38 insertions(+), 26 deletions(-)
diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c
index 19ece9b..1d9602b 100644
--- a/drivers/firewire/fw-sbp2.c
+++ b/drivers/firewire/fw-sbp2.c
@@ -1254,7 +1254,7 @@ static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd,
scsi_done_fn_t done)
memset(orb-request.command_block,
0, sizeof(orb-request.command_block));
- memcpy(orb-request.command_block, cmd-cmnd, COMMAND_SIZE(*cmd-cmnd));
+ memcpy(orb-request.command_block, cmd-cmnd, cmd-cmd_len);
orb-base.callback = complete_command_orb;
orb-base.request_bus =
diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 701046c..7a7f619 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -724,7 +724,7 @@ _zfcp_scsi_dbf_event_common(const char *tag, const char
*tag2, int level,
rec-scsi_result = scsi_cmnd-result;
rec-scsi_cmnd = (unsigned long)scsi_cmnd;
rec-scsi_serial = scsi_cmnd-serial_number;
- memcpy(rec-scsi_opcode, scsi_cmnd-cmnd,
+ memcpy(rec-scsi_opcode, scsi_cmnd-cmnd,
min((int)scsi_cmnd-cmd_len,
ZFCP_DBF_SCSI_OPCODE));
rec-scsi_retries = scsi_cmnd-retries;
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 0dff058..1abbac5 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -4220,7 +4220,7 @@ zfcp_fsf_send_fcp_command_task_handler(struct
zfcp_fsf_req *fsf_req)
ZFCP_LOG_TRACE(scpnt-result =0x%x, command was:\n,
scpnt-result);
ZFCP_HEX_DUMP(ZFCP_LOG_LEVEL_TRACE,
- (void *) scpnt-cmnd, scpnt-cmd_len);
+ scpnt-cmnd, scpnt-cmd_len);
ZFCP_LOG_TRACE(%i bytes sense data provided by FCP\n,
fcp_rsp_iu-fcp_sns_len);
diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index f4c4fe9..f5a9add 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -599,7 +599,7 @@ NCR_700_scsi_done(struct NCR_700_Host_Parameters *hostdata,
(struct NCR_700_command_slot *)SCp-host_scribble;
dma_unmap_single(hostdata-dev, slot-pCmd,
-sizeof(SCp-cmnd), DMA_TO_DEVICE);
+MAX_COMMAND_SIZE, DMA_TO_DEVICE);
if (slot-flags == NCR_700_FLAG_AUTOSENSE) {
char *cmnd = NCR_700_get_sense_cmnd(SCp-device);
#ifdef NCR_700_DEBUG
@@ -1004,7 +1004,7 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct
scsi_cmnd *SCp,
Re: (fwd) Bug#11922: I/O error on blank tapes
On Tue, 5 Feb 2008, Kai Makisara wrote: On Mon, 4 Feb 2008, James Bottomley wrote: On Mon, 2008-02-04 at 22:28 +0100, Borislav Petkov wrote: On Mon, Feb 04, 2008 at 03:22:06PM +0100, maximilian attems wrote: (Added Bart to CC) hello borislav, ... This does still occur with 2.6.22; with a blank tape in my HP DDS-4 drive: $ tar tzvf /dev/nst0 tar: /dev/nst0: Cannot read: Input/output error That's a SCSI tape, not an IDE one. I cc'd the SCSI list This is not a bug, it is a feature. There is _nothing_ on the tape and if you try to read something, you get an error. The same thing applies to reading after the last filemark. Note that after writing a filemark at the beginning of the tape, the situation is different. Now there is a file and the normal EOF semantics apply although there still is no data. I admit that the error return could be more descriptive but the st driver tries to be compatible with other Unices. The behavior can be changed if Linux does not match other Unices. I don't remember if I have tested just this with other Unices. I will try to test this with Tru64 tomorrow. If anyone has data on other Unices, it would be helpful. None of our Tru64 boxes have a user-accessible tape drive any more. However, I have been able to test with a Solaris box. The behavior there matches the Linux behavior: blank tape - i/o error when trying to read. -- Kai - 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/arm/fas216.c compile error
On Sun, Feb 10, 2008 at 08:20:24AM -0600, James Bottomley wrote:
On Sun, 2008-02-10 at 13:58 +, Russell King wrote:
On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
[SCSI] arm: convert to accessors and !use_sg cleanup
Thanks for checking. This patch was in scsi-pending tree since forever,
And we were unable
to get a responsive maintainer to ACK on them. until the breakage cause
went into mainline
we finally managed a Tested-by:.
I guess sometimes people are so busy, you need a bulldozer to shove 20
minutes into they're
schedule.
Oh, I was ill for most of December, particularly at the time that you
sent the patch, and by the time I recovered, it was buried in my mailbox.
Suggest you have some consideration for others who might not be able to
do your beg and call at the immediate moment that you want it, and
consider that their email management skills may not be as l33t as yours.
OK, sorry about this, it's a bit of a cockup all around. The patch that
fixes this problem is still in SCSI pending largely because it's patch
description:
[SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
Doesn't lead one to think it might be build critical, so I concentrated
on getting the other arm patch out.
Russell, could you give it a quick test, and I'll put it in with a
tested-by tag?
It's not looking good:
CC drivers/scsi/arm/fas216.o
drivers/scsi/arm/fas216.c: In function `fas216_rq_sns_done':
drivers/scsi/arm/fas216.c:2021: warning: passing arg 2 of
`scsi_eh_restore_cmnd' from incompatible pointer type
drivers/scsi/arm/fas216.c: In function `fas216_std_done':
drivers/scsi/arm/fas216.c:2107: warning: passing arg 2 of `scsi_eh_prep_cmnd'
from incompatible pointer type
Since the second argument of scsi_eh_prep_cmnd is 'struct scsi_eh_save *ses'
this patch is most definitely bad. Not even booted it.
Thanks,
James
---
From: Boaz Harrosh [EMAIL PROTECTED]
Date: Mon, 10 Sep 2007 22:39:11 +0300
Subject: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
- Use new scsi_eh_prep/restor_cmnd() for synchronous
REQUEST_SENSE invocation.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
Cc: Russell King [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/arm/fas216.c | 16 +++-
drivers/scsi/arm/fas216.h |3 +++
2 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index fb5f202..a715632 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info,
struct scsi_cmnd *SCpnt,
* the upper layers to process. This would have been set
* correctly by fas216_std_done.
*/
+ scsi_eh_restore_cmnd(SCpnt, info-ses);
SCpnt-scsi_done(SCpnt);
}
@@ -2103,23 +2104,12 @@ request_sense:
if (SCpnt-cmnd[0] == REQUEST_SENSE)
goto done;
+ scsi_eh_prep_cmnd(SCpnt, info-ses, NULL, 0, ~0);
fas216_log_target(info, LOG_CONNECT, SCpnt-device-id,
requesting sense);
- memset(SCpnt-cmnd, 0, sizeof (SCpnt-cmnd));
- SCpnt-cmnd[0] = REQUEST_SENSE;
- SCpnt-cmnd[1] = SCpnt-device-lun 5;
- SCpnt-cmnd[4] = sizeof(SCpnt-sense_buffer);
- SCpnt-cmd_len = COMMAND_SIZE(SCpnt-cmnd[0]);
- SCpnt-SCp.buffer = NULL;
- SCpnt-SCp.buffers_residual = 0;
- SCpnt-SCp.ptr = (char *)SCpnt-sense_buffer;
- SCpnt-SCp.this_residual = sizeof(SCpnt-sense_buffer);
- SCpnt-SCp.phase = sizeof(SCpnt-sense_buffer);
+ init_SCp(SCpnt);
SCpnt-SCp.Message = 0;
SCpnt-SCp.Status = 0;
- SCpnt-request_bufflen = sizeof(SCpnt-sense_buffer);
- SCpnt-sc_data_direction = DMA_FROM_DEVICE;
- SCpnt-use_sg = 0;
SCpnt-tag = 0;
SCpnt-host_scribble = (void *)fas216_rq_sns_done;
diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h
index 00e5f05..3e73e26 100644
--- a/drivers/scsi/arm/fas216.h
+++ b/drivers/scsi/arm/fas216.h
@@ -16,6 +16,8 @@
#define NO_IRQ 255
#endif
+#include scsi/scsi_eh.h
+
#include queue.h
#include msgqueue.h
@@ -311,6 +313,7 @@ typedef struct {
/* miscellaneous */
int internal_done; /* flag to indicate
request done */
+ struct scsi_eh_save *ses; /* holds request sense restore
info */
Looks to me like this line has a stray '*' on?
unsigned long magic_end;
} FAS216_Info;
--
Russell King
Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
-
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/arm/fas216.c compile error
On Sun, 2008-02-10 at 22:02 +, Russell King wrote:
On Sun, Feb 10, 2008 at 08:20:24AM -0600, James Bottomley wrote:
On Sun, 2008-02-10 at 13:58 +, Russell King wrote:
On Sun, Feb 10, 2008 at 03:07:09PM +0200, Boaz Harrosh wrote:
It's in mainline 84ac86ca8c6787f9efff28bc04b1b65fe0a5c310
[SCSI] arm: convert to accessors and !use_sg cleanup
Thanks for checking. This patch was in scsi-pending tree since forever,
And we were unable
to get a responsive maintainer to ACK on them. until the breakage cause
went into mainline
we finally managed a Tested-by:.
I guess sometimes people are so busy, you need a bulldozer to shove 20
minutes into they're
schedule.
Oh, I was ill for most of December, particularly at the time that you
sent the patch, and by the time I recovered, it was buried in my mailbox.
Suggest you have some consideration for others who might not be able to
do your beg and call at the immediate moment that you want it, and
consider that their email management skills may not be as l33t as yours.
OK, sorry about this, it's a bit of a cockup all around. The patch that
fixes this problem is still in SCSI pending largely because it's patch
description:
[SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
Doesn't lead one to think it might be build critical, so I concentrated
on getting the other arm patch out.
Russell, could you give it a quick test, and I'll put it in with a
tested-by tag?
It's not looking good:
CC drivers/scsi/arm/fas216.o
drivers/scsi/arm/fas216.c: In function `fas216_rq_sns_done':
drivers/scsi/arm/fas216.c:2021: warning: passing arg 2 of
`scsi_eh_restore_cmnd' from incompatible pointer type
drivers/scsi/arm/fas216.c: In function `fas216_std_done':
drivers/scsi/arm/fas216.c:2107: warning: passing arg 2 of `scsi_eh_prep_cmnd'
from incompatible pointer type
Since the second argument of scsi_eh_prep_cmnd is 'struct scsi_eh_save *ses'
this patch is most definitely bad. Not even booted it.
Yes, there looks to be a fatal screw up in the definition in
FAS216_Info. Could you try this ... I think I've corrected it.
James
---
From 35be7297dc581b42c05ea7751ee595b0ce78f669 Mon Sep 17 00:00:00 2001
From: Boaz Harrosh [EMAIL PROTECTED]
Date: Mon, 10 Sep 2007 22:39:11 +0300
Subject: [SCSI] fas216: Use scsi_eh API for REQUEST_SENSE invocation
- Use new scsi_eh_prep/restor_cmnd() for synchronous
REQUEST_SENSE invocation.
Signed-off-by: Boaz Harrosh [EMAIL PROTECTED]
Cc: Russell King [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/arm/fas216.c | 16 +++-
drivers/scsi/arm/fas216.h |3 +++
2 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index fb5f202..a715632 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct
scsi_cmnd *SCpnt,
* the upper layers to process. This would have been set
* correctly by fas216_std_done.
*/
+ scsi_eh_restore_cmnd(SCpnt, info-ses);
SCpnt-scsi_done(SCpnt);
}
@@ -2103,23 +2104,12 @@ request_sense:
if (SCpnt-cmnd[0] == REQUEST_SENSE)
goto done;
+ scsi_eh_prep_cmnd(SCpnt, info-ses, NULL, 0, ~0);
fas216_log_target(info, LOG_CONNECT, SCpnt-device-id,
requesting sense);
- memset(SCpnt-cmnd, 0, sizeof (SCpnt-cmnd));
- SCpnt-cmnd[0] = REQUEST_SENSE;
- SCpnt-cmnd[1] = SCpnt-device-lun 5;
- SCpnt-cmnd[4] = sizeof(SCpnt-sense_buffer);
- SCpnt-cmd_len = COMMAND_SIZE(SCpnt-cmnd[0]);
- SCpnt-SCp.buffer = NULL;
- SCpnt-SCp.buffers_residual = 0;
- SCpnt-SCp.ptr = (char *)SCpnt-sense_buffer;
- SCpnt-SCp.this_residual = sizeof(SCpnt-sense_buffer);
- SCpnt-SCp.phase = sizeof(SCpnt-sense_buffer);
+ init_SCp(SCpnt);
SCpnt-SCp.Message = 0;
SCpnt-SCp.Status = 0;
- SCpnt-request_bufflen = sizeof(SCpnt-sense_buffer);
- SCpnt-sc_data_direction = DMA_FROM_DEVICE;
- SCpnt-use_sg = 0;
SCpnt-tag = 0;
SCpnt-host_scribble = (void *)fas216_rq_sns_done;
diff --git a/drivers/scsi/arm/fas216.h b/drivers/scsi/arm/fas216.h
index 00e5f05..b65f4cf 100644
--- a/drivers/scsi/arm/fas216.h
+++ b/drivers/scsi/arm/fas216.h
@@ -16,6 +16,8 @@
#define NO_IRQ 255
#endif
+#include scsi/scsi_eh.h
+
#include queue.h
#include msgqueue.h
@@ -311,6 +313,7 @@ typedef struct {
/* miscellaneous */
int internal_done; /* flag to indicate
request done */
+ struct scsi_eh_save ses;/* holds request sense restore
info */
unsigned long magic_end;
} FAS216_Info;
--
1.5.3.8
-
To unsubscribe from this list: send the line unsubscribe
Re: [PATCH] scsi_error: Fix language abuse.
Alan Cox wrote: On Fri, 08 Feb 2008 20:32:54 -0500 Douglas Gilbert [EMAIL PROTECTED] wrote: Alan Cox wrote: The word illegal has a precise dictionary meaning of prohibited by law. Also contrary to or forbidden by official rules, regulations, etc. The OED I have here doesn't seem to think so, however if the words are the ones used in the T10 documentation then I'm happy to drop the patch. The OED (Oxford English dictionary) seems to be at odds with most other online dictionaries with respect to the word illegal. I find the following 'illegal' entry (at www.askoxford.com) daft: USAGE Both illegal and unlawful can mean 'contrary to or forbidden by law', but unlawful has a broader meaning 'not permitted by rules': thus handball in soccer is unlawful, but not illegal. So if we followed the OED's advice then SCSI's illegal request would become unlawful request. I'm yet to see the word unlawful in any technical standard ... 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
Re: [PATCH] scsi: ses fix mem leaking when fail to add intf
On Sunday 10 February 2008 08:28:38 pm James Bottomley wrote: On Sat, 2008-02-09 at 15:15 -0800, Yinghai Lu wrote: [PATCH] scsi: ses fix mem leaking when fail to add intf fix leaking with scomp leaking when failing. also remove one extra space. There are still a few extraneous code moves in this one. This is about the correct minimal set, isn't it? if buf allocation for page 7 get NULL... if put + if (!buf) + goto err_free; still not right, because still undo edev = enclosure_register(cdev-dev, sdev-sdev_gendev.bus_id, components, ses_enclosure_callbacks); all just add + if (!buf) + goto simple_populate; there? YH - 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
Patches not reaching the list
I tried to send few patches on last friday, out of which only one reached the list I resent the rest two and again only on reached the list Again I have resent the third patch two times, but they are still to reach the list. I am using mutt as E-mail client. I am sending this E-mail also from the same client, can any one please help me in sorting this out. Thanks Sathya - 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
[SCSI] ses: fix memory leaks
please check it...
---
From: Yinghai Lu [EMAIL PROTECTED]
[SCSI] ses: fix memory leaks
fix leaking with scomp leaking when failing. Also free page10 on
driver removal and remove one extra space.
Signed-off-by: Yinghai Lu [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/ses.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
Index: linux-2.6/drivers/scsi/ses.c
===
--- linux-2.6.orig/drivers/scsi/ses.c
+++ linux-2.6/drivers/scsi/ses.c
@@ -416,11 +416,11 @@ static int ses_intf_add(struct class_dev
int i, j, types, len, components = 0;
int err = -ENOMEM;
struct enclosure_device *edev;
- struct ses_component *scomp;
+ struct ses_component *scomp = NULL;
if (!scsi_device_enclosure(sdev)) {
/* not an enclosure, but might be in one */
- edev = enclosure_find(sdev-host-shost_gendev);
+ edev = enclosure_find(sdev-host-shost_gendev);
if (edev) {
ses_match_to_enclosure(edev, sdev);
class_device_put(edev-cdev);
@@ -456,9 +456,6 @@ static int ses_intf_add(struct class_dev
if (!buf)
goto err_free;
- ses_dev-page1 = buf;
- ses_dev-page1_len = len;
-
result = ses_recv_diag(sdev, 1, buf, len);
if (result)
goto recv_failed;
@@ -473,6 +470,9 @@ static int ses_intf_add(struct class_dev
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE)
components += type_ptr[1];
}
+ ses_dev-page1 = buf;
+ ses_dev-page1_len = len;
+ buf = NULL;
result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
if (result)
@@ -489,6 +489,7 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev-page2 = buf;
ses_dev-page2_len = len;
+ buf = NULL;
/* The additional information page --- allows us
* to match up the devices */
@@ -506,11 +507,12 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev-page10 = buf;
ses_dev-page10_len = len;
+ buf = NULL;
no_page10:
- scomp = kmalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
+ scomp = kzalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
if (!scomp)
- goto err_free;
+ goto err_free;
edev = enclosure_register(cdev-dev, sdev-sdev_gendev.bus_id,
components, ses_enclosure_callbacks);
@@ -521,10 +523,9 @@ static int ses_intf_add(struct class_dev
edev-scratch = ses_dev;
for (i = 0; i components; i++)
- edev-component[i].scratch = scomp++;
+ edev-component[i].scratch = scomp + i;
/* Page 7 for the descriptors is optional */
- buf = NULL;
result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto simple_populate;
@@ -532,6 +533,8 @@ static int ses_intf_add(struct class_dev
len = (hdr_buf[2] 8) + hdr_buf[3] + 4;
/* add 1 for trailing '\0' we'll use */
buf = kzalloc(len + 1, GFP_KERNEL);
+ if (!buf)
+ goto simple_polulate;
result = ses_recv_diag(sdev, 7, buf, len);
if (result) {
simple_populate:
@@ -598,6 +601,7 @@ static int ses_intf_add(struct class_dev
err = -ENODEV;
err_free:
kfree(buf);
+ kfree(scomp);
kfree(ses_dev-page10);
kfree(ses_dev-page2);
kfree(ses_dev-page1);
@@ -630,6 +634,7 @@ static void ses_intf_remove(struct class
ses_dev = edev-scratch;
edev-scratch = NULL;
+ kfree(ses_dev-page10);
kfree(ses_dev-page1);
kfree(ses_dev-page2);
kfree(ses_dev);
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: ses fix mem leaking when fail to add intf
On Sat, 2008-02-09 at 15:15 -0800, Yinghai Lu wrote:
[PATCH] scsi: ses fix mem leaking when fail to add intf
fix leaking with scomp leaking when failing.
also remove one extra space.
There are still a few extraneous code moves in this one. This is about
the correct minimal set, isn't it?
James
---
From: Yinghai Lu [EMAIL PROTECTED]
Date: Sat, 9 Feb 2008 15:15:47 -0800
Subject: [SCSI] ses: fix memory leaks
fix leaking with scomp leaking when failing. Also free page10 on
driver removal and remove one extra space.
Signed-off-by: Yinghai Lu [EMAIL PROTECTED]
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
drivers/scsi/ses.c | 20
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 2a6e4f4..8abc4a9 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -416,11 +416,11 @@ static int ses_intf_add(struct class_device *cdev,
int i, j, types, len, components = 0;
int err = -ENOMEM;
struct enclosure_device *edev;
- struct ses_component *scomp;
+ struct ses_component *scomp = NULL;
if (!scsi_device_enclosure(sdev)) {
/* not an enclosure, but might be in one */
- edev = enclosure_find(sdev-host-shost_gendev);
+ edev = enclosure_find(sdev-host-shost_gendev);
if (edev) {
ses_match_to_enclosure(edev, sdev);
class_device_put(edev-cdev);
@@ -456,9 +456,6 @@ static int ses_intf_add(struct class_device *cdev,
if (!buf)
goto err_free;
- ses_dev-page1 = buf;
- ses_dev-page1_len = len;
-
result = ses_recv_diag(sdev, 1, buf, len);
if (result)
goto recv_failed;
@@ -473,6 +470,9 @@ static int ses_intf_add(struct class_device *cdev,
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE)
components += type_ptr[1];
}
+ ses_dev-page1 = buf;
+ ses_dev-page1_len = len;
+ buf = NULL;
result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
if (result)
@@ -489,6 +489,7 @@ static int ses_intf_add(struct class_device *cdev,
goto recv_failed;
ses_dev-page2 = buf;
ses_dev-page2_len = len;
+ buf = NULL;
/* The additional information page --- allows us
* to match up the devices */
@@ -506,11 +507,12 @@ static int ses_intf_add(struct class_device *cdev,
goto recv_failed;
ses_dev-page10 = buf;
ses_dev-page10_len = len;
+ buf = NULL;
no_page10:
- scomp = kmalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
+ scomp = kzalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
if (!scomp)
- goto err_free;
+ goto err_free;
edev = enclosure_register(cdev-dev, sdev-sdev_gendev.bus_id,
components, ses_enclosure_callbacks);
@@ -521,7 +523,7 @@ static int ses_intf_add(struct class_device *cdev,
edev-scratch = ses_dev;
for (i = 0; i components; i++)
- edev-component[i].scratch = scomp++;
+ edev-component[i].scratch = scomp + i;
/* Page 7 for the descriptors is optional */
buf = NULL;
@@ -598,6 +600,7 @@ static int ses_intf_add(struct class_device *cdev,
err = -ENODEV;
err_free:
kfree(buf);
+ kfree(scomp);
kfree(ses_dev-page10);
kfree(ses_dev-page2);
kfree(ses_dev-page1);
@@ -630,6 +633,7 @@ static void ses_intf_remove(struct class_device *cdev,
ses_dev = edev-scratch;
edev-scratch = NULL;
+ kfree(ses_dev-page10);
kfree(ses_dev-page1);
kfree(ses_dev-page2);
kfree(ses_dev);
--
1.5.3.8
-
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] drivers/scsi/aic7xxx/aic7xxx_osm_pci.c - remove pointer comparison to 0
Signed-off-by: Joe Perches [EMAIL PROTECTED]
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
index dd6e21d..65e194f 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
@@ -301,7 +301,7 @@ ahc_linux_pci_reserve_io_region(struct ahc_softc *ahc,
u_long *base)
*base = pci_resource_start(ahc-dev_softc, 0);
if (*base == 0)
return (ENOMEM);
- if (request_region(*base, 256, aic7xxx) == 0)
+ if (!request_region(*base, 256, aic7xxx))
return (ENOMEM);
return (0);
}
@@ -318,7 +318,7 @@ ahc_linux_pci_reserve_mem_region(struct ahc_softc *ahc,
start = pci_resource_start(ahc-dev_softc, 1);
if (start != 0) {
*bus_addr = start;
- if (request_mem_region(start, 0x1000, aic7xxx) == 0)
+ if (!request_mem_region(start, 0x1000, aic7xxx))
error = ENOMEM;
if (error == 0) {
*maddr = ioremap_nocache(start, 256);
-
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
