Re: [PATCH block#for-2.6.31] block: fix an oops on BLKPREP_KILL

[Jens Axboe <jens.axboe@oracle.com>]

On Sat, May 30 2009, James Bottomley wrote:
> On Sat, 2009-05-30 at 06:41 +0200, Jens Axboe wrote:
> > On Fri, May 29 2009, James Bottomley wrote:
> > > Doing a bit of torture testing, I ran across a BUG in the block
> > > subsystem (at blk-core.c:2048): the test for if the request is queued.
> > > 
> > > It turns out the trigger was a BLKPREP_KILL coming out of the SCSI prep
> > > function.  Currently for BLKPREP_KILL requests, we send them straight
> > > into __blk_end_request_all() with an error, but they've never been
> > > dequeued, so they trip the bug.  Fix this by starting requests before
> > > killing them.
> > > 
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > 
> > > ---
> > > 
> > > diff --git a/block/blk-core.c b/block/blk-core.c
> > > index 8b3b74e..9a0568c 100644
> > > --- a/block/blk-core.c
> > > +++ b/block/blk-core.c
> > > @@ -1789,6 +1789,7 @@ struct request *blk_peek_request(struct request_queue *q)
> > >  			break;
> > >  		} else if (ret == BLKPREP_KILL) {
> > >  			rq->cmd_flags |= REQ_QUIET;
> > > +			blk_start_request(rq);
> > >  			__blk_end_request_all(rq, -EIO);
> > >  		} else {
> > >  			printk(KERN_ERR "%s: bad return=%d\n", __func__, ret);
> > 
> > Given how illogical that now looks, I think it could do with a comment.
> > I'll add that while applying.
> 
> Um, if it looks illogical, then so is the new everything has to be
> dequeued before completion requirement ...  I agree a comment reminding
> people why it has to work this way doesn't hurt, but if it looks
> illogical then there might be something wrong with the rules requiring
> this to happen ...

It's not a bad idea to be a little extra paranoid with such an API
change, we can always relax this restriction later.

-- 
Jens Axboe