Re: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk

On Mon 09-08-21 11:57:42, Jens Axboe wrote:

On 8/9/21 9:47 AM, Jan Kara wrote:

quoted

On Mon 09-08-21 16:17:43, Christoph Hellwig wrote:

quoted

The backing device information only makes sense for file system I/O,
and thus belongs into the gendisk and not the lower level request_queue
structure.  Move it there.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks mostly good. I'm just unsure whether some queue_to_disk() calls are
safe.

quoted

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c4ac51e54eb..d2725f94491d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c

@@ -525,7 +525,7 @@ void blk_mq_free_request(struct request *rq)
 		__blk_mq_dec_active_requests(hctx);
 
 	if (unlikely(laptop_mode && !blk_rq_is_passthrough(rq)))
-		laptop_io_completion(q->backing_dev_info);
+		laptop_io_completion(queue_to_disk(q)->bdi);

E.g. cannot this get called for a queue that is without a disk?

Should be fine, as it's checking for passthrough. Maybe famous last
words, but we should not be seeing regular IO before disk is setup.

quoted

quoted

@@ -359,8 +359,8 @@ static void wb_timer_fn(struct blk_stat_callback *cb)
 
 	status = latency_exceeded(rwb, cb->stat);
 
-	trace_wbt_timer(rwb->rqos.q->backing_dev_info, status, rqd->scale_step,
-			inflight);
+	trace_wbt_timer(queue_to_disk(rwb->rqos.q)->bdi, status,
+			rqd->scale_step, inflight);
 
 	/*
 	 * If we exceeded the latency target, step down. If we did not,

Or all these calls - is wbt guaranteed to only be setup for a queue with
disk?

Same for this one.

OK, fair enough then. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara [off-list ref]
SUSE Labs, CR