This series contain a few patch to correct comment, correct trace of
vtime_rate and so on. More detail can be found in the respective
changelogs.
---
v2:
Thanks Tejun for review and comment!
Add Acked-by tag from Tejun.
Correct description in patch 3/5 and 4/5.
Drop "blk-iocost: Avoid to call current_hweight_max if iocg->inuse
== iocg->active"
Drop "blk-iocost: Remove redundant initialization of struct ioc_gq"
Drop "blk-iocost: Get ioc_now inside weight_updated"
---
Kemeng Shi (5):
blk-iocost: Fix typo in comment
blk-iocost: Reset vtime_base_rate in ioc_refresh_params
blk-iocost: Trace vtime_base_rate instead of vtime_rate
blk-iocost: Remove vrate member in struct ioc_now
blk-iocost: Correct comment in blk_iocost_init
block/blk-iocost.c | 16 +++++++++-------
include/trace/events/iocost.h | 4 ++--
2 files changed, 11 insertions(+), 9 deletions(-)
--
2.30.0
Since commit ac33e91e2daca ("blk-iocost: implement vtime loss
compensation") rename original vtime_rate to vtime_base_rate
and current vtime_rate is original vtime_rate with compensation.
The current rate showed in tracepoint is mixed with vtime_rate
and vtime_base_rate:
1) In function ioc_adjust_base_vrate, the first trace_iocost_ioc_vrate_adj
shows vtime_rate, the second trace_iocost_ioc_vrate_adj shows
vtime_base_rate.
2) In function iocg_activate shows vtime_rate by calling
TRACE_IOCG_PATH(iocg_activate...
3) In function ioc_check_iocgs shows vtime_rate by calling
TRACE_IOCG_PATH(iocg_idle...
Trace vtime_base_rate instead of vtime_rate as:
1) Before commit ac33e91e2daca ("blk-iocost: implement vtime loss
compensation"), the traced rate is without compensation, so still
show rate without compensation.
2) The vtime_base_rate is more stable while vtime_rate heavily depends on
excess budeget on current period which may change abruptly in next period.
Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
block/blk-iocost.c | 2 +-
include/trace/events/iocost.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 9214733bbc14..b0991b52e3dd 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -977,7 +977,7 @@ static void ioc_adjust_base_vrate(struct ioc *ioc, u32 rq_wait_pct,
if (!ioc->busy_level || (ioc->busy_level < 0 && nr_lagging)) {
if (ioc->busy_level != prev_busy_level || nr_lagging)
- trace_iocost_ioc_vrate_adj(ioc, atomic64_read(&ioc->vtime_rate),
+ trace_iocost_ioc_vrate_adj(ioc, vrate,
missed_ppm, rq_wait_pct,
nr_lagging, nr_shortages);
diff --git a/include/trace/events/iocost.h b/include/trace/events/iocost.h
index 6d1626e7a4ce..af8bfed528fc 100644
--- a/include/trace/events/iocost.h
+++ b/include/trace/events/iocost.h
@@ -38,7 +38,7 @@ DECLARE_EVENT_CLASS(iocost_iocg_state,
__assign_str(cgroup, path);
__entry->now = now->now;
__entry->vnow = now->vnow;
- __entry->vrate = now->vrate;
+ __entry->vrate = iocg->ioc->vtime_base_rate;
__entry->last_period = last_period;
__entry->cur_period = cur_period;
__entry->vtime = vtime;
@@ -160,7 +160,7 @@ TRACE_EVENT(iocost_ioc_vrate_adj,
TP_fast_assign(
__assign_str(devname, ioc_name(ioc));
- __entry->old_vrate = atomic64_read(&ioc->vtime_rate);
+ __entry->old_vrate = ioc->vtime_base_rate;
__entry->new_vrate = new_vrate;
__entry->busy_level = ioc->busy_level;
__entry->read_missed_ppm = missed_ppm[READ];
--
2.30.0
soley -> solely
Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
block/blk-iocost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 495396425bad..be4bc38821e2 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -111,7 +111,7 @@
* busy signal.
*
* As devices can have deep queues and be unfair in how the queued commands
- * are executed, soley depending on rq wait may not result in satisfactory
+ * are executed, solely depending on rq wait may not result in satisfactory
* control quality. For a better control quality, completion latency QoS
* parameters can be configured so that the device is considered saturated
* if N'th percentile completion latency rises above the set point.
--
2.30.0
If we trace vtime_base_rate instead of vtime_rate, there is nowhere
which accesses now->vrate except function ioc_now using now->vrate locally.
Just remove it.
Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
block/blk-iocost.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index b0991b52e3dd..761295ed9c5a 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -556,7 +556,6 @@ struct ioc_now {
u64 now_ns;
u64 now;
u64 vnow;
- u64 vrate;
};
struct iocg_wait {
@@ -1020,10 +1019,11 @@ static void ioc_adjust_base_vrate(struct ioc *ioc, u32 rq_wait_pct,
static void ioc_now(struct ioc *ioc, struct ioc_now *now)
{
unsigned seq;
+ u64 vrate;
now->now_ns = ktime_get();
now->now = ktime_to_us(now->now_ns);
- now->vrate = atomic64_read(&ioc->vtime_rate);
+ vrate = atomic64_read(&ioc->vtime_rate);
/*
* The current vtime is
@@ -1036,7 +1036,7 @@ static void ioc_now(struct ioc *ioc, struct ioc_now *now)
do {
seq = read_seqcount_begin(&ioc->period_seqcount);
now->vnow = ioc->period_at_vtime +
- (now->now - ioc->period_at) * now->vrate;
+ (now->now - ioc->period_at) * vrate;
} while (read_seqcount_retry(&ioc->period_seqcount, seq));
}
--
2.30.0
There is no iocg_pd_init function. The pd_alloc_fn function pointer of
iocost policy is set with ioc_pd_init. Just correct it.
Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
block/blk-iocost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 761295ed9c5a..96c1571a8a1d 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2880,7 +2880,7 @@ static int blk_iocost_init(struct gendisk *disk)
spin_unlock_irq(&ioc->lock);
/*
- * rqos must be added before activation to allow iocg_pd_init() to
+ * rqos must be added before activation to allow ioc_pd_init() to
* lookup the ioc from q. This means that the rqos methods may get
* called before policy activation completion, can't assume that the
* target bio has an iocg associated and need to test for NULL iocg.
--
2.30.0
Friendly ping
on 10/18/2022 8:19 PM, Kemeng Shi wrote:
> This series contain a few patch to correct comment, correct trace of
> vtime_rate and so on. More detail can be found in the respective
> changelogs.
>
> ---
> v2:
> Thanks Tejun for review and comment!
> Add Acked-by tag from Tejun.
> Correct description in patch 3/5 and 4/5.
> Drop "blk-iocost: Avoid to call current_hweight_max if iocg->inuse
> == iocg->active"
> Drop "blk-iocost: Remove redundant initialization of struct ioc_gq"
> Drop "blk-iocost: Get ioc_now inside weight_updated"
> ---
>
> Kemeng Shi (5):
> blk-iocost: Fix typo in comment
> blk-iocost: Reset vtime_base_rate in ioc_refresh_params
> blk-iocost: Trace vtime_base_rate instead of vtime_rate
> blk-iocost: Remove vrate member in struct ioc_now
> blk-iocost: Correct comment in blk_iocost_init
>
> block/blk-iocost.c | 16 +++++++++-------
> include/trace/events/iocost.h | 4 ++--
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
--
Best wishes
Kemeng Shi
Hi jens,
on 10/18/2022 8:19 PM, Kemeng Shi wrote:
> This series contain a few patch to correct comment, correct trace of
> vtime_rate and so on. More detail can be found in the respective
> changelogs.
>
> ---
> v2:
> Thanks Tejun for review and comment!
> Add Acked-by tag from Tejun.
> Correct description in patch 3/5 and 4/5.
> Drop "blk-iocost: Avoid to call current_hweight_max if iocg->inuse
> == iocg->active"
> Drop "blk-iocost: Remove redundant initialization of struct ioc_gq"
> Drop "blk-iocost: Get ioc_now inside weight_updated"
> ---
>
> Kemeng Shi (5):
> blk-iocost: Fix typo in comment
> blk-iocost: Reset vtime_base_rate in ioc_refresh_params
> blk-iocost: Trace vtime_base_rate instead of vtime_rate
> blk-iocost: Remove vrate member in struct ioc_now
> blk-iocost: Correct comment in blk_iocost_init
>
> block/blk-iocost.c | 16 +++++++++-------
> include/trace/events/iocost.h | 4 ++--
> 2 files changed, 11 insertions(+), 9 deletions(-)
Could you apply this patchset?
By the way, my apply for an cloud variant of email was just passed
a few days ago. Is this mail still in spam?
Thanks.
--
Best wishes
Kemeng Shi
On 11/30/22 6:45?PM, Kemeng Shi wrote:
>
> Hi jens,
> on 10/18/2022 8:19 PM, Kemeng Shi wrote:
>> This series contain a few patch to correct comment, correct trace of
>> vtime_rate and so on. More detail can be found in the respective
>> changelogs.
>>
>> ---
>> v2:
>> Thanks Tejun for review and comment!
>> Add Acked-by tag from Tejun.
>> Correct description in patch 3/5 and 4/5.
>> Drop "blk-iocost: Avoid to call current_hweight_max if iocg->inuse
>> == iocg->active"
>> Drop "blk-iocost: Remove redundant initialization of struct ioc_gq"
>> Drop "blk-iocost: Get ioc_now inside weight_updated"
>> ---
>>
>> Kemeng Shi (5):
>> blk-iocost: Fix typo in comment
>> blk-iocost: Reset vtime_base_rate in ioc_refresh_params
>> blk-iocost: Trace vtime_base_rate instead of vtime_rate
>> blk-iocost: Remove vrate member in struct ioc_now
>> blk-iocost: Correct comment in blk_iocost_init
>>
>> block/blk-iocost.c | 16 +++++++++-------
>> include/trace/events/iocost.h | 4 ++--
>> 2 files changed, 11 insertions(+), 9 deletions(-)
> Could you apply this patchset?
> By the way, my apply for an cloud variant of email was just passed
> a few days ago. Is this mail still in spam?
This one wasn't, but I've seen the huaweicloud.com emails fail
the same origination checks in the past.
--
Jens Axboe
On Tue, 18 Oct 2022 20:19:27 +0800, Kemeng Shi wrote:
> This series contain a few patch to correct comment, correct trace of
> vtime_rate and so on. More detail can be found in the respective
> changelogs.
>
Applied, thanks!
[1/5] blk-iocost: Fix typo in comment
commit: e12d34fce7652a870fac62e9749a6623883d6fd8
[2/5] blk-iocost: Reset vtime_base_rate in ioc_refresh_params
commit: c81d97a61c8c3c25a576c3db4c4dd34316256f05
[3/5] blk-iocost: Trace vtime_base_rate instead of vtime_rate
commit: 0c0d362dc97d19d898219c13a357e167a9ee77ee
[4/5] blk-iocost: Remove vrate member in struct ioc_now
commit: f44ed8722034ef7d214fa121ae2402100a3590e1
[5/5] blk-iocost: Correct comment in blk_iocost_init
commit: b4c0482bfe89cd6c4f030314c86aae35642c44a5
Best regards,
--
Jens Axboe
on 12/1/2022 9:46 AM, Jens Axboe wrote:
> On 11/30/22 6:45?PM, Kemeng Shi wrote:
>>
>> Hi jens,
>> on 10/18/2022 8:19 PM, Kemeng Shi wrote:
>>> This series contain a few patch to correct comment, correct trace of
>>> vtime_rate and so on. More detail can be found in the respective
>>> changelogs.
>>>
>>> ---
>>> v2:
>>> Thanks Tejun for review and comment!
>>> Add Acked-by tag from Tejun.
>>> Correct description in patch 3/5 and 4/5.
>>> Drop "blk-iocost: Avoid to call current_hweight_max if iocg->inuse
>>> == iocg->active"
>>> Drop "blk-iocost: Remove redundant initialization of struct ioc_gq"
>>> Drop "blk-iocost: Get ioc_now inside weight_updated"
>>> ---
>>>
>>> Kemeng Shi (5):
>>> blk-iocost: Fix typo in comment
>>> blk-iocost: Reset vtime_base_rate in ioc_refresh_params
>>> blk-iocost: Trace vtime_base_rate instead of vtime_rate
>>> blk-iocost: Remove vrate member in struct ioc_now
>>> blk-iocost: Correct comment in blk_iocost_init
>>>
>>> block/blk-iocost.c | 16 +++++++++-------
>>> include/trace/events/iocost.h | 4 ++--
>>> 2 files changed, 11 insertions(+), 9 deletions(-)
>> Could you apply this patchset?
>> By the way, my apply for an cloud variant of email was just passed
>> a few days ago. Is this mail still in spam?
>
> This one wasn't, but I've seen the huaweicloud.com emails fail
> the same origination checks in the past.
I'm not sure if was there any fix to huaweicloud.com email. I will
use this huaweicloud emails to minimize the trouble before any
better solution is found. Sorry for the inconvenience.
--
Best wishes
Kemeng Shi
On 11/30/22 7:20 PM, Kemeng Shi wrote:
>
>
> on 12/1/2022 9:46 AM, Jens Axboe wrote:
>> On 11/30/22 6:45?PM, Kemeng Shi wrote:
>>>
>>> Hi jens,
>>> on 10/18/2022 8:19 PM, Kemeng Shi wrote:
>>>> This series contain a few patch to correct comment, correct trace of
>>>> vtime_rate and so on. More detail can be found in the respective
>>>> changelogs.
>>>>
>>>> ---
>>>> v2:
>>>> Thanks Tejun for review and comment!
>>>> Add Acked-by tag from Tejun.
>>>> Correct description in patch 3/5 and 4/5.
>>>> Drop "blk-iocost: Avoid to call current_hweight_max if iocg->inuse
>>>> == iocg->active"
>>>> Drop "blk-iocost: Remove redundant initialization of struct ioc_gq"
>>>> Drop "blk-iocost: Get ioc_now inside weight_updated"
>>>> ---
>>>>
>>>> Kemeng Shi (5):
>>>> blk-iocost: Fix typo in comment
>>>> blk-iocost: Reset vtime_base_rate in ioc_refresh_params
>>>> blk-iocost: Trace vtime_base_rate instead of vtime_rate
>>>> blk-iocost: Remove vrate member in struct ioc_now
>>>> blk-iocost: Correct comment in blk_iocost_init
>>>>
>>>> block/blk-iocost.c | 16 +++++++++-------
>>>> include/trace/events/iocost.h | 4 ++--
>>>> 2 files changed, 11 insertions(+), 9 deletions(-)
>>> Could you apply this patchset?
>>> By the way, my apply for an cloud variant of email was just passed
>>> a few days ago. Is this mail still in spam?
>>
>> This one wasn't, but I've seen the huaweicloud.com emails fail
>> the same origination checks in the past.
> I'm not sure if was there any fix to huaweicloud.com email. I will
> use this huaweicloud emails to minimize the trouble before any
> better solution is found. Sorry for the inconvenience.
Thanks, I'll let you know if I run into issues with the cloud
email.
--
Jens Axboe