[PATCH] blk-mq: aggregate ktime_get per request batch

[Christoph Hellwig]

>  	rq->timeout = 0;
>  
> -	if (blk_mq_need_time_stamp(rq))
> -		rq->start_time_ns = ktime_get_ns();
> -	else
> -		rq->start_time_ns = 0;
> +	rq->start_time_ns = now;
>  	rq->part = NULL;
>  #ifdef CONFIG_BLK_RQ_ALLOC_TIME
> -	rq->alloc_time_ns = alloc_time_ns;
> +	rq->alloc_time_ns = now;
>  #endif

Given that rq->start_time_ns and rq->alloc_time_ns are not always
the same (except for the odd flush case), do we even need both fields?

>  static inline struct request *
>  __blk_mq_alloc_requests_batch(struct blk_mq_alloc_data *data,
> -		u64 alloc_time_ns)
> +		u64 now)

Nit: this fits into the previous line now.

>  	return blk_mq_rq_ctx_init(data, blk_mq_tags_from_data(data), tag,
> -					alloc_time_ns);
> +					now);

Same here.

>  static blk_status_t __blk_mq_issue_directly(struct blk_mq_hw_ctx *hctx,
> -					    struct request *rq, bool last)
> +					    struct request *rq, bool last, u64 now)

Overly long line.  FYI, this becomes a lot more readable anyway with
the two tab indent:

static blk_status_t __blk_mq_issue_directly(struct blk_mq_hw_ctx *hctx,
		struct request *rq, bool last, u64 now)

> @@ -2521,6 +2528,7 @@ static blk_status_t __blk_mq_issue_directly(struct blk_mq_hw_ctx *hctx,
>  	 * Any other error (busy), just add it to our list as we
>  	 * previously would have done.
>  	 */
> +	rq->io_start_time_ns = now;

Looking though this more I hate all these extra assignments, and
it will change the accounting a bit from what we had.  Why can't we
pass the time to blk_mq_start_request and just keep it where it was?

> +	u64 now = 0;
> +
> +	if (blk_queue_stat(rq->q))
> +		now = ktime_get_ns();

This code pattern is duplicated a lot.  Can't we have a nice
helper and shortcut it to:

	u64 now = blk_ktime_get_ns_for_stats(rq->q);

and use the opportunity to document the logic in the helpe.

> -static blk_status_t blk_mq_request_issue_directly(struct request *rq, bool last)
> +static blk_status_t blk_mq_request_issue_directly(struct request *rq, bool last, u64 now)

Overly long line here.  But I really wonder why we even needthis helper.