Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221019114357.yipijpetxz7ns5aq@airbuntu> <F9BCD313-15BA-429C-B50C-1391DDC1239B@joelfernandes.org>
 <Y0/+ieCymugrjheC@localhost.localdomain>
In-Reply-To: <Y0/+ieCymugrjheC@localhost.localdomain>
From:   Joel Fernandes <joel@joelfernandes.org>
Date:   Wed, 19 Oct 2022 09:51:24 -0400
Message-ID: <CAEXW_YRAXUmc0zMTo8OowVk2ZNuYfBsvZDXMkVH35aWkpEUo2A@mail.gmail.com>
Subject: Re: [RFC PATCH 00/11] Reviving the Proxy Execution Series
To:     Juri Lelli <juri.lelli@redhat.com>
Cc:     Qais Yousef <qyousef@layalina.io>,
        "Connor O'Brien" <connoro@google.com>,
        linux-kernel@vger.kernel.org, kernel-team@android.com,
        John Stultz <jstultz@google.com>,
        Qais Yousef <qais.yousef@arm.com>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Valentin Schneider <vschneid@redhat.com>,
        Will Deacon <will@kernel.org>,
        Waiman Long <longman@redhat.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        "Paul E . McKenney" <paulmck@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Oct 19, 2022 at 9:41 AM Juri Lelli <juri.lelli@redhat.com> wrote:
>
> On 19/10/22 08:23, Joel Fernandes wrote:
> >
> >
> > > On Oct 19, 2022, at 7:43 AM, Qais Yousef <qyousef@layalina.io> wrote:
> > >
> > > =EF=BB=BFOn 10/17/22 02:23, Joel Fernandes wrote:
> > >
> > >> I ran a test to check CFS time sharing. The accounting on top is con=
fusing,
> > >> but ftrace confirms the proxying happening.
> > >>
> > >> Task A - pid 122
> > >> Task B - pid 123
> > >> Task C - pid 121
> > >> Task D - pid 124
> > >>
> > >> Here D and B just spin all the time. C is lock owner (in-kernel mute=
x) and
> > >> spins all the time, while A blocks on the same in-kernel mutex and r=
emains
> > >> blocked.
> > >>
> > >> Then I did "top -H" while the test was running which gives below out=
put.
> > >> The first column is PID, and the third-last column is CPU percentage=
.
> > >>
> > >> Without PE:
> > >>  121 root      20   0   99496   4   0 R  33.6   0.0   0:02.76 t  (ta=
sk C)
> > >>  123 root      20   0   99496   4   0 R  33.2   0.0   0:02.75 t  (ta=
sk B)
> > >>  124 root      20   0   99496   4   0 R  33.2   0.0   0:02.75 t  (ta=
sk D)
> > >>
> > >> With PE:
> > >>  PID
> > >>  122 root      20   0   99496   4   0 D  25.3   0.0   0:22.21 t  (ta=
sk A)
> > >>  121 root      20   0   99496   4   0 R  25.0   0.0   0:22.20 t  (ta=
sk C)
> > >>  123 root      20   0   99496   4   0 R  25.0   0.0   0:22.20 t  (ta=
sk B)
> > >>  124 root      20   0   99496   4   0 R  25.0   0.0   0:22.20 t  (ta=
sk D)
> > >>
> > >> With PE, I was expecting 2 threads with 25% and 1 thread with 50%. I=
nstead I
> > >> get 4 threads with 25% in the top. Ftrace confirms that the D-state =
task is
> > >> in fact not running and proxying to the owner task so everything see=
ms
> > >> working correctly, but the accounting seems confusing, as in, it is =
confusing
> > >> to see the D-state task task taking 25% CPU when it is obviously "sl=
eeping".
> > >>
> > >> Yeah, yeah, I know D is proxying for C (while being in the uninterru=
ptible
> > >> sleep state), so may be it is OK then, but I did want to bring this =
up :-)
> > >
> > > I seem to remember Valentin raised similar issue about how userspace =
view can
> > > get confusing/misleading:
> > >
> > >    https://www.youtube.com/watch?v=3DUQNOT20aCEg&t=3D3h21m41s
> >
> > Thanks for the pointer! Glad to see the consensus was that this is not
> > acceptable.
> >
> > I think we ought to write a patch to fix the accounting, for this
> > series. I propose adding 2 new entries to proc/pid/stat which I think
> > Juri was also sort of was alluding to:
> >
> > 1. Donated time.
> > 2. Proxied time.
>
> Sounds like a useful addition, at least from a debugging point of view.
>
> > User space can then add or subtract this, to calculate things
> > correctly. Or just display them in new columns. I think it will also
> > actually show how much the proxying is happening for a use case.
>
> Guess we'll however need to be backward compatible with old userspace?
> Probably reporting the owner as running while proxied (as in the
> comparison case vs. rtmutexes Valentin showed).

Hi Juri,
Yes I was thinking of leaving the old metrics alone and just providing
the new ones as additional fields in /proc/pid/stats . Then the tools
adjust as needed with the new information. From kernel PoV we provide
the maximum information.

Thanks,

 - Joel