On Fri, 2018-01-12 at 15:05 -0700, Jens Axboe wrote:
> On 1/12/18 3:00 PM, Bart Van Assche wrote:
> > On Fri, 2018-01-12 at 14:55 -0700, Jens Axboe wrote:
> > > On 1/12/18 2:52 PM, Bart Van Assche wrote:
> > > > When debugging e.g. the SCSI timeout handler it is important that
> > > > requests that have not yet been started or that already have
> > > > completed are also reported through debugfs.
> > > >
> > > > This patch depends on a patch that went upstream recently, namely
> > > > commit 14e3062fb185 ("scsi: core: Fix a scsi_show_rq() NULL pointer
> > > > dereference").
> > >
> > > Why don't we just kill the check, and dump any request that has a
> > > matching hctx? We already know the bit was set, so just print
> > > all of them.
> >
> > It is very helpful during debugging that requests owned by a block driver and
> > requests owned by the block layer core show up in different debugfs files.
> > Removing the check completely would cause all requests to show up in the same
> > debugfs file and would make interpreting the contents of these debugfs files
> > much harder.
>
> Yeah, we'd have to make it just one file at that point. I'm not hugely
> against the queuelist check, but probably warrants a comment as it's not
> immediately clear (as opposed to the idle check, or the previous START
> bit check).
How about the below?
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 19db3f583bf1..da859dac442b 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -394,6 +394,12 @@ struct show_busy_params {
};
/*
+ * 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));
}
Thanks,
Bart.
