linux-kernel - Re: [PATCH 1/7] si time accounting accounts bh

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTikwUh57PKNk_fWzXApoYnCqK9NFi-MzSY-QKUc5@mail.gmail.com>
Date:	Sat, 2 Oct 2010 08:42:02 -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>
Subject: Re: [PATCH 1/7] si time accounting accounts bh_disable'd time to si -v3

On Fri, Oct 1, 2010 at 4:16 PM, Peter Zijlstra <peterz@...radead.org> wrote:
> On Thu, 2010-09-30 at 09:26 -0700, Venkatesh Pallipadi wrote:
>> > One nit: in_serving_softirq() doesn't seem right as either:
>> >
>> >  - we're not accounting ksoftirq in it, or
>> >  - we're are and VIRT_CPU_ACCOUNTING is again broken ;-)
>> >
>> > So only the softirq from irq tails wants to have SOFTIRQ_OFFSET set, the
>> > ksoftirqd stuff can be tested for using PF_flags or something (ksoftirq
>> > doesn't currently have a PF_SOFTIRQ flag, but -rt does and we could
>> > bring that over).
>> >
>>
>> The problem is that ksoftirqd is also handling softirq's and some
>> eventual users of in_serving_softirq (like the network code in this
>> patch) want to differentiate between whether it is the softirq thats
>> running or some real process (!ksoftirqd) context.
>
> The make in_serving_softirq() be something like:
>  (preempt_count() & SOFTIRQ_OFFSET) || (current->flags & PF_SOFTIRQ)
>

Yes. I would also need in_softirq_at_hardirq_tail() for accounting calls :)

>> Also, ksoftirqd adds to softirq counts, does trace softirq, etc. So,
>> it kind of made sense to add the time also to softirq stats as well.
>> If we dont account time to softirq stats, then if some user is looking
>> at say time per softirq using the softirq count will be misled. No?
>
> Simply add back the task accounting when you report it?
>
>> In the other thread you mentioned doing that will cause problems. Were
>> you thinking of scheduler issues or are there other problems charging
>> softirq time this way?
>
> Of course there are.. you're double accounting the time of ksoftirqd,
> and worse, you're adding that back into the equation as part of the !
> sched_fair time.
>

No. There should not be any double accounting with this current
change. We account softirq processing both at hardirq tail and
ksoftirqd as CPU softirq time. It will be taken out of ksoftirqd sched
exec time as with any other thread. And it will be taken out of fair
time available on the CPU as well. Which to me seems to be the right
thing to do, as the this is more coupled with the CPU and ksoftirqd is
just giving context for softirqd to run.

Changing only hardirq tail to have SOFTIRQ_OFFSET and changing
ksoftirqd to SOFTIRQ_OFFSET*2 would result in these additional
ksoftirqd softirqs staying as the part of sched_fair time.

Or I am totally missing something here?

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/