Re: [PATCH] null_blk: add 'requeue' fault attribute

[Omar Sandoval <osandov@osandov.com>]

On Wed, Feb 28, 2018 at 08:28:25AM -0700, Jens Axboe wrote:
> On 2/28/18 1:51 AM, Omar Sandoval wrote:
> > On Tue, Feb 27, 2018 at 03:34:53PM -0700, Jens Axboe wrote:
> >> Similarly to the support we have for testing/faking timeouts for
> >> null_blk, this adds support for triggering a requeue condition.
> >> Considering the issues around restart we've been seeing, this should be
> >> a useful addition to the testing arsenal to ensure that we are handling
> >> requeue conditions correctly.
> >>
> >> This works for queue mode 1 (legacy request_fn based path) and 2 (blk-mq
> >> path), as there's no good way to do requeue with a bio based driver.
> >> This is similar to the timeout path.
> >>
> >> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >>
> >> ---
> >>
> >>  null_blk.c |   55 +++++++++++++++++++++++++++++++++++++++++++------------
> >>  1 file changed, 43 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
> >> index 287a09611c0f..363536572e19 100644
> >> --- a/drivers/block/null_blk.c
> >> +++ b/drivers/block/null_blk.c
> > 
> > [snip]
> > 
> >> @@ -1422,10 +1440,12 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
> >>  
> >>  	blk_mq_start_request(bd->rq);
> >>  
> >> -	if (!should_timeout_request(bd->rq))
> >> -		return null_handle_cmd(cmd);
> >> +	if (should_requeue_request(bd->rq))
> >> +		return BLK_STS_RESOURCE;
> > 
> > Hm, this goes through the less interesting requeue path, add to the
> > dispatch list and __blk_mq_requeue_request(). blk_mq_requeue_request()
> > is the one that I wanted to test since that's the one that needs to call
> > the scheduler hook.
> 
> Until recently, it would have :-)
> 
> Both of them are interesting to test, though. Most of the core stall
> cases would have been triggered by going through the STS_RESOURCE case.
> How about we just make it exercise both? The below patch alternates
> between them when we have chosen to requeue.

Works for me. One idle thought, if we set this up to always requeue,
then it won't make any progress. Maybe we should limit the number of
times each request can be requeued so people (me) don't lock up their
test systems? Either way,

Reviewed-by: Omar Sandoval <osandov@fb.com>