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

[Jan Kara <jack@suse.cz>]

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@lst.de>
> > 
> > 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@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR