linux-kernel - Re: [PATCH 6/6] Account ksoftirqd time as cpustat softirq -v1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 26 Oct 2010 10:35:15 -0700
From:	Venkatesh Pallipadi <venki@...gle.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	linux-kernel@...r.kernel.org, Paul Turner <pjt@...gle.com>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Shaun Ruffell <sruffell@...ium.com>,
	Yong Zhang <yong.zhang0@...il.com>
Subject: Re: [PATCH 6/6] Account ksoftirqd time as cpustat softirq -v1

On Tue, Oct 26, 2010 at 2:50 AM, Peter Zijlstra <peterz@...radead.org> wrote:
> On Tue, 2010-10-26 at 11:33 +0200, Peter Zijlstra wrote:
>> On Mon, 2010-10-25 at 15:30 -0700, Venkatesh Pallipadi wrote:
>> > softirq time in ksoftirqd context is not accounted in ns granularity
>> > per cpu softirq stats, as we want that to be a part of ksoftirqd
>> > exec_runtime.
>> >
>> > Accounting them as softirq on /proc/stat separately.
>> >
>> > Tested-by: Shaun Ruffell <sruffell@...ium.com>
>> >
>> > Signed-off-by: Venkatesh Pallipadi <venki@...gle.com>
>> > ---
>> >  kernel/sched.c |    8 ++++++++
>> >  1 files changed, 8 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/kernel/sched.c b/kernel/sched.c
>> > index 49f6f61..0955050 100644
>> > --- a/kernel/sched.c
>> > +++ b/kernel/sched.c
>> > @@ -3617,6 +3617,14 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
>> >             cpustat->irq = cputime64_add(cpustat->irq, tmp);
>> >     } else if (irqtime_account_si_update()) {
>> >             cpustat->softirq = cputime64_add(cpustat->softirq, tmp);
>> > +   } else if (this_cpu_ksoftirqd() == p) {
>> > +           /*
>> > +            * ksoftirqd time do not get accounted in cpu_softirq_time.
>> > +            * So, we have to handle it separately here.
>> > +            * Also, p->stime needs to be updated for ksoftirqd.
>> > +            */
>> > +           __account_system_time(p, cputime_one_jiffy, one_jiffy_scaled,
>> > +                                   &cpustat->softirq);
>> >     } else if (user_tick) {
>> >             account_user_time(p, cputime_one_jiffy, one_jiffy_scaled);
>> >     } else if (p == rq->idle) {
>>
>>
>> I'm somewhat confused by this patch.. This is significantly different
>> from the thing proposed last time around, which was to use:
>>
>>   cpustat->softirq + this_cpu_ksoftirqd()->se.sum_exec_runtime
>>

Previous version I was using sum_exec_runtime and later I was also
checking whether the current task is ksoftirqd before charging. So, in
effect that was stricter change which would only account when there is
folding and current happens to be ksoftirqd. The second check was
required as we are also accounting ksoftirqd->stime and I didn't want
to do that when current is not ksoftirqd.

>> The above looses the fine grained aspect of the accounting and simply
>> charges a whole jiffy if the current process happens to be ksoftirqd.
>

Yes. This leaves ksoftirqd with coarse granularity as before.

> Btw, both these solutions can cause si + us + ni + sy > 100%, are we ok
> with that?

The current if/else if/.../else/ logic gets called every tick and
charges that tick to what it thinks is the most appropriate bucket.
So, it should not be greater than 100%. ksoftirqd gets accounted only
softirq tick and not as system and thats how it has been currently.

Thanks,
Venki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/