[PATCH] ide: another possible ide panic fix for blk-end-request

[PATCH] ide: another possible ide panic fix for blk-end-request

[Kiyoshi Ueda]

Hi Jens, Bart, Boris,

I have reviewed all blk-end-request patches again to confirm whether
there are any similar problems with the last week's ide-cd panic:
    http://lkml.org/lkml/2008/1/29/140

And I found a possible similar bug in ide-io change:
ide_end_drive_cmd() could be called for blk_pc_request() which could
have bios.  To complete such requests correctly, we need to pass
the actual size of the request.
Otherwise, __blk_end_request() returns 1 because the request still has
bios, and the system will BUG() unnecessarily.

The following patch fixes the bug and should be applied on top of
Linus' git.
Please review and apply.


Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: Borislav Petkov <petkovbb@googlemail.com>
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
---
 drivers/ide/ide-io.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/ide/ide-io.c	2008-02-01 18:20:02.000000000 -0500
+++ b/drivers/ide/ide-io.c	2008-02-01 18:21:50.000000000 -0500
@@ -388,7 +388,8 @@ void ide_end_drive_cmd (ide_drive_t *dri
 	spin_lock_irqsave(&ide_lock, flags);
 	HWGROUP(drive)->rq = NULL;
 	rq->errors = err;
-	if (__blk_end_request(rq, (rq->errors ? -EIO : 0), 0))
+	if (unlikely(__blk_end_request(rq, (rq->errors ? -EIO : 0),
+				       blk_rq_bytes(rq))))
 		BUG();
 	spin_unlock_irqrestore(&ide_lock, flags);
 }

Thanks,
Kiyoshi Ueda

Re: [PATCH] ide: another possible ide panic fix for blk-end-request

[Borislav Petkov]

On Mon, Feb 04, 2008 at 02:53:12PM -0500, Kiyoshi Ueda wrote:
> Hi Jens, Bart, Boris,
> 
> I have reviewed all blk-end-request patches again to confirm whether
> there are any similar problems with the last week's ide-cd panic:
>     http://lkml.org/lkml/2008/1/29/140
> 
> And I found a possible similar bug in ide-io change:
> ide_end_drive_cmd() could be called for blk_pc_request() which could
> have bios.

You mean ide_abort() and ide_error(), right? Because ide{-tape,-floppy,-scsi}
do call already ide_end_request() for non-special rq's
(!blk_special_request()), except ide-scsi filters also on !blk_sense_request().

> To complete such requests correctly, we need to pass
> the actual size of the request.
> Otherwise, __blk_end_request() returns 1 because the request still has
> bios, and the system will BUG() unnecessarily.
> 
> The following patch fixes the bug and should be applied on top of
> Linus' git.
> Please review and apply.
> 
> 
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Cc: Borislav Petkov <petkovbb@googlemail.com>
> Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> ---
>  drivers/ide/ide-io.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> --- a/drivers/ide/ide-io.c	2008-02-01 18:20:02.000000000 -0500
> +++ b/drivers/ide/ide-io.c	2008-02-01 18:21:50.000000000 -0500
> @@ -388,7 +388,8 @@ void ide_end_drive_cmd (ide_drive_t *dri
>  	spin_lock_irqsave(&ide_lock, flags);
>  	HWGROUP(drive)->rq = NULL;
>  	rq->errors = err;
> -	if (__blk_end_request(rq, (rq->errors ? -EIO : 0), 0))
> +	if (unlikely(__blk_end_request(rq, (rq->errors ? -EIO : 0),
> +				       blk_rq_bytes(rq))))
>  		BUG();
>  	spin_unlock_irqrestore(&ide_lock, flags);
>  }
> 
> Thanks,
> Kiyoshi Ueda

-- 
Regards/Gruß,
    Boris.

Re: [PATCH] ide: another possible ide panic fix for blk-end-request

[Kiyoshi Ueda]

Hi Boris,

On Tue, 5 Feb 2008 07:01:57 +0100, Borislav Petkov wrote:
> On Mon, Feb 04, 2008 at 02:53:12PM -0500, Kiyoshi Ueda wrote:
> > Hi Jens, Bart, Boris,
> > 
> > I have reviewed all blk-end-request patches again to confirm whether
> > there are any similar problems with the last week's ide-cd panic:
> >     http://lkml.org/lkml/2008/1/29/140
> > 
> > And I found a possible similar bug in ide-io change:
> > ide_end_drive_cmd() could be called for blk_pc_request() which could
> > have bios.
> 
> You mean ide_abort() and ide_error(), right?
> Because ide{-tape,-floppy,-scsi} do call already ide_end_request()
> for non-special rq's (!blk_special_request()), except ide-scsi
> filters also on !blk_sense_request().

That's right.

Thanks,
Kiyoshi Ueda

Re: [PATCH] ide: another possible ide panic fix for blk-end-request

[Bartlomiej Zolnierkiewicz]

On Monday 04 February 2008, Kiyoshi Ueda wrote:
> Hi Jens, Bart, Boris,
> 
> I have reviewed all blk-end-request patches again to confirm whether
> there are any similar problems with the last week's ide-cd panic:
>     http://lkml.org/lkml/2008/1/29/140
> 
> And I found a possible similar bug in ide-io change:
> ide_end_drive_cmd() could be called for blk_pc_request() which could
> have bios.  To complete such requests correctly, we need to pass
> the actual size of the request.
> Otherwise, __blk_end_request() returns 1 because the request still has
> bios, and the system will BUG() unnecessarily.
> 
> The following patch fixes the bug and should be applied on top of
> Linus' git.
> Please review and apply.
> 
> 
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Cc: Borislav Petkov <petkovbb@googlemail.com>
> Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>

applied, thanks