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 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?

> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index 3ed71b8da887..31086afaad9c 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -97,7 +97,7 @@ static void wb_timestamp(struct rq_wb *rwb, unsigned long *var)
>   */
>  static bool wb_recent_wait(struct rq_wb *rwb)
>  {
> -	struct bdi_writeback *wb = &rwb->rqos.q->backing_dev_info->wb;
> +	struct bdi_writeback *wb = &queue_to_disk(rwb->rqos.q)->bdi->wb;
>  
>  	return time_before(jiffies, wb->dirty_sleep + HZ);
>  }
> @@ -234,7 +234,7 @@ enum {
>  
>  static int latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
>  {
> -	struct backing_dev_info *bdi = rwb->rqos.q->backing_dev_info;
> +	struct backing_dev_info *bdi = queue_to_disk(rwb->rqos.q)->bdi;
>  	struct rq_depth *rqd = &rwb->rq_depth;
>  	u64 thislat;
>  
> @@ -287,7 +287,7 @@ static int latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
>  
>  static void rwb_trace_step(struct rq_wb *rwb, const char *msg)
>  {
> -	struct backing_dev_info *bdi = rwb->rqos.q->backing_dev_info;
> +	struct backing_dev_info *bdi = queue_to_disk(rwb->rqos.q)->bdi;
>  	struct rq_depth *rqd = &rwb->rq_depth;
>  
>  	trace_wbt_step(bdi, msg, rqd->scale_step, rwb->cur_win_nsec,
> @@ -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?

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