* [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode
@ 2017-03-08 12:16 Hou Tao
2017-03-08 14:05 ` Jan Kara
2017-03-08 17:56 ` Jens Axboe
0 siblings, 2 replies; 6+ messages in thread
From: Hou Tao @ 2017-03-08 12:16 UTC (permalink / raw)
To: axboe, linux-block; +Cc: jack, jmoyer, stable, vgoyal
When adding a cfq_group into the cfq service tree, we use CFQ_IDLE_DELAY
as the delay of cfq_group's vdisktime if there have been other cfq_groups
already.
When cfq is under iops mode, commit 9a7f38c42c2b ("cfq-iosched: Convert
from jiffies to nanoseconds") could result in a large iops delay and
lead to an abnormal io schedule delay for the added cfq_group. To fix
it, we just need to revert to the old CFQ_IDLE_DELAY value: HZ / 5
when iops mode is enabled.
Despite having the same value, the delay of a cfq_queue in idle class
and the delay of cfq_queue are different things, so I define two new
macros for the delay of a cfq_group under time-slice mode and IOPs mode.
Fixes: 9a7f38c42c2b92391d9dabaf9f51df7cfe5608e4
Cc: <stable@vger.kernel.org> # 4.8+
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
block/cfq-iosched.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
v2:
- use constant instead of "nsecs_to_jiffies64(CFQ_IDLE_DELAY)"
as suggested by Jan Kara
v1:
https://www.spinics.net/lists/stable/msg160580.html
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 440b95e..69754e8 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -38,9 +38,13 @@ static const u64 cfq_target_latency = (u64)NSEC_PER_SEC * 3/10; /* 300 ms */
static const int cfq_hist_divisor = 4;
/*
- * offset from end of service tree
+ * offset from end of queue service tree for idle class
*/
#define CFQ_IDLE_DELAY (NSEC_PER_SEC / 5)
+/* offset from end of group service tree under time slice mode */
+#define CFQ_SLICE_MODE_GROUP_DELAY (NSEC_PER_SEC / 5)
+/* offset from end of group service under IOPS mode */
+#define CFQ_IOPS_MODE_GROUP_DELAY (200)
/*
* below this threshold, we consider thinktime immediate
@@ -1362,6 +1366,14 @@ cfq_group_service_tree_add(struct cfq_rb_root *st, struct cfq_group *cfqg)
cfqg->vfraction = max_t(unsigned, vfr, 1);
}
+static inline u64 cfq_get_cfqg_vdisktime_delay(struct cfq_data *cfqd)
+{
+ if (!iops_mode(cfqd))
+ return CFQ_SLICE_MODE_GROUP_DELAY;
+ else
+ return CFQ_IOPS_MODE_GROUP_DELAY;
+}
+
static void
cfq_group_notify_queue_add(struct cfq_data *cfqd, struct cfq_group *cfqg)
{
@@ -1381,7 +1393,8 @@ cfq_group_notify_queue_add(struct cfq_data *cfqd, struct cfq_group *cfqg)
n = rb_last(&st->rb);
if (n) {
__cfqg = rb_entry_cfqg(n);
- cfqg->vdisktime = __cfqg->vdisktime + CFQ_IDLE_DELAY;
+ cfqg->vdisktime = __cfqg->vdisktime +
+ cfq_get_cfqg_vdisktime_delay(cfqd);
} else
cfqg->vdisktime = st->min_vdisktime;
cfq_group_service_tree_add(st, cfqg);
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode
2017-03-08 12:16 [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode Hou Tao
@ 2017-03-08 14:05 ` Jan Kara
2017-03-09 11:22 ` Hou Tao
2017-03-08 17:56 ` Jens Axboe
1 sibling, 1 reply; 6+ messages in thread
From: Jan Kara @ 2017-03-08 14:05 UTC (permalink / raw)
To: Hou Tao; +Cc: axboe, linux-block, jack, jmoyer, stable, vgoyal
On Wed 08-03-17 20:16:55, Hou Tao wrote:
> When adding a cfq_group into the cfq service tree, we use CFQ_IDLE_DELAY
> as the delay of cfq_group's vdisktime if there have been other cfq_groups
> already.
>
> When cfq is under iops mode, commit 9a7f38c42c2b ("cfq-iosched: Convert
> from jiffies to nanoseconds") could result in a large iops delay and
> lead to an abnormal io schedule delay for the added cfq_group. To fix
> it, we just need to revert to the old CFQ_IDLE_DELAY value: HZ / 5
> when iops mode is enabled.
>
> Despite having the same value, the delay of a cfq_queue in idle class
> and the delay of cfq_queue are different things, so I define two new
> macros for the delay of a cfq_group under time-slice mode and IOPs mode.
>
> Fixes: 9a7f38c42c2b92391d9dabaf9f51df7cfe5608e4
> Cc: <stable@vger.kernel.org> # 4.8+
> Signed-off-by: Hou Tao <houtao1@huawei.com>
OK, the number 200 is somewhat arbitrary but so is HZ/5 so I guess we are
OK. You can add:
Acked-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/cfq-iosched.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> v2:
> - use constant instead of "nsecs_to_jiffies64(CFQ_IDLE_DELAY)"
> as suggested by Jan Kara
>
> v1:
> https://www.spinics.net/lists/stable/msg160580.html
>
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 440b95e..69754e8 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -38,9 +38,13 @@ static const u64 cfq_target_latency = (u64)NSEC_PER_SEC * 3/10; /* 300 ms */
> static const int cfq_hist_divisor = 4;
>
> /*
> - * offset from end of service tree
> + * offset from end of queue service tree for idle class
> */
> #define CFQ_IDLE_DELAY (NSEC_PER_SEC / 5)
> +/* offset from end of group service tree under time slice mode */
> +#define CFQ_SLICE_MODE_GROUP_DELAY (NSEC_PER_SEC / 5)
> +/* offset from end of group service under IOPS mode */
> +#define CFQ_IOPS_MODE_GROUP_DELAY (200)
>
> /*
> * below this threshold, we consider thinktime immediate
> @@ -1362,6 +1366,14 @@ cfq_group_service_tree_add(struct cfq_rb_root *st, struct cfq_group *cfqg)
> cfqg->vfraction = max_t(unsigned, vfr, 1);
> }
>
> +static inline u64 cfq_get_cfqg_vdisktime_delay(struct cfq_data *cfqd)
> +{
> + if (!iops_mode(cfqd))
> + return CFQ_SLICE_MODE_GROUP_DELAY;
> + else
> + return CFQ_IOPS_MODE_GROUP_DELAY;
> +}
> +
> static void
> cfq_group_notify_queue_add(struct cfq_data *cfqd, struct cfq_group *cfqg)
> {
> @@ -1381,7 +1393,8 @@ cfq_group_notify_queue_add(struct cfq_data *cfqd, struct cfq_group *cfqg)
> n = rb_last(&st->rb);
> if (n) {
> __cfqg = rb_entry_cfqg(n);
> - cfqg->vdisktime = __cfqg->vdisktime + CFQ_IDLE_DELAY;
> + cfqg->vdisktime = __cfqg->vdisktime +
> + cfq_get_cfqg_vdisktime_delay(cfqd);
> } else
> cfqg->vdisktime = st->min_vdisktime;
> cfq_group_service_tree_add(st, cfqg);
> --
> 2.5.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode
2017-03-08 12:16 [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode Hou Tao
2017-03-08 14:05 ` Jan Kara
@ 2017-03-08 17:56 ` Jens Axboe
1 sibling, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2017-03-08 17:56 UTC (permalink / raw)
To: Hou Tao, linux-block; +Cc: jack, jmoyer, stable, vgoyal
On 03/08/2017 05:16 AM, Hou Tao wrote:
> When adding a cfq_group into the cfq service tree, we use CFQ_IDLE_DELAY
> as the delay of cfq_group's vdisktime if there have been other cfq_groups
> already.
>
> When cfq is under iops mode, commit 9a7f38c42c2b ("cfq-iosched: Convert
> from jiffies to nanoseconds") could result in a large iops delay and
> lead to an abnormal io schedule delay for the added cfq_group. To fix
> it, we just need to revert to the old CFQ_IDLE_DELAY value: HZ / 5
> when iops mode is enabled.
>
> Despite having the same value, the delay of a cfq_queue in idle class
> and the delay of cfq_queue are different things, so I define two new
> macros for the delay of a cfq_group under time-slice mode and IOPs mode.
Added, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode
2017-03-08 14:05 ` Jan Kara
@ 2017-03-09 11:22 ` Hou Tao
2017-05-31 3:09 ` Hou Tao
0 siblings, 1 reply; 6+ messages in thread
From: Hou Tao @ 2017-03-09 11:22 UTC (permalink / raw)
To: Jan Kara, axboe; +Cc: linux-block, jmoyer, stable, vgoyal
On 2017/3/8 22:05, Jan Kara wrote:
> On Wed 08-03-17 20:16:55, Hou Tao wrote:
>> When adding a cfq_group into the cfq service tree, we use CFQ_IDLE_DELAY
>> as the delay of cfq_group's vdisktime if there have been other cfq_groups
>> already.
>>
>> When cfq is under iops mode, commit 9a7f38c42c2b ("cfq-iosched: Convert
>> from jiffies to nanoseconds") could result in a large iops delay and
>> lead to an abnormal io schedule delay for the added cfq_group. To fix
>> it, we just need to revert to the old CFQ_IDLE_DELAY value: HZ / 5
>> when iops mode is enabled.
>>
>> Despite having the same value, the delay of a cfq_queue in idle class
>> and the delay of cfq_queue are different things, so I define two new
>> macros for the delay of a cfq_group under time-slice mode and IOPs mode.
>>
>> Fixes: 9a7f38c42c2b92391d9dabaf9f51df7cfe5608e4
>> Cc: <stable@vger.kernel.org> # 4.8+
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>
> OK, the number 200 is somewhat arbitrary but so is HZ/5 so I guess we are
> OK. You can add:
>
> Acked-by: Jan Kara <jack@suse.cz>
Oops, sorry for the arbitrary 200. My intention was to use HZ / 5 instead of 200
to keep consistent with the old CFQ_IDLE_DELAY. And I spot two typos in commit
message: "the delay of cfq_queue" -> "the delay of cfq_group" and "IOPs" -> "iops".
Jan, could you please fix them ? And I also attach a revised patch to fix them.
From: Hou Tao <houtao1@huawei.com>
Date: Wed, 1 Mar 2017 09:02:33 +0800
Subject: [PATCH] cfq-iosched: fix the delay of cfq_group's vdisktime under
iops mode
When adding a cfq_group into the cfq service tree, we use CFQ_IDLE_DELAY
as the delay of cfq_group's vdisktime if there have been other cfq_groups
already.
When cfq is under iops mode, commit 9a7f38c42c2b ("cfq-iosched: Convert
from jiffies to nanoseconds") could result in a large iops delay and
lead to an abnormal io schedule delay for the added cfq_group. To fix
it, we just need to revert to the old CFQ_IDLE_DELAY value: HZ / 5
when iops mode is enabled.
Despite having the same value, the delay of a cfq_queue in idle class
and the delay of cfq_group are different things, so I define two new
macros for the delay of a cfq_group under time-slice mode and iops mode.
Fixes: 9a7f38c42c2b92391d9dabaf9f51df7cfe5608e4
Cc: <stable@vger.kernel.org> # 4.8+
Signed-off-by: Hou Tao <houtao1@huawei.com>
Acked-by: Jan Kara <jack@suse.cz>
---
block/cfq-iosched.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 440b95e..2762505 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -38,9 +38,13 @@ static const u64 cfq_target_latency = (u64)NSEC_PER_SEC * 3/10; /* 300 ms */
static const int cfq_hist_divisor = 4;
/*
- * offset from end of service tree
+ * offset from end of queue service tree for idle class
*/
#define CFQ_IDLE_DELAY (NSEC_PER_SEC / 5)
+/* offset from end of group service tree under time slice mode */
+#define CFQ_SLICE_MODE_GROUP_DELAY (NSEC_PER_SEC / 5)
+/* offset from end of group service under IOPS mode */
+#define CFQ_IOPS_MODE_GROUP_DELAY (HZ / 5)
/*
* below this threshold, we consider thinktime immediate
@@ -1362,6 +1366,14 @@ cfq_group_service_tree_add(struct cfq_rb_root *st, struct cfq_group *cfqg)
cfqg->vfraction = max_t(unsigned, vfr, 1);
}
+static inline u64 cfq_get_cfqg_vdisktime_delay(struct cfq_data *cfqd)
+{
+ if (!iops_mode(cfqd))
+ return CFQ_SLICE_MODE_GROUP_DELAY;
+ else
+ return CFQ_IOPS_MODE_GROUP_DELAY;
+}
+
static void
cfq_group_notify_queue_add(struct cfq_data *cfqd, struct cfq_group *cfqg)
{
@@ -1381,7 +1393,8 @@ cfq_group_notify_queue_add(struct cfq_data *cfqd, struct cfq_group *cfqg)
n = rb_last(&st->rb);
if (n) {
__cfqg = rb_entry_cfqg(n);
- cfqg->vdisktime = __cfqg->vdisktime + CFQ_IDLE_DELAY;
+ cfqg->vdisktime = __cfqg->vdisktime +
+ cfq_get_cfqg_vdisktime_delay(cfqd);
} else
cfqg->vdisktime = st->min_vdisktime;
cfq_group_service_tree_add(st, cfqg);
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode
2017-03-09 11:22 ` Hou Tao
@ 2017-05-31 3:09 ` Hou Tao
2017-05-31 15:25 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Hou Tao @ 2017-05-31 3:09 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Jan Kara
Hi Jens,
I didn't found the patch in your linux-block git tree and the vanilla git tree.
Maybe you have forgot this CFQ fix ?
Regards,
Tao
On 2017/3/9 19:22, Hou Tao wrote:
> On 2017/3/8 22:05, Jan Kara wrote:
>> On Wed 08-03-17 20:16:55, Hou Tao wrote:
>>> When adding a cfq_group into the cfq service tree, we use CFQ_IDLE_DELAY
>>> as the delay of cfq_group's vdisktime if there have been other cfq_groups
>>> already.
>>>
>>> When cfq is under iops mode, commit 9a7f38c42c2b ("cfq-iosched: Convert
>>> from jiffies to nanoseconds") could result in a large iops delay and
>>> lead to an abnormal io schedule delay for the added cfq_group. To fix
>>> it, we just need to revert to the old CFQ_IDLE_DELAY value: HZ / 5
>>> when iops mode is enabled.
>>>
>>> Despite having the same value, the delay of a cfq_queue in idle class
>>> and the delay of cfq_queue are different things, so I define two new
>>> macros for the delay of a cfq_group under time-slice mode and IOPs mode.
>>>
>>> Fixes: 9a7f38c42c2b92391d9dabaf9f51df7cfe5608e4
>>> Cc: <stable@vger.kernel.org> # 4.8+
>>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>>
>> OK, the number 200 is somewhat arbitrary but so is HZ/5 so I guess we are
>> OK. You can add:
>>
>> Acked-by: Jan Kara <jack@suse.cz>
> Oops, sorry for the arbitrary 200. My intention was to use HZ / 5 instead of 200
> to keep consistent with the old CFQ_IDLE_DELAY. And I spot two typos in commit
> message: "the delay of cfq_queue" -> "the delay of cfq_group" and "IOPs" -> "iops".
>
> Jan, could you please fix them ? And I also attach a revised patch to fix them.
>
>
> From: Hou Tao <houtao1@huawei.com>
> Date: Wed, 1 Mar 2017 09:02:33 +0800
> Subject: [PATCH] cfq-iosched: fix the delay of cfq_group's vdisktime under
> iops mode
>
> When adding a cfq_group into the cfq service tree, we use CFQ_IDLE_DELAY
> as the delay of cfq_group's vdisktime if there have been other cfq_groups
> already.
>
> When cfq is under iops mode, commit 9a7f38c42c2b ("cfq-iosched: Convert
> from jiffies to nanoseconds") could result in a large iops delay and
> lead to an abnormal io schedule delay for the added cfq_group. To fix
> it, we just need to revert to the old CFQ_IDLE_DELAY value: HZ / 5
> when iops mode is enabled.
>
> Despite having the same value, the delay of a cfq_queue in idle class
> and the delay of cfq_group are different things, so I define two new
> macros for the delay of a cfq_group under time-slice mode and iops mode.
>
> Fixes: 9a7f38c42c2b92391d9dabaf9f51df7cfe5608e4
> Cc: <stable@vger.kernel.org> # 4.8+
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> Acked-by: Jan Kara <jack@suse.cz>
> ---
> block/cfq-iosched.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 440b95e..2762505 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -38,9 +38,13 @@ static const u64 cfq_target_latency = (u64)NSEC_PER_SEC * 3/10; /* 300 ms */
> static const int cfq_hist_divisor = 4;
>
> /*
> - * offset from end of service tree
> + * offset from end of queue service tree for idle class
> */
> #define CFQ_IDLE_DELAY (NSEC_PER_SEC / 5)
> +/* offset from end of group service tree under time slice mode */
> +#define CFQ_SLICE_MODE_GROUP_DELAY (NSEC_PER_SEC / 5)
> +/* offset from end of group service under IOPS mode */
> +#define CFQ_IOPS_MODE_GROUP_DELAY (HZ / 5)
>
> /*
> * below this threshold, we consider thinktime immediate
> @@ -1362,6 +1366,14 @@ cfq_group_service_tree_add(struct cfq_rb_root *st, struct cfq_group *cfqg)
> cfqg->vfraction = max_t(unsigned, vfr, 1);
> }
>
> +static inline u64 cfq_get_cfqg_vdisktime_delay(struct cfq_data *cfqd)
> +{
> + if (!iops_mode(cfqd))
> + return CFQ_SLICE_MODE_GROUP_DELAY;
> + else
> + return CFQ_IOPS_MODE_GROUP_DELAY;
> +}
> +
> static void
> cfq_group_notify_queue_add(struct cfq_data *cfqd, struct cfq_group *cfqg)
> {
> @@ -1381,7 +1393,8 @@ cfq_group_notify_queue_add(struct cfq_data *cfqd, struct cfq_group *cfqg)
> n = rb_last(&st->rb);
> if (n) {
> __cfqg = rb_entry_cfqg(n);
> - cfqg->vdisktime = __cfqg->vdisktime + CFQ_IDLE_DELAY;
> + cfqg->vdisktime = __cfqg->vdisktime +
> + cfq_get_cfqg_vdisktime_delay(cfqd);
> } else
> cfqg->vdisktime = st->min_vdisktime;
> cfq_group_service_tree_add(st, cfqg);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode
2017-05-31 3:09 ` Hou Tao
@ 2017-05-31 15:25 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2017-05-31 15:25 UTC (permalink / raw)
To: Hou Tao; +Cc: linux-block, Jan Kara
On 05/30/2017 09:09 PM, Hou Tao wrote:
> Hi Jens,
>
> I didn't found the patch in your linux-block git tree and the vanilla git tree.
> Maybe you have forgot this CFQ fix ?
Looks like that did get missed, sorry about that. I've queued it up now.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-31 15:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-08 12:16 [PATCH v2] cfq-iosched: fix the delay of cfq_group's vdisktime under iops mode Hou Tao
2017-03-08 14:05 ` Jan Kara
2017-03-09 11:22 ` Hou Tao
2017-05-31 3:09 ` Hou Tao
2017-05-31 15:25 ` Jens Axboe
2017-03-08 17:56 ` 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).