linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joel Fernandes <joel@joelfernandes.org>
To: Qais Yousef <qyousef@layalina.io>
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>
Subject: Re: [RFC PATCH 00/11] Reviving the Proxy Execution Series
Date: Thu, 20 Oct 2022 04:51:17 -0400	[thread overview]
Message-ID: <158BE4D6-039A-4D0B-B4DA-0FF7E35BCA1A@joelfernandes.org> (raw)
In-Reply-To: <20221019193015.mczb4ew2m4h2qjjy@airbuntu>



> On Oct 19, 2022, at 3:30 PM, Qais Yousef <qyousef@layalina.io> wrote:
> 
> On 10/19/22 15:41, Juri Lelli wrote:
>>> On 19/10/22 08:23, Joel Fernandes wrote:
>>> 
>>> 
>>>> On Oct 19, 2022, at 7:43 AM, Qais Yousef <qyousef@layalina.io> wrote:
>>>> 
>>>> On 10/17/22 02:23, Joel Fernandes wrote:
>>>> 
>>>>> I ran a test to check CFS time sharing. The accounting on top is confusing,
>>>>> 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 mutex) and
>>>>> spins all the time, while A blocks on the same in-kernel mutex and remains
>>>>> blocked.
>>>>> 
>>>>> Then I did "top -H" while the test was running which gives below output.
>>>>> 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  (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)
>>>>> 
>>>>> 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)
>>>>> 
>>>>> With PE, I was expecting 2 threads with 25% and 1 thread with 50%. Instead 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 seems
>>>>> 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 "sleeping".
>>>>> 
>>>>> Yeah, yeah, I know D is proxying for C (while being in the uninterruptible
>>>>> 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=UQNOT20aCEg&t=3h21m41s
>>> 
>>> 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.
> 
> 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.
>> 
>> 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).
>> 
> 
> Or invent a new task_state? Doesn't have to be a real one, just report a new
> 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 the state
is also important to add so that top displays percentage properly.

 Best,

 -J

> Cheers
> 
> --
> Qais Yousef

  reply	other threads:[~2022-10-20  8:52 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-03 21:44 [RFC PATCH 00/11] Reviving the Proxy Execution Series Connor O'Brien
2022-10-03 21:44 ` [RFC PATCH 01/11] locking/ww_mutex: Remove wakeups from under mutex::wait_lock Connor O'Brien
2022-10-04 16:01   ` Waiman Long
2022-10-12 23:54     ` Joel Fernandes
2022-10-20 18:43     ` Connor O'Brien
2022-10-03 21:44 ` [RFC PATCH 02/11] locking/mutex: Rework task_struct::blocked_on Connor O'Brien
2022-10-03 21:44 ` [RFC PATCH 03/11] kernel/locking: Add p->blocked_on wrapper Connor O'Brien
2022-10-03 21:44 ` [RFC PATCH 04/11] locking/mutex: make mutex::wait_lock irq safe Connor O'Brien
2022-10-13  4:30   ` Joel Fernandes
2022-10-03 21:44 ` [RFC PATCH 05/11] sched: Split scheduler execution context Connor O'Brien
2022-10-14 17:01   ` Joel Fernandes
2022-10-19 17:17   ` Valentin Schneider
2022-10-20 18:43     ` Connor O'Brien
2022-10-03 21:44 ` [RFC PATCH 06/11] kernel/locking: Expose mutex_owner() Connor O'Brien
2022-10-03 21:44 ` [RFC PATCH 07/11] sched: Add proxy execution Connor O'Brien
2022-10-12  1:54   ` Joel Fernandes
2022-10-12  9:46     ` Juri Lelli
2022-10-14 17:07     ` Joel Fernandes
2022-10-15 13:53     ` Peter Zijlstra
2022-10-16 20:48       ` Steven Rostedt
2022-10-17  4:03         ` Joel Fernandes
2022-10-17  7:26         ` Peter Zijlstra
2022-10-24 22:33           ` Qais Yousef
2022-10-25 11:19             ` Joel Fernandes
2022-10-25 22:10               ` Qais Yousef
2022-10-15 15:28     ` Peter Zijlstra
2022-10-15 15:08   ` Peter Zijlstra
2022-10-15 15:10   ` Peter Zijlstra
2022-10-15 15:47   ` Peter Zijlstra
2022-10-24 10:13   ` Dietmar Eggemann
2022-10-29  3:31     ` Joel Fernandes
2022-10-31 16:39       ` Dietmar Eggemann
2022-10-31 18:00         ` Joel Fernandes
2022-11-04 17:09           ` Dietmar Eggemann
2022-11-21  0:22             ` Joel Fernandes
2022-11-21  1:49               ` Joel Fernandes
2022-11-21  3:59                 ` Joel Fernandes
2022-11-22 18:45                   ` Joel Fernandes
2023-01-09  8:51   ` Chen Yu
2022-10-03 21:44 ` [RFC PATCH 08/11] sched: Fixup task CPUs for potential proxies Connor O'Brien
2022-10-03 21:44 ` [RFC PATCH 09/11] sched/rt: Fix proxy/current (push,pull)ability Connor O'Brien
2022-10-10 11:40   ` Valentin Schneider
2022-10-14 22:32     ` Connor O'Brien
2022-10-19 17:05       ` Valentin Schneider
2022-10-20 13:30         ` Juri Lelli
2022-10-20 16:14           ` Valentin Schneider
2022-10-21  2:22         ` Connor O'Brien
2022-10-03 21:45 ` [RFC PATCH 10/11] torture: support randomized shuffling for proxy exec testing Connor O'Brien
2022-11-12 16:54   ` Joel Fernandes
2022-11-14 20:44     ` Connor O'Brien
2022-11-15 16:02       ` Joel Fernandes
2022-10-03 21:45 ` [RFC PATCH 11/11] locktorture: support nested mutexes Connor O'Brien
2022-10-06  9:59 ` [RFC PATCH 00/11] Reviving the Proxy Execution Series Juri Lelli
2022-10-06 10:07   ` Peter Zijlstra
2022-10-06 12:14     ` Juri Lelli
2022-10-15 15:44 ` Peter Zijlstra
2022-10-17  2:23 ` Joel Fernandes
2022-10-19 11:43   ` Qais Yousef
2022-10-19 12:23     ` Joel Fernandes
2022-10-19 13:41       ` Juri Lelli
2022-10-19 13:51         ` Joel Fernandes
2022-10-19 19:30         ` Qais Yousef
2022-10-20  8:51           ` Joel Fernandes [this message]
2022-10-17  3:25 ` Chengming Zhou
2022-10-17  3:56   ` Joel Fernandes
2022-10-17  4:26     ` Chengming Zhou
2022-10-17 12:27       ` Joel Fernandes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=158BE4D6-039A-4D0B-B4DA-0FF7E35BCA1A@joelfernandes.org \
    --to=joel@joelfernandes.org \
    --cc=boqun.feng@gmail.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=connoro@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=jstultz@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=qais.yousef@arm.com \
    --cc=qyousef@layalina.io \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).