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;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Joel Fernandes <joel@joelfernandes.org>
Mime-Version: 1.0 (1.0)
Subject: Re: [RFC PATCH 00/11] Reviving the Proxy Execution Series
Date:   Thu, 20 Oct 2022 04:51:17 -0400
Message-Id: <158BE4D6-039A-4D0B-B4DA-0FF7E35BCA1A@joelfernandes.org>
References: <20221019193015.mczb4ew2m4h2qjjy@airbuntu>
Cc:     Juri Lelli <juri.lelli@redhat.com>,
        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>
In-Reply-To: <20221019193015.mczb4ew2m4h2qjjy@airbuntu>
To:     Qais Yousef <qyousef@layalina.io>
Precedence: bulk


> On Oct 19, 2022, at 3:30 PM, Qais Yousef <qyousef@layalina.io> wrote:
>=20
> =EF=BB=BFOn 10/19/22 15:41, Juri Lelli wrote:
>>> On 19/10/22 08:23, Joel Fernandes wrote:
>>>=20
>>>=20
>>>> On Oct 19, 2022, at 7:43 AM, Qais Yousef <qyousef@layalina.io> wrote:
>>>>=20
>>>> =EF=BB=BFOn 10/17/22 02:23, Joel Fernandes wrote:
>>>>=20
>>>>> I ran a test to check CFS time sharing. The accounting on top is confu=
sing,
>>>>> but ftrace confirms the proxying happening.
>>>>>=20
>>>>> Task A - pid 122
>>>>> Task B - pid 123
>>>>> Task C - pid 121
>>>>> Task D - pid 124
>>>>>=20
>>>>> Here D and B just spin all the time. C is lock owner (in-kernel mutex)=
 and
>>>>> spins all the time, while A blocks on the same in-kernel mutex and rem=
ains
>>>>> blocked.
>>>>>=20
>>>>> Then I did "top -H" while the test was running which gives below outpu=
t.
>>>>> The first column is PID, and the third-last column is CPU percentage.
>>>>>=20
>>>>> Without PE:
>>>>> 121 root      20   0   99496   4   0 R  33.6   0.0   0:02.76 t  (task C=
)
>>>>> 123 root      20   0   99496   4   0 R  33.2   0.0   0:02.75 t  (task B=
)
>>>>> 124 root      20   0   99496   4   0 R  33.2   0.0   0:02.75 t  (task D=
)
>>>>>=20
>>>>> With PE:
>>>>> PID
>>>>> 122 root      20   0   99496   4   0 D  25.3   0.0   0:22.21 t  (task A=
)
>>>>> 121 root      20   0   99496   4   0 R  25.0   0.0   0:22.20 t  (task C=
)
>>>>> 123 root      20   0   99496   4   0 R  25.0   0.0   0:22.20 t  (task B=
)
>>>>> 124 root      20   0   99496   4   0 R  25.0   0.0   0:22.20 t  (task D=
)
>>>>>=20
>>>>> With PE, I was expecting 2 threads with 25% and 1 thread with 50%. Ins=
tead I
>>>>> get 4 threads with 25% in the top. Ftrace confirms that the D-state ta=
sk is
>>>>> in fact not running and proxying to the owner task so everything seems=

>>>>> working correctly, but the accounting seems confusing, as in, it is co=
nfusing
>>>>> to see the D-state task task taking 25% CPU when it is obviously "slee=
ping".
>>>>>=20
>>>>> Yeah, yeah, I know D is proxying for C (while being in the uninterrupt=
ible
>>>>> sleep state), so may be it is OK then, but I did want to bring this up=
 :-)
>>>>=20
>>>> I seem to remember Valentin raised similar issue about how userspace vi=
ew can
>>>> get confusing/misleading:
>>>>=20
>>>>   https://www.youtube.com/watch?v=3DUQNOT20aCEg&t=3D3h21m41s
>>>=20
>>> Thanks for the pointer! Glad to see the consensus was that this is not
>>> acceptable.
>>>=20
>>> 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:
>>>=20
>>> 1. Donated time.
>>> 2. Proxied time.
>>=20
>> Sounds like a useful addition, at least from a debugging point of view.
>=20
> They look useful addition to me too.

Thanks.

>>> 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.
>>=20
>> 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).
>>=20
>=20
> Or invent a new task_state? Doesn't have to be a real one, just report a n=
ew
> letter for tasks in PE state. We could use 'r' to indicate running BUT..

This is a good idea, especially for tracing. I still feel the time taken in t=
he state
is also important to add so that top displays percentage properly.

 Best,

 -J

> Cheers
>=20
> --
> Qais Yousef