Re: [PATCH] blk-mq-debugfs: Also show requests that have not yet been started

[Bart Van Assche <bvanassche@acm.org>]

On Wed, 2018-10-03 at 16:12 -0600, Jens Axboe wrote:
> On 10/3/18 3:42 PM, Bart Van Assche wrote:
> > On Fri, 2018-01-12 at 22:11 +0000, Bart Van Assche wrote:
> > >  /*
> > > + * Show "busy" requests - these are the requests owned by the block driver.
> > > + * The test list_empty(&rq->queuelist) is used to figure out whether or not
> > > + * a request is owned by the block driver. That test works because the block
> > > + * layer core uses list_del_init() consistently to remove a request from one
> > > + * of the request lists.
> > > + *
> > >   * Note: the state of a request may change while this function is in progress,
> > >   * e.g. due to a concurrent blk_mq_finish_request() call.
> > >   */
> > > @@ -402,7 +408,7 @@ static void hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
> > >  	const struct show_busy_params *params = data;
> > >  
> > >  	if (blk_mq_map_queue(rq->q, rq->mq_ctx->cpu) == params->hctx &&
> > > -	    blk_mq_rq_state(rq) != MQ_RQ_IDLE)
> > > +	    list_empty(&rq->queuelist))
> > >  		__blk_mq_debugfs_rq_show(params->m,
> > >  					 list_entry_rq(&rq->queuelist));
> > >  }
> > 
> > Hello Jens,
> > 
> > Can you share your opinion about the above patch?
> 
> I just convince myself that the list check is super useful. The request
> could be on any number of lists, either not yet seen by the driver, or
> maybe sitting in dispatch. You're only going to be finding these
> requests if the tag is allocated, which means that it's already in some
> sort of non-idle state.
> 
> So enlighten me why we need the list check at all? Wouldn't it be better
> to simply remove it, and ensure that the debug print includes things
> like list state, rq state, etc?

Hello Jens,

I have tried to leave the list_empty() check out but if I do that then
requests that have the state "idle" (allocated but not yet started) also
show up. I think these should be left out from the output produced by
reading the "busy" attribute because such requests are not interesting
when analyzing an I/O lockup:

nullb0/hctx1/busy:00000000abe67123 {.op=READ, .cmd_flags=, .rq_flags=IO_STAT|STATS, .s
tate=idle, .tag=63, .internal_tag=-1}

Thanks,

Bart.