| Message ID | 20180112215207.17852-1-bart.vanassche@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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. Thanks, Bart.
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).
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c index 19db3f583bf1..ccd5ef08c3f0 100644 --- a/block/blk-mq-debugfs.c +++ b/block/blk-mq-debugfs.c @@ -402,7 +402,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)); }
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"). Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> Cc: Ming Lei <ming.lei@redhat.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Hannes Reinecke <hare@suse.com> Cc: Johannes Thumshirn <jthumshirn@suse.de> Cc: Martin K. Petersen <martin.petersen@oracle.com> --- block/blk-mq-debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)