From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>,
Christoph Hellwig <hch@infradead.org>,
Sebastian Siewior <ide+bug@ml.breakpoint.cc>,
Tejun Heo <htejun@gmail.com>,
linux-ide@vger.kernel.org, Jens Axboe <jens.axboe@oracle.com>
Subject: Re: Current git --> kaboom [bisect] seems IDE related.
Date: Mon, 11 Feb 2008 19:30:01 +0300 [thread overview]
Message-ID: <47B07809.6010404@ru.mvista.com> (raw)
In-Reply-To: <200802101932.06946.bzolnier@gmail.com>
Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] ide-disk: fix flush requests (take 2)
> commit 813a0eb233ee67d7166241a8b389b6a76f2247f9
> Author: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 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 <ide-bug@ml.breakpoint.cc>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Cc: Tejun Heo <htejun@gmail.com>
> Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> 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
next prev parent reply other threads:[~2008-02-11 16:29 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-09 19:32 Current git --> kaboom [bisect] seems IDE related Sebastian Siewior
2008-02-09 20:28 ` Bartlomiej Zolnierkiewicz
2008-02-09 21:22 ` Sebastian Siewior
2008-02-09 23:06 ` Bartlomiej Zolnierkiewicz
2008-02-10 5:26 ` Christoph Hellwig
2008-02-10 13:38 ` Bartlomiej Zolnierkiewicz
2008-02-10 14:19 ` James Bottomley
2008-02-10 18:32 ` Bartlomiej Zolnierkiewicz
2008-02-10 19:51 ` Sebastian Siewior
2008-02-10 23:16 ` Bartlomiej Zolnierkiewicz
2008-02-11 16:30 ` Sergei Shtylyov [this message]
2008-02-11 19:41 ` Bartlomiej Zolnierkiewicz
2008-02-10 14:43 ` Christoph Hellwig
2008-02-10 15:07 ` Boaz Harrosh
2008-02-10 18:59 ` [PATCHSET 0/3] varlen extended and vendor-specific cdbs Boaz Harrosh
2008-02-10 19:05 ` Subject: [PATCH 1/3] Let scsi_cmnd->cmnd use request->cmd buffer Boaz Harrosh
2008-02-12 17:45 ` Christoph Hellwig
2008-02-12 18:10 ` Boaz Harrosh
2008-02-12 19:41 ` James Bottomley
2008-02-13 9:24 ` Boaz Harrosh
2008-02-10 19:09 ` [PATCH 2/3] block layer varlen-cdb Boaz Harrosh
2008-02-12 17:48 ` Christoph Hellwig
2008-02-12 17:54 ` Boaz Harrosh
2008-02-12 18:07 ` Boaz Harrosh
2008-02-10 19:12 ` [PATCH 3/3] scsi: varlen extended and vendor-specific cdbs Boaz Harrosh
2008-02-12 17:51 ` Christoph Hellwig
2008-02-12 18:17 ` Boaz Harrosh
2008-03-25 15:57 ` [PATCHSET 0/3] Is it time for " Boaz Harrosh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=47B07809.6010404@ru.mvista.com \
--to=sshtylyov@ru.mvista.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=bzolnier@gmail.com \
--cc=hch@infradead.org \
--cc=htejun@gmail.com \
--cc=ide+bug@ml.breakpoint.cc \
--cc=jens.axboe@oracle.com \
--cc=linux-ide@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.