Re: [PATCH] improvement of fastfail operation

Mike Christie
2004-03-31T05:18:39Z

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1991 bytes --]

On 30 Mar 2004, James Bottomley wrote:

> On Mon, 2004-03-29 at 07:17, Masao Fukuchi wrote:
> > By the way, how about item 1.
> > I think this is bug and we need to validate fastfail for command timeout.
> > If you agree with this, please put it into official patch.
> > (See attached patch)
>
> Well, the patch doesn't apply, but I put an equivalent in the tree.
>
> > Next, I tested the fastfail recovery using Kernel 2.6.4.
> > But the command didn't complete forever.
> > Then I installed following Mike Christie's patch and it worked fine.
> > http://marc.theaimsgroup.com/?l=linux-scsi&m=107904932710899&w=2
> > Please put it into official patch.
>
> This patch is obviously wrong on its face. It completes the request
> without regard to any leftovers.
>
> There's clearly a problem somewhere in the completion path for commands
> that run out of retries, but whatever it is, this patch only covers it
> up it doesn't solve the root problem.
>

This patch
http://marc.theaimsgroup.com/?l=linux-scsi&m=107904932710899&w=2
is correct wrt how the block layer uses hard_nr_sectors ;)

The blkdev.h comments state that hard_nr_sectors is the number
of sectors left to be completed. This is managed by the block layer
for drivers like SCSI when end_that_request_* calls
__end_that_request_first which for partial completions calls
blk_recalc_rq_sectors. This function updates hard_nr_sectors
by the completed number of bytes, so by scsi_end_request doing this:

if (end_that_request_chunk(req, uptodate, bytes)) {
int leftover = (req->hard_nr_sectors << 9) - bytes;

we end up with "bytes" getting subtracted twice and the final
bio not getting completed.

I know we went through that already. It is just to update
Masao/linux-scsi.

I had sent the attached patch to Jens (cc'd him)
that modifies how blk_recalc_rq_sectors uses hard_nr_sectors,
but I think it might break some other drivers that expect the
hard_nr_sectors to be updated as the request is completed.

Mike
~

[-- Attachment #2: fix blk_recalc_rq_sectors --]
[-- Type: TEXT/PLAIN, Size: 858 bytes --]

--- linux-2.6.5-rc3-orig/drivers/block/ll_rw_blk.c 2004-03-30 19:08:16.862221536 -0800
+++ linux-2.6.5-rc3/drivers/block/ll_rw_blk.c 2004-03-30 19:09:47.624270316 -0800
@@ -2579,16 +2579,14 @@ void blk_recalc_rq_sectors(struct reques
{
if (blk_fs_request(rq)) {
rq->hard_sector += nsect;
- rq->hard_nr_sectors -= nsect;

/*
* Move the I/O submission pointers ahead if required,
* i.e. for drivers not aware of rq->cbio.
*/
- if ((rq->nr_sectors >= rq->hard_nr_sectors) &&
- (rq->sector <= rq->hard_sector)) {
+ if (rq->sector <= rq->hard_sector) {
rq->sector = rq->hard_sector;
- rq->nr_sectors = rq->hard_nr_sectors;
+ rq->nr_sectors -= nsect;
rq->hard_cur_sectors = bio_cur_sectors(rq->bio);
rq->current_nr_sectors = rq->hard_cur_sectors;
rq->nr_cbio_segments = bio_segments(rq->bio);