2019-05-10 18:06:11

by Eric Wheeler

[permalink] [raw]
Subject: [PATCH] bfq: backport: update internal depth state when queue depth changes

From: Jens Axboe <[email protected]>

commit 77f1e0a52d26242b6c2dba019f6ebebfb9ff701e upstream

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.

Signed-off-by: Eric Wheeler <[email protected]>
Tested-by: Kai Krakow <[email protected]>
Reported-by: Paolo Valente <[email protected]>
Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
Signed-off-by: Jens Axboe <[email protected]>
Cc: [email protected]
---
block/bfq-iosched.c | 8 +++++++-
block/blk-mq.c | 2 ++
include/linux/elevator.h | 1 +
3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 653100f..0f7cdbc 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5223,7 +5223,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;
@@ -5231,6 +5231,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;
}

@@ -5653,6 +5658,7 @@ static ssize_t bfq_low_latency_store(struct elevator_queue *e,
.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 e3c39ea..7a57368 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2878,6 +2878,8 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
}
if (ret)
break;
+ if (q->elevator && q->elevator->type->ops.mq.depth_updated)
+ q->elevator->type->ops.mq.depth_updated(hctx);
}

if (!ret)
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index a02deea..a2bf4a6 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -99,6 +99,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 *);
--
1.8.3.1


2019-05-10 20:20:05

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH] bfq: backport: update internal depth state when queue depth changes

On Fri, May 10, 2019 at 10:56:32AM -0700, Eric Wheeler wrote:
>From: Jens Axboe <[email protected]>
>
>commit 77f1e0a52d26242b6c2dba019f6ebebfb9ff701e upstream
>
>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.
>
>Signed-off-by: Eric Wheeler <[email protected]>
>Tested-by: Kai Krakow <[email protected]>
>Reported-by: Paolo Valente <[email protected]>
>Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
>Signed-off-by: Jens Axboe <[email protected]>
>Cc: [email protected]

I wasn't clear on what was backported here, so I've queued the upstream
version on 4.19 and 4.14, it doesn't seem to be relevant to older
branches.

--
Thanks,
Sasha

2019-05-10 23:21:39

by Eric Wheeler

[permalink] [raw]
Subject: Re: [PATCH] bfq: backport: update internal depth state when queue depth changes

On Fri, 10 May 2019, Sasha Levin wrote:

> On Fri, May 10, 2019 at 10:56:32AM -0700, Eric Wheeler wrote:
> >From: Jens Axboe <[email protected]>
> >
> >commit 77f1e0a52d26242b6c2dba019f6ebebfb9ff701e upstream
> >
> >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.
> >
> >Signed-off-by: Eric Wheeler <[email protected]>
> >Tested-by: Kai Krakow <[email protected]>
> >Reported-by: Paolo Valente <[email protected]>
> >Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
> >Signed-off-by: Jens Axboe <[email protected]>
> >Cc: [email protected]
>
> I wasn't clear on what was backported here, so I've queued the upstream
> version on 4.19 and 4.14, it doesn't seem to be relevant to older
> branches.


Thanks Sasha. We needed it for 4.19, I wasn't sure how far it would patch
back so I left the version off. BFQ was merged in 4.12 iirc, so if it
patched against 4.14, then 4.19 and 4.14 are the only ones that need it.

--
Eric Wheeler


>
> --
> Thanks,
> Sasha
>

2019-05-11 00:07:07

by Holger Hoffstätte

[permalink] [raw]
Subject: Re: [PATCH] bfq: backport: update internal depth state when queue depth changes

On 5/11/19 1:17 AM, Eric Wheeler wrote:
> On Fri, 10 May 2019, Sasha Levin wrote:
>
>> On Fri, May 10, 2019 at 10:56:32AM -0700, Eric Wheeler wrote:
>>> From: Jens Axboe <[email protected]>
>>>
>>> commit 77f1e0a52d26242b6c2dba019f6ebebfb9ff701e upstream
>>>
>>> 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.
>>>
>>> Signed-off-by: Eric Wheeler <[email protected]>
>>> Tested-by: Kai Krakow <[email protected]>
>>> Reported-by: Paolo Valente <[email protected]>
>>> Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
>>> Signed-off-by: Jens Axboe <[email protected]>
>>> Cc: [email protected]
>>
>> I wasn't clear on what was backported here, so I've queued the upstream
>> version on 4.19 and 4.14, it doesn't seem to be relevant to older
>> branches.
>
>
> Thanks Sasha. We needed it for 4.19, I wasn't sure how far it would patch
> back so I left the version off. BFQ was merged in 4.12 iirc, so if it
> patched against 4.14, then 4.19 and 4.14 are the only ones that need it.

It was applied in mainline for 5.1 and also applies fine to 5.0.x, so IMHO
that wouldn't hurt either.

thanks,
Holger

2019-05-13 07:07:30

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] bfq: backport: update internal depth state when queue depth changes

On Fri, May 10, 2019 at 04:18:55PM -0400, Sasha Levin wrote:
> On Fri, May 10, 2019 at 10:56:32AM -0700, Eric Wheeler wrote:
> > From: Jens Axboe <[email protected]>
> >
> > commit 77f1e0a52d26242b6c2dba019f6ebebfb9ff701e upstream
> >
> > 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.
> >
> > Signed-off-by: Eric Wheeler <[email protected]>
> > Tested-by: Kai Krakow <[email protected]>
> > Reported-by: Paolo Valente <[email protected]>
> > Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
> > Signed-off-by: Jens Axboe <[email protected]>
> > Cc: [email protected]
>
> I wasn't clear on what was backported here, so I've queued the upstream
> version on 4.19 and 4.14, it doesn't seem to be relevant to older
> branches.

I only see this added to the 5.0 and 4.19 queues, did you forget to push
the 4.14 update?

thanks,

greg k-h

2019-05-13 15:42:15

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH] bfq: backport: update internal depth state when queue depth changes

On Mon, May 13, 2019 at 09:03:37AM +0200, Greg KH wrote:
>On Fri, May 10, 2019 at 04:18:55PM -0400, Sasha Levin wrote:
>> On Fri, May 10, 2019 at 10:56:32AM -0700, Eric Wheeler wrote:
>> > From: Jens Axboe <[email protected]>
>> >
>> > commit 77f1e0a52d26242b6c2dba019f6ebebfb9ff701e upstream
>> >
>> > 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.
>> >
>> > Signed-off-by: Eric Wheeler <[email protected]>
>> > Tested-by: Kai Krakow <[email protected]>
>> > Reported-by: Paolo Valente <[email protected]>
>> > Fixes: f0635b8a416e ("bfq: calculate shallow depths at init time")
>> > Signed-off-by: Jens Axboe <[email protected]>
>> > Cc: [email protected]
>>
>> I wasn't clear on what was backported here, so I've queued the upstream
>> version on 4.19 and 4.14, it doesn't seem to be relevant to older
>> branches.
>
>I only see this added to the 5.0 and 4.19 queues, did you forget to push
>the 4.14 update?

Sorry, I meant to say that I applied it to 5.0 and 4.19. The patch it
fixes, f0635b8a416e ("bfq: calculate shallow depths at init time"),
isn't in 4.14 nor was it backported there by us.

--
Thanks,
Sasha