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 <bzolnier@xxxxxxxxx>
Subject: [PATCH] ide-disk: fix flush requests
commit 813a0eb233ee67d7166241a8b389b6a76f2247f9
Author: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
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 <ide-bug@xxxxxxxxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
Cc: Jens Axboe <jens.axboe@xxxxxxxxxx>
Cc: Tejun Heo <htejun@xxxxxxxxx>
Cc: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
---
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_CDB 48 /* max sizeof(ide_task_t) */
+#else
#define BLK_MAX_CDB 16
+#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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
