On Wed, 2017-04-12 at 10:23 +0200, Jan Kara wrote:
> +#ifndef CONFIG_BLK_WBT_MQ
> + if (q->mq_ops)
> + return;
> +#endif
> +#ifndef CONFIG_BLK_WBT_SQ
> + if (q->request_fn)
> + return;
> +#endif
> +
> + /*
> + * If this fails, we don't get throttling
> + */
> + wbt_init(q);
Hello Jan,
How about using positive logic to enable WBT, e.g. as follows? Wouldn't
that make the code easier to read?
if ((IS_ENABLED(CONFIG_BLK_WBT_MQ) && q->mq_ops) ||
(IS_ENABLED(CONFIG_BLK_WBT_SQ) && q->request_fn))
wbt_init(q);
> +static void deadline_registered_queue(struct request_queue *q)
> +{
> + wbt_enable_default(q);
> +}
> +
> /*
> * sysfs parts below
> */
> @@ -445,6 +451,7 @@ static struct elevator_type iosched_deadline = {
> .elevator_latter_req_fn = elv_rb_latter_request,
> .elevator_init_fn = deadline_init_queue,
> .elevator_exit_fn = deadline_exit_queue,
> + .elevator_registered_fn = deadline_registered_queue,
> },
>
> .elevator_attrs = deadline_attrs,
> [ ... ]
> @@ -91,6 +92,11 @@ static void noop_exit_queue(struct elevator_queue *e)
> kfree(nd);
> }
>
> +static void noop_registered_queue(struct request_queue *q)
> +{
> + wbt_enable_default(q);
> +}
> +
> static struct elevator_type elevator_noop = {
> .ops.sq = {
> .elevator_merge_req_fn = noop_merged_requests,
> @@ -100,6 +106,7 @@ static struct elevator_type elevator_noop = {
> .elevator_latter_req_fn = noop_latter_request,
> .elevator_init_fn = noop_init_queue,
> .elevator_exit_fn = noop_exit_queue,
> + .elevator_registered_fn = noop_registered_queue,
> },
> .elevator_name = "noop",
> .elevator_owner = THIS_MODULE,
This approach is not suited for blk-mq because with blk-mq "none" means no
scheduler and hence no struct elevator_type. Please consider not to add any
elevator_registered_fn() callbacks to noop and deadline but instead to call
wbt_enable_default() from elv_unregister_queue().
Thanks,
Bart.
