linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* bfq: update internal depth state when queue depth changes
@ 2019-01-18 17:34 Jens Axboe
  2019-01-19 11:13 ` Kai Krakow
  0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2019-01-18 17:34 UTC (permalink / raw)
  To: linux-block; +Cc: Paolo Valente, Oleksandr Natalenko, hurikhan77+bko

A previous commit moved the shallow depth and BFQ depth map calculations
to be done at init time, moving it outside of the hotter IO path. This
potentially causes hangs if the users changes the depth of the scheduler
map, by writing to the 'nr_requests' sysfs file for that device.

Add a blk-mq-sched hook that allows blk-mq to inform the scheduler if
the depth changes, so that the scheduler can update its internal state.

Reported-by: Paolo Valente <paolo.valente@linaro.org>
Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index cd307767a134..b09589915667 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5342,7 +5342,7 @@ static unsigned int bfq_update_depths(struct bfq_data *bfqd,
 	return min_shallow;
 }
 
-static int bfq_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int index)
+static void bfq_depth_updated(struct blk_mq_hw_ctx *hctx)
 {
 	struct bfq_data *bfqd = hctx->queue->elevator->elevator_data;
 	struct blk_mq_tags *tags = hctx->sched_tags;
@@ -5350,6 +5350,11 @@ static int bfq_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int index)
 
 	min_shallow = bfq_update_depths(bfqd, &tags->bitmap_tags);
 	sbitmap_queue_min_shallow_depth(&tags->bitmap_tags, min_shallow);
+}
+
+static int bfq_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int index)
+{
+	bfq_depth_updated(hctx);
 	return 0;
 }
 
@@ -5772,6 +5777,7 @@ static struct elevator_type iosched_bfq_mq = {
 		.requests_merged	= bfq_requests_merged,
 		.request_merged		= bfq_request_merged,
 		.has_work		= bfq_has_work,
+		.depth_updated		= bfq_depth_updated,
 		.init_hctx		= bfq_init_hctx,
 		.init_sched		= bfq_init_queue,
 		.exit_sched		= bfq_exit_queue,
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 3ba37b9e15e9..a047b297ade5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3101,6 +3101,8 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
 		}
 		if (ret)
 			break;
+		if (q->elevator && q->elevator->type->ops.depth_updated)
+			q->elevator->type->ops.depth_updated(hctx);
 	}
 
 	if (!ret)
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 2e9e2763bf47..6e8bc53740f0 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -31,6 +31,7 @@ struct elevator_mq_ops {
 	void (*exit_sched)(struct elevator_queue *);
 	int (*init_hctx)(struct blk_mq_hw_ctx *, unsigned int);
 	void (*exit_hctx)(struct blk_mq_hw_ctx *, unsigned int);
+	void (*depth_updated)(struct blk_mq_hw_ctx *);
 
 	bool (*allow_merge)(struct request_queue *, struct request *, struct bio *);
 	bool (*bio_merge)(struct blk_mq_hw_ctx *, struct bio *);

-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: bfq: update internal depth state when queue depth changes
  2019-01-18 17:34 bfq: update internal depth state when queue depth changes Jens Axboe
@ 2019-01-19 11:13 ` Kai Krakow
  2019-01-19 14:36   ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Kai Krakow @ 2019-01-19 11:13 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Paolo Valente, Oleksandr Natalenko, hurikhan77+bko

