[RFC] blkio_delay_total is calculated mistakenly

[RFC] blkio_delay_total is calculated mistakenly

[Weiwei Jia]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

Hi,

When I read Linux Kernel source codes about how "blkio_delay_total"
(delay waiting for synchronous block I/O to complete for one task) is
calculated in Linux/kernel/delayacct.c file [1], I find that
"blkio_delay_total" adds "task->delays->blkio_delay" to be the total
delay time for one task to wait for synchronous I/O dynamically but
"tsk->delays->blkio_delay" has already been calculated as the total
delay time that one task is waiting for synchronous I/O [2]
dynamically. I think "blkio_delay_total" should not add
"task->delays->blkio_delay" since "task->delays->blkio_delay" is
already the total delay time we want.

Am I missing anything?

BTW, the patch in the attachment may solve this problem if I don't
miss anything. Thank you.


[1] http://lxr.free-electrons.com/source/kernel/delayacct.c?v=4.7#L121
[2] http://lxr.free-electrons.com/source/kernel/delayacct.c?v=4.7#L52

Thanks,
Weiwei Jia

[-- Attachment #2: 0001-fix-the-mistake-blkio_delay_total-calculation-way.patch --]
[-- Type: text/x-patch, Size: 983 bytes --]

From 8c290bce66f0ef2b117c8f47a13bbf2d80c500f0 Mon Sep 17 00:00:00 2001
From: Weiwei Jia <harryxiyou@gmail.com>
Date: Tue, 14 Feb 2017 20:18:34 -0500
Subject: [PATCH] fix the mistake blkio_delay_total calculation way

Signed-off-by: Weiwei Jia <harryxiyou@gmail.com>

---
 kernel/delayacct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index 435c14a..0d8a6cc 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -118,7 +118,7 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
 	/* zero XXX_total, non-zero XXX_count implies XXX stat overflowed */
 
 	spin_lock_irqsave(&tsk->delays->lock, flags);
-	tmp = d->blkio_delay_total + tsk->delays->blkio_delay;
+	tmp = tsk->delays->blkio_delay;
 	d->blkio_delay_total = (tmp < d->blkio_delay_total) ? 0 : tmp;
 	tmp = d->swapin_delay_total + tsk->delays->swapin_delay;
 	d->swapin_delay_total = (tmp < d->swapin_delay_total) ? 0 : tmp;
-- 
2.1.4

Re: [RFC] blkio_delay_total is calculated mistakenly

[Greg KH]

On Tue, Feb 14, 2017 at 08:27:36PM -0500, Weiwei Jia wrote:
> Hi,
> 
> When I read Linux Kernel source codes about how "blkio_delay_total"
> (delay waiting for synchronous block I/O to complete for one task) is
> calculated in Linux/kernel/delayacct.c file [1], I find that
> "blkio_delay_total" adds "task->delays->blkio_delay" to be the total
> delay time for one task to wait for synchronous I/O dynamically but
> "tsk->delays->blkio_delay" has already been calculated as the total
> delay time that one task is waiting for synchronous I/O [2]
> dynamically. I think "blkio_delay_total" should not add
> "task->delays->blkio_delay" since "task->delays->blkio_delay" is
> already the total delay time we want.
> 
> Am I missing anything?

Why are you sending this to me?  You have read
Documentation/SubmittingPatches, right?

Please use scripts/get_maintainers.pl for who to send patches to.

thanks,

greg k-h

Re: [RFC] blkio_delay_total is calculated mistakenly

[Weiwei Jia]

On Tue, Feb 14, 2017 at 10:00 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Feb 14, 2017 at 08:27:36PM -0500, Weiwei Jia wrote:
>> Hi,
>>Balbir Singh <bsingharora@gmail.com>
>> When I read Linux Kernel source codes about how "blkio_delay_total"
>> (delay waiting for synchronous block I/O to complete for one task) is
>> calculated in Linux/kernel/delayacct.c file [1], I find that
>> "blkio_delay_total" adds "task->delays->blkio_delay" to be the total
>> delay time for one task to wait for synchronous I/O dynamically but
>> "tsk->delays->blkio_delay" has already been calculated as the total
>> delay time that one task is waiting for synchronous I/O [2]
>> dynamically. I think "blkio_delay_total" should not add
>> "task->delays->blkio_delay" since "task->delays->blkio_delay" is
>> already the total delay time we want.
>>
>> Am I missing anything?
>
> Why are you sending this to me?  You have read
> Documentation/SubmittingPatches, right?
>
> Please use scripts/get_maintainers.pl for who to send patches to.

Okay. Have cc'ed Balbir Singh <bsingharora@gmail.com> already.

Thanks,
Weiwei Jia

Re: [RFC] blkio_delay_total is calculated mistakenly

[Weiwei Jia]

Ping?

Can anyone please add some comments for this change? Thank you.

Best,
Weiwei Jia

On Tue, Feb 14, 2017 at 8:27 PM, Weiwei Jia <harrynjit@gmail.com> wrote:
> Hi,
>
> When I read Linux Kernel source codes about how "blkio_delay_total"
> (delay waiting for synchronous block I/O to complete for one task) is
> calculated in Linux/kernel/delayacct.c file [1], I find that
> "blkio_delay_total" adds "task->delays->blkio_delay" to be the total
> delay time for one task to wait for synchronous I/O dynamically but
> "tsk->delays->blkio_delay" has already been calculated as the total
> delay time that one task is waiting for synchronous I/O [2]
> dynamically. I think "blkio_delay_total" should not add
> "task->delays->blkio_delay" since "task->delays->blkio_delay" is
> already the total delay time we want.
>
> Am I missing anything?
>
> BTW, the patch in the attachment may solve this problem if I don't
> miss anything. Thank you.
>
>
> [1] http://lxr.free-electrons.com/source/kernel/delayacct.c?v=4.7#L121
> [2] http://lxr.free-electrons.com/source/kernel/delayacct.c?v=4.7#L52
>
> Thanks,
> Weiwei Jia