From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Qais Yousef <qais.yousef@arm.com>
Cc: vincent.donnefort@arm.com, mingo@redhat.com,
peterz@infradead.org, vincent.guittot@linaro.org,
linux-kernel@vger.kernel.org, valentin.schneider@arm.com,
Phil Auld <pauld@redhat.com>
Subject: Re: [PATCH v2] sched/debug: Add new tracepoint to track cpu_capacity
Date: Wed, 2 Sep 2020 12:44:42 +0200 [thread overview]
Message-ID: <58f5d2e8-493b-7ce1-6abd-57705e5ab437@arm.com> (raw)
In-Reply-To: <20200828172658.dxygk7j672gho4ax@e107158-lin.cambridge.arm.com>
+ Phil Auld <pauld@redhat.com>
On 28/08/2020 19:26, Qais Yousef wrote:
> On 08/28/20 19:10, Dietmar Eggemann wrote:
>> On 28/08/2020 12:27, Qais Yousef wrote:
>>> On 08/28/20 10:00, vincent.donnefort@arm.com wrote:
>>>> From: Vincent Donnefort <vincent.donnefort@arm.com>
[...]
>> Can you remind me why we have all these helper functions like
>> sched_trace_rq_cpu_capacity?
>
> struct rq is defined in kernel/sched/sched.h. It's not exported. Exporting
> these helper functions was the agreement to help modules trace internal info.
> By passing generic info you decouple the tracepoint from giving specific info
> and allow the modules to extract all the info they need from the same
> tracepoint. IE: if you need more than just cpu_capacity from this tracepoint,
> you can get that without having to continuously add extra arguments everytime
> you need an extra piece of info. Unless this info is not in the rq of course.
I think this decoupling is not necessary. The natural place for those
scheduler trace_event based on trace_points extension files is
kernel/sched/ and here the internal sched.h can just be included.
If someone really wants to build this as an out-of-tree module there is
an easy way to make kernel/sched/sched.h visible.
CFLAGS_sched_tp.o := -I$KERNEL_SRC/kernel/sched
all:
make -C $KERNEL_SRC M=$(PWD) modules
This allowed me to build our trace_event extension module (sched_tp.c,
sched_events.h) out-of-tree and I was able to get rid of all the
sched_trace_foo() functions (in fair.c, include/linux/sched.h) and code
there content directly in foo.c
There are two things we would need exported from the kernel:
(1) cfs_rq_tg_path() to print the path of a taskgroup cfs_rq or se.
(2) sched_uclamp_used so uclamp_rq_util_with() can be used in
sched_events.h.
I put Phil Auld on cc because of his trace_point
sched_update_nr_running_tp. I think Phil was using sched_tp as a base so
I can't see an issue why we can't also remove sched_trace_rq_nr_running().
>> In case we would let the extra code (which transforms trace points into
>> trace events) know the internals of struct rq we could handle those
>> things in the TRACE_EVENT and/or the register_trace_##name(void
>> (*probe)(data_proto), void *data) thing.
>> We always said when the internal things will change this extra code will
>> break. So that's not an issue.
>
> The problem is that you need to export struct rq in a public header. Which we
> don't want to do. I have been trying to find out how to use BTF so we can
> remove these functions. Haven't gotten far away yet - but it should be doable
> and it's a question of me finding enough time to understand what was currently
> done and if I can re-use something or need to come up with extra infrastructure
> first.
Let's keep the footprint of these trace points as small as possible in
the scheduler code.
I'm putting the changes I described above in our monthly EAS integration
right now and when this worked out nicely I will share the patches on lkml.
next prev parent reply other threads:[~2020-09-02 10:44 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-28 9:00 [PATCH v2] sched/debug: Add new tracepoint to track cpu_capacity vincent.donnefort
2020-08-28 10:27 ` Qais Yousef
2020-08-28 17:10 ` Dietmar Eggemann
2020-08-28 17:26 ` Qais Yousef
2020-09-02 10:44 ` Dietmar Eggemann [this message]
2020-09-02 13:54 ` Phil Auld
2020-09-07 11:02 ` Qais Yousef
2020-09-08 13:19 ` Phil Auld
2020-09-08 15:22 ` Qais Yousef
2021-01-04 18:26 ` Qais Yousef
2021-01-04 18:59 ` Alexei Starovoitov
2021-01-05 11:38 ` Qais Yousef
2021-01-05 16:44 ` Alexei Starovoitov
2021-01-06 11:27 ` Qais Yousef
2021-01-06 23:42 ` Andrii Nakryiko
2021-01-07 11:23 ` Qais Yousef
2021-01-11 14:04 ` Peter Zijlstra
2021-01-11 14:08 ` Qais Yousef
2020-09-07 10:48 ` Qais Yousef
2020-09-07 11:13 ` peterz
2020-09-07 14:51 ` Qais Yousef
2020-09-08 11:17 ` Dietmar Eggemann
2020-09-08 15:17 ` Qais Yousef
2020-09-08 16:17 ` Dietmar Eggemann
2021-01-04 15:18 ` Qais Yousef
2020-10-05 7:43 ` [tip: sched/core] " tip-bot2 for Vincent Donnefort
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=58f5d2e8-493b-7ce1-6abd-57705e5ab437@arm.com \
--to=dietmar.eggemann@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pauld@redhat.com \
--cc=peterz@infradead.org \
--cc=qais.yousef@arm.com \
--cc=valentin.schneider@arm.com \
--cc=vincent.donnefort@arm.com \
--cc=vincent.guittot@linaro.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).