Resent due to Gmail tricking me into using HTML format. :-(

Am Fr., 18. Jan. 2019 um 18:34 Uhr schrieb Jens Axboe <axboe@kernel.dk>:
>
> A previous commit moved the shallow depth and BFQ depth map calculations
> to be done at init time, moving it outside of the hotter IO path. This
> potentially causes hangs if the users changes the depth of the scheduler
> map, by writing to the 'nr_requests' sysfs file for that device.
>
> Add a blk-mq-sched hook that allows blk-mq to inform the scheduler if
> the depth changes, so that the scheduler can update its internal state.
>
> Reported-by: Paolo Valente <paolo.valente@linaro.org>
> Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
> Signed-off-by: Jens Axboe <axboe@kernel.dk>

Works fine here in 4.18 as far as I could test (running a full backup,
some compilations and Gentoo package upgrades, filling memory with web
browsers to force swapping).

$ grep ^ /sys/block/*/queue/nr_requests
/sys/block/bcache0/queue/nr_requests:128
/sys/block/bcache1/queue/nr_requests:128
/sys/block/bcache2/queue/nr_requests:128
/sys/block/bcache3/queue/nr_requests:128
/sys/block/bcache4/queue/nr_requests:128
/sys/block/sda/queue/nr_requests:256
/sys/block/sdb/queue/nr_requests:256
/sys/block/sdc/queue/nr_requests:256
/sys/block/sdd/queue/nr_requests:256
/sys/block/sde/queue/nr_requests:256
/sys/block/sdf/queue/nr_requests:256

Tested-by: Kai Krakow <kai@kaishome.de>

> ---
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index cd307767a134..b09589915667 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -5342,7 +5342,7 @@ static unsigned int bfq_update_depths(struct bfq_data *bfqd,
>         return min_shallow;
>  }
>
> -static int bfq_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int index)
> +static void bfq_depth_updated(struct blk_mq_hw_ctx *hctx)
>  {
>         struct bfq_data *bfqd = hctx->queue->elevator->elevator_data;
>         struct blk_mq_tags *tags = hctx->sched_tags;
> @@ -5350,6 +5350,11 @@ static int bfq_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int index)
>
>         min_shallow = bfq_update_depths(bfqd, &tags->bitmap_tags);
>         sbitmap_queue_min_shallow_depth(&tags->bitmap_tags, min_shallow);
> +}
> +
> +static int bfq_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int index)
> +{
> +       bfq_depth_updated(hctx);
>         return 0;
>  }
>
> @@ -5772,6 +5777,7 @@ static struct elevator_type iosched_bfq_mq = {
>                 .requests_merged        = bfq_requests_merged,
>                 .request_merged         = bfq_request_merged,
>                 .has_work               = bfq_has_work,
> +               .depth_updated          = bfq_depth_updated,
>                 .init_hctx              = bfq_init_hctx,
>                 .init_sched             = bfq_init_queue,
>                 .exit_sched             = bfq_exit_queue,
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 3ba37b9e15e9..a047b297ade5 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -3101,6 +3101,8 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
>                 }
>                 if (ret)
>                         break;
> +               if (q->elevator && q->elevator->type->ops.depth_updated)
> +                       q->elevator->type->ops.depth_updated(hctx);
>         }
>
>         if (!ret)
> diff --git a/include/linux/elevator.h b/include/linux/elevator.h
> index 2e9e2763bf47..6e8bc53740f0 100644
> --- a/include/linux/elevator.h
> +++ b/include/linux/elevator.h
> @@ -31,6 +31,7 @@ struct elevator_mq_ops {
>         void (*exit_sched)(struct elevator_queue *);
>         int (*init_hctx)(struct blk_mq_hw_ctx *, unsigned int);
>         void (*exit_hctx)(struct blk_mq_hw_ctx *, unsigned int);
> +       void (*depth_updated)(struct blk_mq_hw_ctx *);
>
>         bool (*allow_merge)(struct request_queue *, struct request *, struct bio *);
>         bool (*bio_merge)(struct blk_mq_hw_ctx *, struct bio *);
>
> --
> Jens Axboe
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: bfq: update internal depth state when queue depth changes
  2019-01-19 11:13 ` Kai Krakow
@ 2019-01-19 14:36   ` Jens Axboe
  2019-04-13 15:18     ` Paolo Valente
  0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2019-01-19 14:36 UTC (permalink / raw)
  To: Kai Krakow
  Cc: linux-block, Paolo Valente, Oleksandr Natalenko, hurikhan77+bko

On 1/19/19 4:13 AM, Kai Krakow wrote:
> Am Fr., 18. Jan. 2019 um 18:34 Uhr schrieb Jens Axboe <axboe@kernel.dk>:
>>
>> A previous commit moved the shallow depth and BFQ depth map calculations
>> to be done at init time, moving it outside of the hotter IO path. This
>> potentially causes hangs if the users changes the depth of the scheduler
>> map, by writing to the 'nr_requests' sysfs file for that device.
>>
>> Add a blk-mq-sched hook that allows blk-mq to inform the scheduler if
>> the depth changes, so that the scheduler can update its internal state.
>>
>> Reported-by: Paolo Valente <paolo.valente@linaro.org>
>> Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> Works fine here in 4.18 as far as I could test (running a full backup,
> some compilations and Gentoo package upgrades, filling memory with web
> browsers to force swapping).
> 
> $ grep ^ /sys/block/*/queue/nr_requests
> /sys/block/bcache0/queue/nr_requests:128
> /sys/block/bcache1/queue/nr_requests:128
> /sys/block/bcache2/queue/nr_requests:128
> /sys/block/bcache3/queue/nr_requests:128
> /sys/block/bcache4/queue/nr_requests:128
> /sys/block/sda/queue/nr_requests:256
> /sys/block/sdb/queue/nr_requests:256
> /sys/block/sdc/queue/nr_requests:256
> /sys/block/sdd/queue/nr_requests:256
> /sys/block/sde/queue/nr_requests:256
> /sys/block/sdf/queue/nr_requests:256
> 
> Tested-by: Kai Krakow <kai@kaishome.de>

Great, thanks for testing!

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: bfq: update internal depth state when queue depth changes
  2019-01-19 14:36   ` Jens Axboe
@ 2019-04-13 15:18     ` Paolo Valente
  2019-04-13 15:34       ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Valente @ 2019-04-13 15:18 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Kai Krakow, linux-block, Oleksandr Natalenko, hurikhan77+bko

Hi Jens,
I'm not finding this fix in either current rc or for-5.2.  Am I missing
something?

Thanks,
Paolo

> Il giorno 19 gen 2019, alle ore 15:36, Jens Axboe <axboe@kernel.dk> ha scritto:
> 
> On 1/19/19 4:13 AM, Kai Krakow wrote:
>> Am Fr., 18. Jan. 2019 um 18:34 Uhr schrieb Jens Axboe <axboe@kernel.dk>:
>>> 
>>> A previous commit moved the shallow depth and BFQ depth map calculations
>>> to be done at init time, moving it outside of the hotter IO path. This
>>> potentially causes hangs if the users changes the depth of the scheduler
>>> map, by writing to the 'nr_requests' sysfs file for that device.
>>> 
>>> Add a blk-mq-sched hook that allows blk-mq to inform the scheduler if
>>> the depth changes, so that the scheduler can update its internal state.
>>> 
>>> Reported-by: Paolo Valente <paolo.valente@linaro.org>
>>> Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> 
>> Works fine here in 4.18 as far as I could test (running a full backup,
>> some compilations and Gentoo package upgrades, filling memory with web
>> browsers to force swapping).
>> 
>> $ grep ^ /sys/block/*/queue/nr_requests
>> /sys/block/bcache0/queue/nr_requests:128
>> /sys/block/bcache1/queue/nr_requests:128
>> /sys/block/bcache2/queue/nr_requests:128
>> /sys/block/bcache3/queue/nr_requests:128
>> /sys/block/bcache4/queue/nr_requests:128
>> /sys/block/sda/queue/nr_requests:256
>> /sys/block/sdb/queue/nr_requests:256
>> /sys/block/sdc/queue/nr_requests:256
>> /sys/block/sdd/queue/nr_requests:256
>> /sys/block/sde/queue/nr_requests:256
>> /sys/block/sdf/queue/nr_requests:256
>> 
>> Tested-by: Kai Krakow <kai@kaishome.de>
> 
> Great, thanks for testing!
> 
> -- 
> Jens Axboe
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: bfq: update internal depth state when queue depth changes
  2019-04-13 15:18     ` Paolo Valente
@ 2019-04-13 15:34       ` Jens Axboe
  0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2019-04-13 15:34 UTC (permalink / raw)
  To: Paolo Valente
  Cc: Kai Krakow, linux-block, Oleksandr Natalenko, hurikhan77+bko

On 4/13/19 9:18 AM, Paolo Valente wrote:
> Hi Jens,
> I'm not finding this fix in either current rc or for-5.2.  Am I missing
> something?

Looks like that slipped through the cracks, I have applied it for 5.1
just now.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-04-13 15:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-18 17:34 bfq: update internal depth state when queue depth changes Jens Axboe
2019-01-19 11:13 ` Kai Krakow
2019-01-19 14:36   ` Jens Axboe
2019-04-13 15:18     ` Paolo Valente
2019-04-13 15:34       ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).