On Mon 09-08-21 11:57:42, Jens Axboe wrote: > On 8/9/21 9:47 AM, Jan Kara wrote: > > On Mon 09-08-21 16:17:43, Christoph Hellwig wrote: > >> 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@xxxxxx> > > > > Looks mostly good. I'm just unsure whether some queue_to_disk() calls are > > safe. > > > >> 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. > > >> @@ -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@xxxxxxx> Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR