linux-kernel - Re: [PATCH v3] sched/cputime: let ktimers align with ksoftirqd in accounting CPUTIME

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]

Date:   Sat, 2 Dec 2023 05:28:15 -0500
From:   Yuanhan Zhang <zyhtheonly@...il.com>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     zyhtheonly@...h.net, tglx@...utronix.de, rostedt@...dmis.org,
        mingo@...hat.com, Venkatesh Pallipadi <venki@...gle.com>,
        peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, linux-kernel@...r.kernel.org,
        dietmar.eggemann@....com, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com, vschneid@...hat.com
Subject: Re: [PATCH v3] sched/cputime: let ktimers align with ksoftirqd in
 accounting CPUTIME_SOFTIRQ

Hi,
Thanks for your kindly interpretation.

Sebastian Andrzej Siewior <bigeasy@...utronix.de> 于2023年12月1日周五 11:16写道：
>
> On 2023-12-01 16:05:41 [+0800], tiozhang wrote:
> > In CONFIG_PREEMPT_RT kernel, ktimers also calls __do_softirq,
> > so when accounting CPUTIME_SOFTIRQ, ktimers need to be accounted the same
> > as ksoftirqd.
>
> I still don't understand why this is a good thing and why want to align
> it with ksoftirqd and what breaks if we don't.

My motivation of doing this is to keep CPUTIME_SOFTIRQ in /proc/stat
remaining more accurate in PREEPT_RT kernel.

If we dont align, ktimers' cpu time is added to CPUTIME_SYSTEM when
CONFIG_IRQ_TIME_ACCOUNTING is enabled, make our stats less accurate..

>
> This "skip ksoftirqd for accounting" has been added in commit
>         b52bfee445d31 ("sched: Add IRQ_TIME_ACCOUNTING, finer accounting of irq time")
>
> At this point (v2.6.37) it had no accounting of time spent in ksoftirqd as
> SOFTIRQ time. This was then fixed/ added by commit
>         414bee9ba613a ("softirqs: Account ksoftirqd time as cpustat softirq")
>
> which went in v2.6.39. It started accounting it when it was noticed by
> the tick. So it is less accurate. The "benefit" seems to be that this
> accounting pops up in /proc/stat. As per-CPU or overall.
>
> I *guess* this was to align the softirqs which occur at the end of an
> interrupt with those which were outsourced to ksoftirqd because they
> took too long. This would patch the wording
>         … wanted to see more complete solution in not accounting irq
>         processing time to tasks at all.
>         https://lore.kernel.org/all/1284688596-6731-1-git-send-email-venki@google.com/
>
> Or it tried to preserve the current status.
>
> A different account occurs for SOFTIRQs if they occur as port of
> hardirq and are maybe deferred to ksoftirqd vs a task raising softirqs
> on their own like packet over loopback.
> Don't see the benefit that but this is my interpretation based on what
> it does.
>
> This was v2.6.39. Since then we got threaded interrupts (also v2.6.39)
> or threaded-NAPI which utilise mostly the same mechanism as ksoftirqd
> but are treated differently. I don't see why ktimers should align with
> ksoftirqd and honestly and I don't understand why ksoftirqd had to be
> excluded to in the first place.

The main diff between ksoftirqd and force-threaded interrupt is that ksoftirq
is in SOFTIRQ_OFFSET (serving softirqs) while  force-threaded is in
SOFTIRQ_DISABLE_OFFSET (by using local_disable_bh).

CPUTIME_SOFTIRQ serves for time in SOFTIRQ_OFFSET (processing softirqs).
See
https://lore.kernel.org/all/1285619753-10892-1-git-send-email-venki@google.com/

So this leads to ksoftirqd is counted into CPUTIME_SOFTIRQ but irq-threads
into CPUTIME_SYSTEM.

Since ktimers is also in SOFTIRQ_OFFSET, align it with ksoftirq will
put it into CPUTIME_SOFTIRQ, making /proc/stat more accurate.

>
> Sorry, but I would need to go on than this.

Thanks for your time.

>
> > Signed-off-by: tiozhang <tiozhang@...iglobal.com>
>
> Sebastian