Re: Current git -- kaboom [bisect] seems IDE related.
Bartlomiej Zolnierkiewicz wrote:
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]
Acked-by: Sergei Shtylyov [EMAIL PROTECTED]
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));
Why not kzalloc() then?
MBR, Sergei
-
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 Monday 11 February 2008, Sergei Shtylyov wrote:
Bartlomiej Zolnierkiewicz wrote:
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]
Acked-by: Sergei Shtylyov [EMAIL PROTECTED]
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));
Why not kzalloc() then?
Well, yes. However since the patch has been already merged upstream and it
is only a temporary solution I don't think it is worth doing it now (OTOH if
somebody sends me a patch I'll happily merge it).
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: 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: 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: 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
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
Re: Current git -- kaboom [bisect] seems IDE related.
Hi,
On Saturday 09 February 2008, Sebastian Siewior wrote:
Hello,
current git results in a freeze. I tracked it down to
| error = type-get_sb(type, flags, name, data, mnt);
in vfs_kern_mount(), fs/super.c (what is called from mount_root() during
the boot process). I use XFS on my rootfs partition. After this, I
started to bisect.
During the bisect process I stepped into two things. The commit
|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
|
|Based on the earlier work by Tejun Heo.
|
|There should be no functionality changes caused by this patch.
|
|Cc: Tejun Heo [EMAIL PROTECTED]
|Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
|
|:04 04 e4708c62aef233d2b07da20aedd644182258f1f1
66aea951e7c32aa82621d8460b0f97ff132f23a6 M drivers
turned the working boot process into [1]. Since this isn't the freeze I
was looking I continued my bisect process. The commit
It could be that due to block layer changes to I/O barrier handling
allocating ATA command structure on the stack is no longer safe.
Please try booting with hdx=noflush kernel parameter or please try
the attached patch which should fix the issue (if my theory is correct).
[...]
turned the Oops into the freeze. There was no working commit in between
(well, git-bisect did not find one).
My .config [2], my lspci and cpuinfo [3].
[1] http://download.breakpoint.cc/lnx-2.6.24-post-crash.jpg
[2] http://download.breakpoint.cc/notebook-config.txt
[3] http://download.breakpoint.cc/system.txt
---
drivers/ide/ide-disk.c | 14 +--
include/linux/ide.h| 206 -
2 files changed, 111 insertions(+), 109 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 = drive-tf_task;
- 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/ide.h
===
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -32,6 +32,108 @@
# define SUPPORT_VLB_SYNC 1
#endif
+enum {
+ IDE_TFLAG_LBA48 = (1 0),
+ IDE_TFLAG_NO_SELECT_MASK= (1 1),
+ IDE_TFLAG_FLAGGED = (1 2),
+ IDE_TFLAG_OUT_DATA = (1 3),
+ IDE_TFLAG_OUT_HOB_FEATURE = (1 4),
+ IDE_TFLAG_OUT_HOB_NSECT = (1 5),
+ IDE_TFLAG_OUT_HOB_LBAL = (1 6),
+ IDE_TFLAG_OUT_HOB_LBAM = (1 7),
+ IDE_TFLAG_OUT_HOB_LBAH = (1 8),
+ IDE_TFLAG_OUT_HOB = IDE_TFLAG_OUT_HOB_FEATURE |
+ IDE_TFLAG_OUT_HOB_NSECT |
+ IDE_TFLAG_OUT_HOB_LBAL |
+ IDE_TFLAG_OUT_HOB_LBAM |
+ IDE_TFLAG_OUT_HOB_LBAH,
+ IDE_TFLAG_OUT_FEATURE = (1 9),
+ IDE_TFLAG_OUT_NSECT = (1 10),
+ IDE_TFLAG_OUT_LBAL = (1 11),
+ IDE_TFLAG_OUT_LBAM = (1 12),
+ IDE_TFLAG_OUT_LBAH = (1 13),
+ IDE_TFLAG_OUT_TF= IDE_TFLAG_OUT_FEATURE |
+ IDE_TFLAG_OUT_NSECT |
+ IDE_TFLAG_OUT_LBAL |
+ IDE_TFLAG_OUT_LBAM |
+ IDE_TFLAG_OUT_LBAH,
+ IDE_TFLAG_OUT_DEVICE= (1 14),
+ IDE_TFLAG_WRITE = (1 15),
+ IDE_TFLAG_FLAGGED_SET_IN_FLAGS = (1 16),
+ IDE_TFLAG_IN_DATA = (1 17),
+ IDE_TFLAG_CUSTOM_HANDLER= (1 18),
+ IDE_TFLAG_DMA_PIO_FALLBACK = (1 19),
+
Re: Current git -- kaboom [bisect] seems IDE related.
* Bartlomiej Zolnierkiewicz | 2008-02-09 21:28:39 [+0100]: It could be that due to block layer changes to I/O barrier handling allocating ATA command structure on the stack is no longer safe. Please try booting with hdx=noflush kernel parameter or please try the attached patch which should fix the issue (if my theory is correct). That was quick. hdx=noflush worked well. The patch delayed the disk access as you can see in [1]. After the IPI message the hd led enlightened and was active until lost interrupt message. The following boot process was verry slow. Basically everything what required disk access took a while and the hd-led was almost continuous on. I aborted the boot process via magic keys after about 15 minutes. [1] http://download.breakpoint.cc/lnx-2.6.24-post-long-boot.jpg 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
Re: Current git -- kaboom [bisect] seems IDE related.
On Saturday 09 February 2008, Sebastian Siewior wrote:
* Bartlomiej Zolnierkiewicz | 2008-02-09 21:28:39 [+0100]:
It could be that due to block layer changes to I/O barrier handling
allocating ATA command structure on the stack is no longer safe.
Update: it was never a very brilliant idea...
Please try booting with hdx=noflush kernel parameter or please try
the attached patch which should fix the issue (if my theory is correct).
That was quick.
hdx=noflush worked well. The patch delayed the disk access as you can
see in [1]. After the IPI message the hd led enlightened and was
active until lost interrupt message. The following boot process was
verry slow. Basically everything what required disk access took a while
and the hd-led was almost continuous on. I aborted the boot process via
magic keys after about 15 minutes.
[1] http://download.breakpoint.cc/lnx-2.6.24-post-long-boot.jpg
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. ]
---
drivers/ide/ide-disk.c | 12 ++--
include/linux/blkdev.h |5 +
2 files changed, 11 insertions(+), 6 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));
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, 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, 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): debian:~/xfs-cmds/xfstests# sh check [ 95.710144] SGI XFS with ACLs, security attributes, realtime, large block numbers, no debug enabled [ 95.742472] SGI XFS Quota Management subsystem [ 95.762573] BUG: unable to handle kernel NULL pointer dereference at 000d [ 95.763889] IP: [c02e0132] idedisk_prepare_flush+0x4a/0x75 [ 95.763889] *pdpt = 2d87c001 *pde = [ 95.763889] Oops: 0002 [#1] SMP [ 95.763889] Modules linked in: xfs binfmt_misc dm_snapshot dm_mirror i2c_piix4 i2c_core [ 95.763889] [ 95.763889] Pid: 2331, comm: mount Not tainted (2.6.24-09479-g531021f-dirty #43) [ 95.763889] EIP: 0060:[c02e0132] EFLAGS: 00010093 CPU: 0 [ 95.763889] EIP is at idedisk_prepare_flush+0x4a/0x75 [ 95.763889] EAX: EBX: eec92458 ECX: c083e9f4 EDX: eec92458 [ 95.763889] ESI: eec92000 EDI: c025ab93 EBP: ed897bec ESP: ed897be8 [ 95.763889] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 [ 95.763889] Process mount (pid: 2331, ti=ed896000 task=ed8a2000 task.ti=ed896000) [ 95.763889] Stack: ed897c00 c025ab80 eec92000 eec9237c ed897c20 ed897c14 c025ad60 [ 95.763889]c083e9f4 eec92000 eec92000 ed897c30 c025783e c0107fb9 ed83d0e0 c083e9f4 [ 95.763889]c083e9f4 eec92000 ed897c90 c02d7467 eecce2a0 c083e700 ed8a26f0 [ 95.763889] Call Trace: [ 95.763889] [c025ab80] ? queue_flush+0x5c/0x6f [ 95.763889] [c025ad60] ? blk_do_ordered+0xfb/0x223 [ 95.763889] [c025783e] ? elv_next_request+0x17b/0x1ac [ 95.763889] [c0107fb9] ? native_sched_clock+0x9a/0xaf [ 95.763889] [c02d7467] ? ide_do_request+0x243/0x829 [ 95.763889] [c012a29c] ? del_timer+0x4c/0x53 [ 95.763889] [c03c546b] ? _spin_unlock_irqrestore+0x2f/0x3c [ 95.763889] [c012a29c] ? del_timer+0x4c/0x53 [ 95.763889] [c02d7d22] ? do_ide_request+0x17/0x19 [ 95.763889] [c025793c] ? elv_insert+0xcd/0x1a3 [ 95.763889] [c0258282] ? drive_stat_acct+0x12c/0x140 [ 95.763889] [c0257a98] ? __elv_add_request+0x86/0x8b [ 95.763889] [c0259f7c] ? __make_request+0x2d9/0x31a [ 95.763889] [c0104d3f] ? dump_trace+0xcc/0xd8 [ 95.763889] [c010a46e] ? save_stack_address+0x0/0x28 [ 95.763889] [c0258bda] ? generic_make_request+0x360/0x38e [ 95.763889] [c010a4e8] ? save_stack_trace+0x1d/0x3b [ 95.763889] [c015200e] ? mempool_alloc_slab+0xe/0x10 [ 95.763889] [c01520e6] ? mempool_alloc+0x35/0xd0 [ 95.763889] [c0259c9b] ? submit_bio+0xc6/0xce [ 95.763889] [c018ef05] ? bio_add_page+0x2a/0x31 [ 95.763889] [f0904841] ? _xfs_buf_ioapply+0x1e6/0x20a [xfs] [ 95.763889] [f09048a0] ? xfs_buf_iorequest+0x3b/0x5e [xfs] [ 95.763889] [f0907cf1] ? xfsbdstrat+0x1e/0x2c [xfs] [ 95.763889] [f090954e] ? xfs_barrier_test+0x30/0x6a [xfs] [ 95.763889] [f09095d8] ? xfs_mountfs_check_barriers+0x50/0x7a [xfs] [ 95.763889] [f08fbfee] ? xfs_mount+0x249/0x2fb [xfs] [ 95.763889] [f090a456] ? xfs_fs_fill_super+0xb2/0x1c8 [xfs] [ 95.763889] [c01718bd] ? get_sb_bdev+0xe2/0x120 [ 95.763889] [c016c613] ? __kmalloc+0x38/0xae [ 95.763889] [f0908dc2] ? xfs_fs_get_sb+0x13/0x15 [xfs] [ 95.763889] [f090a3a4] ? xfs_fs_fill_super+0x0/0x1c8 [xfs] [ 95.763889] [c01714cb] ? vfs_kern_mount+0x3b/0x76 [ 95.763889] [c017154a] ? do_kern_mount+0x32/0xba [ 95.763889] [c0183735] ? do_new_mount+0x46/0x71 [ 95.763889] [c01838c5] ? do_mount+0x165/0x183 [ 95.763889] [c0154755] ? __get_free_pages+0x45/0x4c [ 95.763889] [c0181bd3] ? copy_mount_options+0x27/0x10b [ 95.763889] [c018394b] ? sys_mount+0x68/0xa4 [ 95.763889] [c0103cae] ? sysenter_past_esp+0x5f/0xa5 [ 95.763889] === [ 95.763889] Code: 24 00 00 3d 00 24 00 00 75 1e 83 b9 8c 00 00 00 00 77 0c 81 b9 88 00 00 00 ff ff ff 0f 76 09 c6 05 0d 00 00 00 ea eb 07 8b 45 fc c6 40 0d e7 8b 45 fc c7 40 10 00 7e 00 00 8b 45 fc c7 40 14 00 [ 95.763889] EIP: [c02e0132] idedisk_prepare_flush+0x4a/0x75 SS:ESP 0068:ed897be8 [ 95.763889] ---[ end trace 3d3002e6db2539b3 ]--- - 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
