Re: ide/arm/bast-ide.c compile error
FYI, from maintainers: SIMTEC EB2410ITX (BAST) P: Ben Dooks P: Vincent Sanders M: [EMAIL PROTECTED] W: http://www.simtec.co.uk/products/EB2410ITX/ S: Supported On Sat, Feb 09, 2008 at 02:15:15AM +0200, Adrian Bunk wrote: Commit 9e016a719209d95338e314b46c3012cc7feaaeec causes the following compile error: -- snip -- ... CC drivers/ide/arm/bast-ide.o /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/ide/arm/bast-ide.c: In function 'bastide_register': /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/ide/arm/bast-ide.c:31: error: 'hwif' redeclared as different kind of symbol /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/ide/arm/bast-ide.c:29: error: previous definition of 'hwif' was here make[4]: *** [drivers/ide/arm/bast-ide.o] Error 1 -- snip -- 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 -- 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-ide 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 Sunday 10 February 2008, Christoph Hellwig wrote:
On Sun, Feb 10, 2008 at 12:06:10AM +0100, Bartlomiej Zolnierkiewicz wrote:
Please try booting with hdx=noflush kernel parameter or please try
the attached patch which should fix the issue (if my theory is correct).
hda=noflush hdb=noflush hdd=noflush fixes the qemu setup for me.
Thanks for testing.
Thanks, I see now that there can be 1 flush request queued at a given
time.
Please dump the old patch and try this one.
[ Christoph: this may also fix your qemu/kvm+xfs problem. ]
It doesn't hang anymore but gives me the following oops instead (that is
after fixing the build as the bigger request-cmd breaks the scsi
build):
[...]
The OOPS is most likely (again) my fault - I was rushing out to push out
the fix and memset() line didn't get converted.
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:
From: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
Subject: [PATCH] ide-disk: fix flush requests
commit 813a0eb233ee67d7166241a8b389b6a76f2247f9
Author: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
Date: Fri Jan 25 22:17:10 2008 +0100
ide: switch idedisk_prepare_flush() to use REQ_TYPE_ATA_TASKFILE requests
...
broke flush requests.
Allocating IDE command structure on the stack for flush requests is not
a very brilliant idea:
- idedisk_prepare_flush() only prepares the request and it doesn't wait
for it to be completed
- there are can be multiple flush requests queued in the queue
Fix it by temporarily increasing size of BLK_MAX_CDB to accomodate for
ide_task_t structure if IDE subsystem is going to be used.
[ Jens: This will be fixed properly before 2.6.25 but this bug is rather
critical and the proper solution requires some more work + testing. ]
Thanks to Sebastian Siewior for bisecting/reporting the problem.
Cc: Sebastian Siewior [EMAIL PROTECTED]
Cc: Christoph Hellwig [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
Cc: Jens Axboe [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
Cc: Sergei Shtylyov [EMAIL PROTECTED]
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
---
drivers/ide/ide-disk.c | 14 +++---
include/linux/blkdev.h |5 +
2 files changed, 12 insertions(+), 7 deletions(-)
Index: b/drivers/ide/ide-disk.c
===
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -590,20 +590,20 @@ static ide_proc_entry_t idedisk_proc[] =
static void idedisk_prepare_flush(struct request_queue *q, struct request *rq)
{
ide_drive_t *drive = q-queuedata;
- ide_task_t task;
+ ide_task_t *task = (ide_task_t *)rq-cmd[0];
- memset(task, 0, sizeof(task));
+ memset(task, 0, sizeof(*task));
if (ide_id_has_flush_cache_ext(drive-id)
(drive-capacity64 = (1UL 28)))
- task.tf.command = WIN_FLUSH_CACHE_EXT;
+ task-tf.command = WIN_FLUSH_CACHE_EXT;
else
- task.tf.command = WIN_FLUSH_CACHE;
- task.tf_flags = IDE_TFLAG_OUT_TF | IDE_TFLAG_OUT_DEVICE;
- task.data_phase = TASKFILE_NO_DATA;
+ task-tf.command = WIN_FLUSH_CACHE;
+ task-tf_flags = IDE_TFLAG_OUT_TF | IDE_TFLAG_OUT_DEVICE;
+ task-data_phase = TASKFILE_NO_DATA;
rq-cmd_type = REQ_TYPE_ATA_TASKFILE;
rq-cmd_flags |= REQ_SOFTBARRIER;
- rq-special = task;
+ rq-special = task;
}
/*
Index: b/include/linux/blkdev.h
===
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -134,7 +134,12 @@ enum rq_flag_bits {
#define REQ_ALLOCED(1 __REQ_ALLOCED)
#define REQ_RW_META(1 __REQ_RW_META)
+/* FIXME: temporary hack to make flush requests work */
+#ifdef CONFIG_IDE
+#define BLK_MAX_CDB48 /* max sizeof(ide_task_t) */
+#else
#define BLK_MAX_CDB16
+#endif
/*
* try to put the fields that are referenced together in the same cacheline.
-
To unsubscribe from this list: send the line unsubscribe linux-ide 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, 2008-02-10 at 14:38 +0100, Bartlomiej Zolnierkiewicz wrote: On Sunday 10 February 2008, Christoph Hellwig wrote: On Sun, Feb 10, 2008 at 12:06:10AM +0100, Bartlomiej Zolnierkiewicz wrote: Please try booting with hdx=noflush kernel parameter or please try the attached patch which should fix the issue (if my theory is correct). hda=noflush hdb=noflush hdd=noflush fixes the qemu setup for me. Thanks for testing. Thanks, I see now that there can be 1 flush request queued at a given time. Please dump the old patch and try this one. [ Christoph: this may also fix your qemu/kvm+xfs problem. ] It doesn't hang anymore but gives me the following oops instead (that is after fixing the build as the bigger request-cmd breaks the scsi build): [...] The OOPS is most likely (again) my fault - I was rushing out to push out the fix and memset() line didn't get converted. 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: Doing something like this would have to be audited in SCSI ... we do assume sizeof(rq-cmd) == sizeof(scmd-cmnd) which will no longer be true. As long as sizeof(rq-cmd) is never used in SCSI code, it's probably safe. Although raising MAX_CDB by a factor of three has memory concerns as well, which aren't trivial and make this a bit too much of a hack. It's also incredibly fragile given that either ide_task_t could increase in size or someone could reduce MAX_CDB both with fatal consequences. Why not just use kmalloc(GFP_ATOMIC) instead? That will succeed 99% of the time and you can turn barriers off in a failure case. You'll have to free it in ide_end_drive_cmd(), but I think you've got (just) a spare tf_flag to mark a volatile task that needs kfree here. James - To unsubscribe from this list: send the line unsubscribe linux-ide 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-ide 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-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ide: add struct ide_port_info instances to legacy host drivers
On Sunday 10 February 2008, Atsushi Nemoto wrote: On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] wrote: + /* reset DMA masks only for SFF-style DMA controllers */ + if ((d-host_flags IDE_HFLAG_NO_DMA) == 0 hwif-dma_base == 0) + hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0; It might be too late, but host_flags IDE_HFLAGS_NO_DMA seems wrong for me. It is too late for -git but not too late for 2.6.25-rc1. ;-) Could you make a patch please? Thanks, Bart - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ide: add struct ide_port_info instances to legacy host drivers
On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] wrote: + /* reset DMA masks only for SFF-style DMA controllers */ + if ((d-host_flags IDE_HFLAG_NO_DMA) == 0 hwif-dma_base == 0) + hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0; It might be too late, but host_flags IDE_HFLAGS_NO_DMA seems wrong for me. --- Atsushi Nemoto - To unsubscribe from this list: send the line unsubscribe linux-ide 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 Sunday 10 February 2008, James Bottomley wrote:
On Sun, 2008-02-10 at 14:38 +0100, Bartlomiej Zolnierkiewicz wrote:
On Sunday 10 February 2008, Christoph Hellwig wrote:
On Sun, Feb 10, 2008 at 12:06:10AM +0100, Bartlomiej Zolnierkiewicz wrote:
Please try booting with hdx=noflush kernel parameter or please try
the attached patch which should fix the issue (if my theory is
correct).
hda=noflush hdb=noflush hdd=noflush fixes the qemu setup for me.
Thanks for testing.
Thanks, I see now that there can be 1 flush request queued at a given
time.
Please dump the old patch and try this one.
[ Christoph: this may also fix your qemu/kvm+xfs problem. ]
It doesn't hang anymore but gives me the following oops instead (that is
after fixing the build as the bigger request-cmd breaks the scsi
build):
[...]
The OOPS is most likely (again) my fault - I was rushing out to push out
the fix and memset() line didn't get converted.
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:
Doing something like this would have to be audited in SCSI ... we do
assume sizeof(rq-cmd) == sizeof(scmd-cmnd) which will no longer be
true. As long as sizeof(rq-cmd) is never used in SCSI code, it's
probably safe.
Although raising MAX_CDB by a factor of three has memory concerns as
well, which aren't trivial and make this a bit too much of a hack. It's
also incredibly fragile given that either ide_task_t could increase in
size or someone could reduce MAX_CDB both with fatal consequences.
Why not just use kmalloc(GFP_ATOMIC) instead? That will succeed 99% of
the time and you can turn barriers off in a failure case. You'll have
It seems to be too late to turn barriers off as all of the above happens
_inside_ prepare_flush_fn function. Nevertheless this is a much nicer
workaround and it should be sufficent for the time being - thanks James!
to free it in ide_end_drive_cmd(), but I think you've got (just) a spare
tf_flag to mark a volatile task that needs kfree here.
My precious last tf_flag... fortunately some other ones can be recycled...
Sebastian/Christoph, please test the final patch (after your ACK I'll push
it to Linus together with the rest of pending IDE fixes).
From: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
Subject: [PATCH] ide-disk: fix flush requests (take 2)
commit 813a0eb233ee67d7166241a8b389b6a76f2247f9
Author: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
Date: Fri Jan 25 22:17:10 2008 +0100
ide: switch idedisk_prepare_flush() to use REQ_TYPE_ATA_TASKFILE requests
...
broke flush requests.
Allocating IDE command structure on the stack for flush requests is not
a very brilliant idea:
- idedisk_prepare_flush() only prepares the request and it doesn't wait
for it to be completed
- there are can be multiple flush requests queued in the queue
Fix the problem (per hints from James Bottomley) by:
- dynamically allocating ide_task_t instance using kmalloc(..., GFP_ATOMIC)
- adding new taskfile flag (IDE_TFLAG_DYN)
- calling kfree() in ide_end_drive_command() if IDE_TFLAG_DYN is set
(while at it rename 'args' to 'task' and fix whitespace damage)
[ This will be fixed properly before 2.6.25 but this bug is rather
critical and the proper solution requires some more work + testing. ]
Thanks to Sebastian Siewior and Christoph Hellwig for reporitng the
problem and testing patches (extra thanks to Sebastian for bisecting
it to the guilty commmit).
Cc: Sebastian Siewior [EMAIL PROTECTED]
Cc: Christoph Hellwig [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
Cc: Jens Axboe [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
Cc: Sergei Shtylyov [EMAIL PROTECTED]
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
---
drivers/ide/ide-disk.c | 18 +++---
drivers/ide/ide-io.c | 16 ++--
include/linux/ide.h|2 ++
3 files changed, 23 insertions(+), 13 deletions(-)
Index: b/drivers/ide/ide-disk.c
===
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -590,20 +590,24 @@ static ide_proc_entry_t idedisk_proc[] =
static void idedisk_prepare_flush(struct request_queue *q, struct request *rq)
{
ide_drive_t *drive = q-queuedata;
- ide_task_t task;
+ ide_task_t *task = kmalloc(sizeof(*task), GFP_ATOMIC);
- memset(task, 0, sizeof(task));
+ /* FIXME: map struct ide_taskfile on rq-cmd[] */
+ BUG_ON(task == NULL);
+
+ memset(task, 0, sizeof(*task));
if (ide_id_has_flush_cache_ext(drive-id)
(drive-capacity64 = (1UL 28)))
-
Re: [PATCH] ide: introduce CONFIG_BLK_DEV_IDEDMA_SFF option
On Saturday 09 February 2008, Sergei Shtylyov wrote: Introduce new option CONFIG_BLK_DEV_IDEDMA_SFF for non-PCI SFF-8038i compatible bus mastering IDE controllers (which there are a few known), thus fixing a hack made for Palmchip BK3710 controller... Signed-off-by: Sergei Shtylyov [EMAIL PROTECTED] thanks, applied --- Bart, did you mean something like that? Anton, make sure it works, just in case... :-) I later noticed that there is still pci_alloc_consistent() call which needs to be converted to dma_alloc_coherent() in ide-dma.c... :/ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Current qc_defer implementation may lead to infinite recursion
Hi Tejun, due to your commit 31cc23b34913bc173680bdc87af79e551bf8cc0d libata now sets max_host_blocked and max_device_blocked to 1 for all devices it manages. Under certain conditions this may lead to system lockups due to infinite recursion as I have explained to James on the scsi list (kept you cc-ed). James told me that it was the business of libata to make sure that such a recursion cannot happen. In my discussion with James I imprudently claimed that this was easy to fix in libata. However, after giving the matter some thought, I'm not at all sure as to what exactly should be done about it. The easy bit is that max_host_blocked and max_device_blocked should be left alone as long as the low level driver does not provide the -qc_defer() callback. But even if the driver has defined this callback, ata_std_qc_defer() for one will not prevent this recursion on a uniprocessor, whereas things might work out well on an SMP system due to the lock fiddling in the scsi midlayer. As a conclusion, the current implementation makes it imperative to leave max_host_blocked and max_device_blocked alone on a uniprocessor system. For SMP systems the current implementation might just be fine but even there it might just as well be a good idea to make the adjustment depending on -qc_defer != NULL. Any ideas? Regards, Elias - To unsubscribe from this list: send the line unsubscribe linux-ide 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-ide 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-ide 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,
is there a value to CONFIG_NO_ATA_LEGACY?
in drivers/ata/libata-sff.c:
...
#if defined(CONFIG_NO_ATA_LEGACY)
/* Some platforms with PCI limits cannot address compat
port space. In that case we punt if their firmware has
left a device in compatibility mode */
if (legacy_mode) {
printk(KERN_ERR ata: Compatibility mode ATA is not
supported on this platform, skipping.\n);
return -EOPNOTSUPP;
}
#endif
...
there is no Kconfig file that defines that variable -- should that
test be removed? or is something eventually coming that defines it?
rday
--
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
Home page: http://crashcourse.ca
Fedora Cookbook:http://crashcourse.ca/wiki/index.php/Fedora_Cookbook
-
To unsubscribe from this list: send the line unsubscribe linux-ide 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.
* Bartlomiej Zolnierkiewicz | 2008-02-10 19:32:06 [+0100]: Sebastian/Christoph, please test the final patch (after your ACK I'll push it to Linus together with the rest of pending IDE fixes). This seems to work. Sebastian - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libata: Forcing PIO0 mode on reset must not freeze system
Calling ap-ops-set_piomode(ap, dev) on a device/controller which got
already removed, locks the system hard. Reproducibly on an X60 attached to
a dock station containing a cdrom device with doing
$ echo 1 /sys/devices/platform/dock.0/undock echo 123 /dev/sr0
This calls ata_eh_reset(...) which in turn tries to force PIO mode 0. But
the device is already gone.
Bisecting revealed the following commit as culprit:
commit cdeab1140799f09c5f728a5ff85e0bdfa5679cd2
Author: Tejun Heo [EMAIL PROTECTED]
Date: Mon Oct 29 16:41:09 2007 +0900
libata: relocate forcing PIO0 on reset
Forcing PIO0 on reset was done inside ata_bus_softreset(), which is a
bit out of place as it should be applied to all resets - hard, soft
and implementation which don't use ata_bus_softreset(). Relocate it
such that...
* For new EH, it's done in ata_eh_reset() before calling prereset.
* For old EH, it's done before calling ap-ops-phy_reset() in
ata_bus_probe().
This makes PIO0 forced after all resets. Another difference is that
reset itself is done after PIO0 is forced.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Acked-by: Alan Cox [EMAIL PROTECTED]
Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
ATTENTION! The following patch solves the problem on my system, but please
be aware that I don't really know what I'm doing because I don't have the
big picture. There's surely a better way to check if the device/controller
is still functional than calling ata_link_{online,offline}.
Signed-off-by: Holger Macht [EMAIL PROTECTED]
---
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 4e31071..d6a7c57 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2131,23 +2131,25 @@ int ata_eh_reset(struct ata_link *link, int classify,
ata_eh_about_to_do(link, NULL, ehc-i.action ATA_EH_RESET_MASK);
- ata_link_for_each_dev(dev, link) {
- /* If we issue an SRST then an ATA drive (not ATAPI)
-* may change configuration and be in PIO0 timing. If
-* we do a hard reset (or are coming from power on)
-* this is true for ATA or ATAPI. Until we've set a
-* suitable controller mode we should not touch the
-* bus as we may be talking too fast.
-*/
- dev-pio_mode = XFER_PIO_0;
+ if (ata_link_online(link) != ata_link_offline(link)) {
+ ata_link_for_each_dev(dev, link) {
+ /* If we issue an SRST then an ATA drive (not ATAPI)
+* may change configuration and be in PIO0 timing. If
+* we do a hard reset (or are coming from power on)
+* this is true for ATA or ATAPI. Until we've set a
+* suitable controller mode we should not touch the
+* bus as we may be talking too fast.
+*/
+ dev-pio_mode = XFER_PIO_0;
- /* If the controller has a pio mode setup function
-* then use it to set the chipset to rights. Don't
-* touch the DMA setup as that will be dealt with when
-* configuring devices.
-*/
- if (ap-ops-set_piomode)
- ap-ops-set_piomode(ap, dev);
+ /* If the controller has a pio mode setup function
+* then use it to set the chipset to rights. Don't
+* touch the DMA setup as that will be dealt with when
+* configuring devices.
+*/
+ if (ap-ops-set_piomode)
+ ap-ops-set_piomode(ap, dev);
+ }
}
/* Determine which reset to use and record in ehc-i.action.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
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-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: About forcing 32bit DMA patch for AMD690G(SB600)
Shane Huang [EMAIL PROTECTED] wrote: Dear Tejun: My 5 cents: Just order the board. These stock PC hardware are too cheap these days, it doesn't make any sense to try to debug somewhat difficult problem remotely if the hardware is available on the market. Even if you have to spend your own money, it will be money well spent compared to the time and effort you'll have to spend in comparison - hardware is just too cheap. Yes you are right, we are trying to get one ASUS M2A-VM board. Sorry folks I've been offline for a few weeks I notice I missed out a lot of important emails. Now that I see I need to verify certain things, I'm very happy to do it. Just give me a day or two. Thanks Shane Thanks Srihari Get the name you always wanted with the new y7mail email address. www.yahoo7.com.au/y7mail - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sata_mv: platform driver allocs dma without create
When the sata_mv driver is used as a platform driver,
mv_create_dma_pools() is never called so it fails when trying
to alloc in mv_pool_start().
Signed-off-by: Byron Bradley [EMAIL PROTECTED]
Acked-by: Mark Lord [EMAIL PROTECTED]
---
Mark, based on the comment from Andrew Morton on the sata_mv: fix loop
with last port patch I have changed your Signed-off-by to an Acked-by.
Hopefully thats correct.
drivers/ata/sata_mv.c | 44
1 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index f5333ce..04b5717 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -2881,6 +2881,26 @@ done:
return rc;
}
+static int mv_create_dma_pools(struct mv_host_priv *hpriv, struct device *dev)
+{
+ hpriv-crqb_pool = dmam_pool_create(crqb_q, dev, MV_CRQB_Q_SZ,
+MV_CRQB_Q_SZ, 0);
+ if (!hpriv-crqb_pool)
+ return -ENOMEM;
+
+ hpriv-crpb_pool = dmam_pool_create(crpb_q, dev, MV_CRPB_Q_SZ,
+MV_CRPB_Q_SZ, 0);
+ if (!hpriv-crpb_pool)
+ return -ENOMEM;
+
+ hpriv-sg_tbl_pool = dmam_pool_create(sg_tbl, dev, MV_SG_TBL_SZ,
+MV_SG_TBL_SZ, 0);
+ if (!hpriv-sg_tbl_pool)
+ return -ENOMEM;
+
+ return 0;
+}
+
/**
* mv_platform_probe - handle a positive probe of an soc Marvell
* host
@@ -2934,6 +2954,10 @@ static int mv_platform_probe(struct platform_device
*pdev)
hpriv-base = ioremap(res-start, res-end - res-start + 1);
hpriv-base -= MV_SATAHC0_REG_BASE;
+ rc = mv_create_dma_pools(hpriv, pdev-dev);
+ if (rc)
+ return rc;
+
/* initialize adapter */
rc = mv_init_host(host, chip_soc);
if (rc)
@@ -3070,26 +3094,6 @@ static void mv_print_info(struct ata_host *host)
scc_s, (MV_HP_FLAG_MSI hpriv-hp_flags) ? MSI : INTx);
}
-static int mv_create_dma_pools(struct mv_host_priv *hpriv, struct device *dev)
-{
- hpriv-crqb_pool = dmam_pool_create(crqb_q, dev, MV_CRQB_Q_SZ,
-MV_CRQB_Q_SZ, 0);
- if (!hpriv-crqb_pool)
- return -ENOMEM;
-
- hpriv-crpb_pool = dmam_pool_create(crpb_q, dev, MV_CRPB_Q_SZ,
-MV_CRPB_Q_SZ, 0);
- if (!hpriv-crpb_pool)
- return -ENOMEM;
-
- hpriv-sg_tbl_pool = dmam_pool_create(sg_tbl, dev, MV_SG_TBL_SZ,
-MV_SG_TBL_SZ, 0);
- if (!hpriv-sg_tbl_pool)
- return -ENOMEM;
-
- return 0;
-}
-
/**
* mv_pci_init_one - handle a positive probe of a PCI Marvell host
* @pdev: PCI device found
--
1.5.4.rc2.38.gd6da3
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.26-git0: IDE oops during boot
On 2/7/08, Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] wrote: On Thursday 07 February 2008, Kamalesh Babulal wrote: Bartlomiej Zolnierkiewicz wrote: Hi, On Wednesday 06 February 2008, Pavel Machek wrote: On Wed 2008-02-06 11:53:34, Pavel Machek wrote: Hi! Trying to boot 2.6.25-git0 (few days old), I get BUG: unable to handle kernel paging request at ..ffb0 IP at init_irq+0x42e init_irq? hmm... Call trace: ide_device_add_all this comes from ide-generic (Generic IDE host driver) ide_generic_init kernel_init child_rip vgacon_cursor kernel_init child_rip Excerpt from config: CONFIG_IDE=y CONFIG_BLK_DEV_IDE=y Disabling CONFIG_IDE made my machine boot, as it was using libata anyway. Kamalesh/Pavel: Could you try latest git and see if the OOPS is still there? [ Yeah, I'm unable to reproduce it. :( ] Thanks, Bart Hi Bart, The panic is reproducible with the 2.6.24-git16 kernel, the call trace is similar to the previous one Thanks, I again reviewed ide-probe.c changes but nothing seems wrong... Could you please bisect it down to the guilty commit? Kamalesh, were you able to bisect this down? I just got hit by the same panic on a 4-way x86_64, with 2.6.24-git22. Thanks, Nish - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: is there a value to CONFIG_NO_ATA_LEGACY?
On Sun, 10 Feb 2008 14:44:16 -0500 (EST)
Robert P. J. Day [EMAIL PROTECTED] wrote:
in drivers/ata/libata-sff.c:
...
#if defined(CONFIG_NO_ATA_LEGACY)
/* Some platforms with PCI limits cannot address compat
port space. In that case we punt if their firmware has
left a device in compatibility mode */
if (legacy_mode) {
printk(KERN_ERR ata: Compatibility mode ATA is not
supported on this platform, skipping.\n);
return -EOPNOTSUPP;
}
#endif
...
there is no Kconfig file that defines that variable -- should that
test be removed? or is something eventually coming that defines it?
It is used by some of the unsubmitted tree stuff certainly and is there
because some platforms will need it (eg FRV).
Alan
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: is there a value to CONFIG_NO_ATA_LEGACY?
On Sun, 10 Feb 2008, Alan Cox wrote:
On Sun, 10 Feb 2008 14:44:16 -0500 (EST)
Robert P. J. Day [EMAIL PROTECTED] wrote:
in drivers/ata/libata-sff.c:
...
#if defined(CONFIG_NO_ATA_LEGACY)
/* Some platforms with PCI limits cannot address compat
port space. In that case we punt if their firmware has
left a device in compatibility mode */
if (legacy_mode) {
printk(KERN_ERR ata: Compatibility mode ATA is not
supported on this platform, skipping.\n);
return -EOPNOTSUPP;
}
#endif
...
there is no Kconfig file that defines that variable -- should that
test be removed? or is something eventually coming that defines it?
It is used by some of the unsubmitted tree stuff certainly and is there
because some platforms will need it (eg FRV).
ok, good enough, i'm just trying to cull my list of unreferenced
Kconfig variables.
rday
--
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
Home page: http://crashcourse.ca
Fedora Cookbook:http://crashcourse.ca/wiki/index.php/Fedora_Cookbook
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ide: add struct ide_port_info instances to legacy host drivers
On Sunday 10 February 2008, Bartlomiej Zolnierkiewicz wrote: On Sunday 10 February 2008, Atsushi Nemoto wrote: On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] wrote: + /* reset DMA masks only for SFF-style DMA controllers */ + if ((d-host_flags IDE_HFLAG_NO_DMA) == 0 hwif-dma_base == 0) + hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0; It might be too late, but host_flags IDE_HFLAGS_NO_DMA seems wrong for me. It is too late for -git but not too late for 2.6.25-rc1. ;-) Could you make a patch please? I went ahead and made a patch (sorry, I needed it for git pull request :). From: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] Subject: [PATCH] ide: ide_init_port() bugfix On Sunday 10 February 2008, Atsushi Nemoto wrote: On Sun, 06 Jan 2008 18:03:10 +0100, Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] wrote: + /* reset DMA masks only for SFF-style DMA controllers */ + if ((d-host_flags IDE_HFLAG_NO_DMA) == 0 hwif-dma_base == 0) + hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0; It might be too late, but host_flags IDE_HFLAGS_NO_DMA seems wrong for me. Fix regression caused by commmit c413b9b94d9a8e7548cc4b2e04b7df0439ce76fd (ide: add struct ide_port_info instances to legacy host drivers). Reported-by: Atsushi Nemoto [EMAIL PROTECTED] Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] --- drivers/ide/ide-probe.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/drivers/ide/ide-probe.c === --- a/drivers/ide/ide-probe.c +++ b/drivers/ide/ide-probe.c @@ -1355,7 +1355,7 @@ static void ide_init_port(ide_hwif_t *hw hwif-ultra_mask = d-udma_mask; /* reset DMA masks only for SFF-style DMA controllers */ - if ((d-host_flags IDE_HFLAG_NO_DMA) == 0 hwif-dma_base == 0) + if ((d-host_flags IDE_HFLAG_NO_DMA) == 0 hwif-dma_base == 0) hwif-swdma_mask = hwif-mwdma_mask = hwif-ultra_mask = 0; if (d-host_flags IDE_HFLAG_RQSIZE_256) - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ide: fix comment in init_irq()
APUS support is gone... Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] --- drivers/ide/ide-probe.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/drivers/ide/ide-probe.c === --- a/drivers/ide/ide-probe.c +++ b/drivers/ide/ide-probe.c @@ -1051,7 +1051,7 @@ static int init_irq (ide_hwif_t *hwif) int sa = 0; #if defined(__mc68000__) sa = IRQF_SHARED; -#endif /* __mc68000__ || CONFIG_APUS */ +#endif /* __mc68000__ */ if (IDE_CHIPSET_IS_PCI(hwif-chipset)) sa = IRQF_SHARED; - To unsubscribe from this list: send the line unsubscribe linux-ide 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 Sunday 10 February 2008, Sebastian Siewior wrote: * Bartlomiej Zolnierkiewicz | 2008-02-10 19:32:06 [+0100]: Sebastian/Christoph, please test the final patch (after your ACK I'll push it to Linus together with the rest of pending IDE fixes). This seems to work. Thanks. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ide: remove stale comment from ide-lib.c
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
---
drivers/ide/ide-lib.c |9 -
1 file changed, 9 deletions(-)
Index: b/drivers/ide/ide-lib.c
===
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -21,15 +21,6 @@
#include asm/uaccess.h
#include asm/io.h
-/*
- * IDE library routines. These are plug in code that most
- * drivers can use but occasionally may be weird enough
- * to want to do their own thing with
- *
- * Add common non I/O op stuff here. Make sure it has proper
- * kernel-doc function headers or your patch will be rejected
- */
-
static const char *udma_str[] =
{ UDMA/16, UDMA/25, UDMA/33, UDMA/44,
UDMA/66, UDMA/100, UDMA/133, UDMA7 };
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[git patches] IDE fixes
Hi,
We merged _a_lot_ of IDE patches for 2.6.25 so no wonder that a few bugs
showed up (yes, mostly brown paper ones of mine :). This update should
put it under control again (there are two more open regression bugreports
left and they are going to be addressed as soon as we have more data).
- fix nasty bug in handling of flush requests (extra thanks to Sebastian
Siewior / James Bottomley / Christoph Hellwig for help with fixing it)
- fix ide_port_init() regression (spotted by Atsushi Nemoto)
- fix build of bast-ide of gayle host drivers (Adrian Bunk)
- fixes for Palm BK3710 support (Sergei Shtylyov and me)
- fix another possible ide-cd panic (Kiyoshi Ueda)
- other minor fixes
Please pull from:
master.kernel.org:/pub/scm/linux/kernel/git/bart/ide-2.6.git/
to receive the following updates:
drivers/ide/Kconfig| 26 --
drivers/ide/arm/bast-ide.c | 12 ++
drivers/ide/arm/palm_bk3710.c | 74 ---
drivers/ide/ide-cd.c |2 +-
drivers/ide/ide-disk.c | 18 ++
drivers/ide/ide-dma.c | 14
drivers/ide/ide-io.c | 19 ++
drivers/ide/ide-iops.c | 10 +
drivers/ide/ide-lib.c |9 -
drivers/ide/ide-probe.c|4 +-
drivers/ide/ide-tape.c | 34 --
drivers/ide/ide.c |4 +--
drivers/ide/legacy/gayle.c |2 +-
drivers/ide/pci/cs5520.c |5 ---
drivers/ide/pci/pdc202xx_old.c | 22
include/linux/ide.h| 11 +++---
16 files changed, 107 insertions(+), 159 deletions(-)
Adrian Bunk (1):
ide: fix ide/legacy/gayle.c compilation
Bartlomiej Zolnierkiewicz (12):
palm_bk3710: ide_register_hw() - ide_device_add()
palm_bk3710: fix ide_unregister() usage
palm_bk3710: port initialization/probing bugfix
palm_bk3710: use struct ide_port_info
pdc202xx_old: always enable burst mode
ide: remove stale version number
ide-tape: remove never executed code
bast-ide: build fix
ide-disk: fix flush requests (take 2)
ide: ide_init_port() bugfix
ide: fix comment in init_irq()
ide: remove stale comment from ide-lib.c
Benjamin Herrenschmidt (1):
cs5520: remove stale comment
Borislav Petkov (1):
ide-cd: replace ntohs with generic byteorder macro be16_to_cpu
Kiyoshi Ueda (1):
ide: another possible ide panic fix for blk-end-request
Sergei Shtylyov (2):
ide: insert BUG_ON() into __ide_set_handler() (take 2)
ide: introduce CONFIG_BLK_DEV_IDEDMA_SFF option
diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig
index 043c34a..df752e6 100644
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -378,6 +378,9 @@ config BLK_DEV_IDEPNP
would like the kernel to automatically detect and activate
it, say Y here.
+config BLK_DEV_IDEDMA_SFF
+ bool
+
if PCI
comment PCI IDE chipsets support
@@ -459,6 +462,7 @@ config BLK_DEV_RZ1000
config BLK_DEV_IDEDMA_PCI
bool
select BLK_DEV_IDEPCI
+ select BLK_DEV_IDEDMA_SFF
config BLK_DEV_AEC62XX
tristate AEC62XX chipset support
@@ -688,23 +692,6 @@ config BLK_DEV_PDC202XX_OLD
If unsure, say N.
-config PDC202XX_BURST
- bool Special UDMA Feature
- depends on BLK_DEV_PDC202XX_OLD
- help
- This option causes the pdc202xx driver to enable UDMA modes on the
- PDC202xx even when the PDC202xx BIOS has not done so.
-
- It was originally designed for the PDC20246/Ultra33, whose BIOS will
- only setup UDMA on the first two PDC20246 cards. It has also been
- used successfully on a PDC20265/Ultra100, allowing use of UDMA modes
- when the PDC20265 BIOS has been disabled (for faster boot up).
-
- Please read the comments at the top of
- file:drivers/ide/pci/pdc202xx_old.c.
-
- If unsure, say N.
-
config BLK_DEV_PDC202XX_NEW
tristate PROMISE PDC202{68|69|70|71|75|76|77} support
select BLK_DEV_IDEDMA_PCI
@@ -1016,7 +1003,7 @@ config BLK_DEV_Q40IDE
config BLK_DEV_PALMCHIP_BK3710
tristate Palmchip bk3710 IDE controller support
depends on ARCH_DAVINCI
- select BLK_DEV_IDEDMA_PCI
+ select BLK_DEV_IDEDMA_SFF
help
Say Y here if you want to support the onchip IDE controller on the
TI DaVinci SoC
@@ -1124,7 +,8 @@ config BLK_DEV_UMC8672
endif
config BLK_DEV_IDEDMA
- def_bool BLK_DEV_IDEDMA_PCI || BLK_DEV_IDEDMA_PMAC ||
BLK_DEV_IDEDMA_ICS || BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA
+ def_bool BLK_DEV_IDEDMA_SFF || BLK_DEV_IDEDMA_PMAC || \
+BLK_DEV_IDEDMA_ICS || BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA
config IDE_ARCH_OBSOLETE_INIT
def_bool ALPHA || (ARM !ARCH_L7200) || BLACKFIN || X86 || IA64 ||
M32R || MIPS || PARISC || PPC || (SUPERH64 BLK_DEV_IDEPCI) || SPARC
diff --git
[PATCH] Prevent IDE boot ops on NUMA system in mainline
Without this patch a Opteron test system here oopses at boot with currentg git.
Calling to_pci_dev() on a NULL pointer gives a negative value so the following
NULL
pointer check never triggers and then an illegal address is referenced. Check
the
unadjusted original device pointer for NULL instead.
Signed-off-by: Andi Kleen [EMAIL PROTECTED]
Index: linux/include/linux/ide.h
===
--- linux.orig/include/linux/ide.h
+++ linux/include/linux/ide.h
@@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
static inline int hwif_to_node(ide_hwif_t *hwif)
{
struct pci_dev *dev = to_pci_dev(hwif-dev);
- return dev ? pcibus_to_node(dev-bus) : -1;
+ return hwif-dev ? pcibus_to_node(dev-bus) : -1;
}
static inline ide_drive_t *ide_get_paired_drive(ide_drive_t *drive)
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: Forcing PIO0 mode on reset must not freeze system
Hello,
Holger Macht wrote:
Calling ap-ops-set_piomode(ap, dev) on a device/controller which got
already removed, locks the system hard. Reproducibly on an X60 attached to
a dock station containing a cdrom device with doing
$ echo 1 /sys/devices/platform/dock.0/undock echo 123 /dev/sr0
This calls ata_eh_reset(...) which in turn tries to force PIO mode 0. But
the device is already gone.
Bisecting revealed the following commit as culprit:
commit cdeab1140799f09c5f728a5ff85e0bdfa5679cd2
Author: Tejun Heo [EMAIL PROTECTED]
Date: Mon Oct 29 16:41:09 2007 +0900
libata: relocate forcing PIO0 on reset
Forcing PIO0 on reset was done inside ata_bus_softreset(), which is a
bit out of place as it should be applied to all resets - hard, soft
and implementation which don't use ata_bus_softreset(). Relocate it
such that...
* For new EH, it's done in ata_eh_reset() before calling prereset.
* For old EH, it's done before calling ap-ops-phy_reset() in
ata_bus_probe().
This makes PIO0 forced after all resets. Another difference is that
reset itself is done after PIO0 is forced.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Acked-by: Alan Cox [EMAIL PROTECTED]
Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
ATTENTION! The following patch solves the problem on my system, but please
be aware that I don't really know what I'm doing because I don't have the
big picture. There's surely a better way to check if the device/controller
is still functional than calling ata_link_{online,offline}.
In the above example, even the reset sequence itself can cause hang if
the hardware is implemented slightly differently. The reason why
set_piomode() locks up but reset sequence doesn't is simple dumb luck.
I think the proper fix is to tell libata to detach the cdrom before
undocking.
Signed-off-by: Holger Macht [EMAIL PROTECTED]
NACK.
--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide 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-ide 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-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Current qc_defer implementation may lead to infinite recursion
Tejun Heo [EMAIL PROTECTED] wrote: Elias Oltmanns wrote: Hi Tejun, due to your commit 31cc23b34913bc173680bdc87af79e551bf8cc0d libata now sets max_host_blocked and max_device_blocked to 1 for all devices it manages. Under certain conditions this may lead to system lockups due to infinite recursion as I have explained to James on the scsi list (kept you cc-ed). James told me that it was the business of libata to make sure that such a recursion cannot happen. In my discussion with James I imprudently claimed that this was easy to fix in libata. However, after giving the matter some thought, I'm not at all sure as to what exactly should be done about it. The easy bit is that max_host_blocked and max_device_blocked should be left alone as long as the low level driver does not provide the -qc_defer() callback. But even if the driver has defined this callback, ata_std_qc_defer() for one will not prevent this recursion on a uniprocessor, whereas things might work out well on an SMP system due to the lock fiddling in the scsi midlayer. As a conclusion, the current implementation makes it imperative to leave max_host_blocked and max_device_blocked alone on a uniprocessor system. For SMP systems the current implementation might just be fine but even there it might just as well be a good idea to make the adjustment depending on -qc_defer != NULL. Hmmm... The reason why max_host_blocked and max_device_blocked are set to 1 is to let libata re-consider status after each command completion as blocked status can be rather complex w/ PMP. I haven't really followed the code yet but you're saying that blocked count of 2 should be used for that behavior, right? Not quite. On an SMP system the current implementation will probably do exactly what you had in mind. In particular, setting max_device_blocked and max_host_blocked to 1 seems to be the right thing to do in this case. Another strange thing is that there hasn't been any such lock up / infinite recursion report till now although -qc_defer mechanism bas been used widely for some time now. Can you reproduce the problem w/o the disk shock protection? No, unfortunately, I'm unable to reproduce this without the patch on my machine. This is for purely technical reasons though because I'm using ata_piix. Running a vanilla kernel, I'd expect everything to work just fine except for one case: A non-SMP system using a driver that provides the -qc_defer() callback. Currently, the -qc_defer() callback is the only thing that can possibly send a non zero return value to the scsi midlayer. Once it does, however, the driver will only get a chance to complete some qcs before -qc_defer() is called again provided that multithreading is supported. So, what I'm saying is this: If the low level driver doesn't provide a -qc_defer() callback, there is no (obvious) reason why max_device_blocked and max_host_blocked should be set to 1 since libata won't gain anything by it. However, it is not a bug either, even though James considers it suboptimal and I will have to think about a solution for my patch. On the other hand, once a driver defines the -qc_defer() callback, we really have a bug because things will go wrong once -qc_defer() returns non zero on a uniprocessor. So, in this case max_device_blocked and max_host_blocked should be set to 1 on an SMP system and *have to* be bigger than 1 otherwise. Regards, Elias - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.26-git0: IDE oops during boot
Nish Aravamudan wrote:
On 2/7/08, Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] wrote:
On Thursday 07 February 2008, Kamalesh Babulal wrote:
Bartlomiej Zolnierkiewicz wrote:
Hi,
On Wednesday 06 February 2008, Pavel Machek wrote:
On Wed 2008-02-06 11:53:34, Pavel Machek wrote:
Hi!
Trying to boot 2.6.25-git0 (few days old), I get
BUG: unable to handle kernel paging request at ..ffb0
IP at init_irq+0x42e
init_irq? hmm...
Call trace:
ide_device_add_all
this comes from ide-generic
(Generic IDE host driver)
ide_generic_init
kernel_init
child_rip
vgacon_cursor
kernel_init
child_rip
Excerpt from config:
CONFIG_IDE=y
CONFIG_BLK_DEV_IDE=y
Disabling CONFIG_IDE made my machine boot, as it was using libata
anyway.
Kamalesh/Pavel:
Could you try latest git and see if the OOPS is still there?
[ Yeah, I'm unable to reproduce it. :( ]
Thanks,
Bart
Hi Bart,
The panic is reproducible with the 2.6.24-git16 kernel, the call trace is
similar to the previous one
Thanks, I again reviewed ide-probe.c changes but nothing seems wrong...
Could you please bisect it down to the guilty commit?
Kamalesh, were you able to bisect this down? I just got hit by the
same panic on a 4-way x86_64, with 2.6.24-git22.
Thanks,
Nish
Hi Nish,
I tried bisecting and the guilty patch seems to be
36501650ec45b1db308c3b51886044863be2d762 is first bad commit
commit 36501650ec45b1db308c3b51886044863be2d762
Author: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
Date: Fri Feb 1 23:09:31 2008 +0100
ide: keep pointer to struct device instead of struct pci_dev in ide_hwif_t
the gdb output, also points to the changes made by the guilty patch
(gdb) p ide_device_add_all
$1 = {int (u8 *, const struct ide_port_info *)} 0x804176ac
ide_device_add_all
(gdb) p/x 0x804176ac+0xb60
$2 = 0x8041820c
(gdb) l *0x8041820c
0x8041820c is in ide_device_add_all (drivers/ide/ide-probe.c:1249).
1244goto out;
1245}
1246
1247sg_init_table(hwif-sg_table, hwif-sg_max_nents);
1248
1249if (init_irq(hwif) == 0)
1250goto done;
1251
1252old_irq = hwif-irq;
1253/*
(gdb)
(gdb) p init_irq
$1 = {int (ide_hwif_t *)} 0x8041721f init_irq
(gdb) p/x 0x8041721f+0x1a4
$2 = 0x804173c3
(gdb) l *0x804173c3
0x804173c3 is in init_irq (include/asm/pci.h:101).
96 /* Returns the node based on pci bus */
97 static inline int __pcibus_to_node(struct pci_bus *bus)
98 {
99 struct pci_sysdata *sd = bus-sysdata;
100
101 return sd-node;
102 }
103
104 static inline cpumask_t __pcibus_to_cpumask(struct pci_bus *bus)
105 {
(gdb)
--
Thanks Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.
-
To unsubscribe from this list: send the line unsubscribe linux-ide 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-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Current qc_defer implementation may lead to infinite recursion
Elias Oltmanns wrote: Hi Tejun, due to your commit 31cc23b34913bc173680bdc87af79e551bf8cc0d libata now sets max_host_blocked and max_device_blocked to 1 for all devices it manages. Under certain conditions this may lead to system lockups due to infinite recursion as I have explained to James on the scsi list (kept you cc-ed). James told me that it was the business of libata to make sure that such a recursion cannot happen. In my discussion with James I imprudently claimed that this was easy to fix in libata. However, after giving the matter some thought, I'm not at all sure as to what exactly should be done about it. The easy bit is that max_host_blocked and max_device_blocked should be left alone as long as the low level driver does not provide the -qc_defer() callback. But even if the driver has defined this callback, ata_std_qc_defer() for one will not prevent this recursion on a uniprocessor, whereas things might work out well on an SMP system due to the lock fiddling in the scsi midlayer. As a conclusion, the current implementation makes it imperative to leave max_host_blocked and max_device_blocked alone on a uniprocessor system. For SMP systems the current implementation might just be fine but even there it might just as well be a good idea to make the adjustment depending on -qc_defer != NULL. Hmmm... The reason why max_host_blocked and max_device_blocked are set to 1 is to let libata re-consider status after each command completion as blocked status can be rather complex w/ PMP. I haven't really followed the code yet but you're saying that blocked count of 2 should be used for that behavior, right? Another strange thing is that there hasn't been any such lock up / infinite recursion report till now although -qc_defer mechanism bas been used widely for some time now. Can you reproduce the problem w/o the disk shock protection? Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
