Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
MIME-Version: 1.0
References: <20231201073240.T9bFNCkU@linutronix.de> <20231201080522.GA31309@didi-ThinkCentre-M930t-N000>
 <20231201161640.Z0cJLUi3@linutronix.de> <CAEQmJ=jbB5ag5HRMjZqjych_wj_v6wTMLQxJbnJsgwNdn37kbw@mail.gmail.com>
 <20231205153146.OSpCIs1G@linutronix.de> <CAEQmJ=iNVUuQQrm4bJgud=e7yhgwNb4Q1-BgOQF98D7NP9r29A@mail.gmail.com>
 <20231207103536.30ae05aa@gandalf.local.home>
In-Reply-To: <20231207103536.30ae05aa@gandalf.local.home>
From:   Yuanhan Zhang <zyhtheonly@gmail.com>
Date:   Thu, 7 Dec 2023 12:19:28 -0500
Message-ID: <CAEQmJ=jceiVevd9d3i4h0BHCNc8FFONJqEaiKyN9p21rg0QdkQ@mail.gmail.com>
Subject: Re: [PATCH v3] sched/cputime: let ktimers align with ksoftirqd in
 accounting CPUTIME_SOFTIRQ
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        zyhtheonly@yeah.net, tglx@linutronix.de, mingo@redhat.com,
        Venkatesh Pallipadi <venki@google.com>, peterz@infradead.org,
        juri.lelli@redhat.com, vincent.guittot@linaro.org,
        linux-kernel@vger.kernel.org, dietmar.eggemann@arm.com,
        bsegall@google.com, mgorman@suse.de, bristot@redhat.com,
        vschneid@redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

Steven Rostedt <rostedt@goodmis.org> =E4=BA=8E2023=E5=B9=B412=E6=9C=887=E6=
=97=A5=E5=91=A8=E5=9B=9B 10:35=E5=86=99=E9=81=93=EF=BC=9A
>
> On Thu, 7 Dec 2023 18:43:47 +0800
> Yuanhan Zhang <zyhtheonly@gmail.com> wrote:
>
> > It results in if we do not handle ksoftirqd like this, we will have a
> > bigger SYSTEM and less SOFTIRQ.
>
> And honestly that's what we want. Interrupts and softirqs that execute in
> interrupts and softirq context take away from the system. That is, if the=
y
> are not explicitly blocked (local_irq_disable/local_bh_disable) they
> interrupt the current task and take up the time of the current task. We
> need to differentiate this because this context has no "task" context to
> measure.
>
> We do not want to add ksoftirqd or threaded interrupt handlers / softirqs
> to this measurement. Sure, they are handling interrupt and softirq code,
> but they have their own context that can be measured like any other task.
>
> If we blur this with real irqs and softirqs, then we will not know what
> those real irqs and softirqs are measuring.

Yes you say clearly enough and it makes some sense to me!
So my understanding is that in PREEMPT_RT, it is better to put ksoftirqd's =
time
into SYSTEM since they are just in their "task" context.

If my understanding is right, how about we just exclude ksoftirqd
like what I do in the last email?
(something like
    else if (in_serving_softirq() &&
               (IS_ENABLED(CONFIG_PREEMPT_RT) || curr !=3D this_cpu_ksoftir=
qd()))

If this looks okay to you, I'm happy to send a new patch for this :)

>
> > So my point is if we do not align ktimers, ktimers would act like
> > **observation on *not-excluded ksoftirq patched* kernel** part in the
> > above example,
> > and this might make SOFTIRQ less than expected, /proc/stat less accurat=
e.
>
> No it does not. When a softirq kicks off it's work to a thread (ksoftirq,
> threaded softirqd, or simply a workqueue), it's no longer running in
> softirq context, and should not be measured as such.
>
> The measurement is not about how much work the softirq is doing (otherwis=
e
> we need to add workqueues started by softirqs too), it's about measuring
> the actual irq and softirq context. In PREEMPT_RT, we try to eliminate th=
at
> context as much as possible.

Thanks anyway, I think I begin to learn a bit more about PREEMPT_RT...

>
> So seeing less is a feature not a bug!
>
> -- Steve