sched: Make schedstats a runtime tunable that is disabled by default

schedstats is very useful during debugging and performance tuning but it incurs overhead. As such, even though it can be disabled at build time, it is often enabled as the information is useful. This patch adds a kernel command-line and sysctl tunable to enable or disable schedstats on demand. It is disabled by default as someone who knows they need it can also learn to enable it when necessary.

Quoted 6 lines · Mel Gorman

schedstats is very useful during debugging and performance tuning but it incurs overhead. As such, even though it can be disabled at build time, it is often enabled as the information is useful. This patch adds a kernel command-line and sysctl tunable to enable or disable schedstats on demand. It is disabled by default as someone who knows they need it can also learn to enable it when necessary.

So the reason its often enabled in distro configs is (IIRC) that it enables trace_sched_stat_{wait,sleep,iowait,blocked}().

I've not looked at the details of this patch, but I suspect this patch would make these tracepoints available but non-functional unless you poke the magic button.

Quoted 8 lines · Peter Zijlstra

Quoted 6 lines · Mel Gorman

schedstats is very useful during debugging and performance tuning but it incurs overhead. As such, even though it can be disabled at build time, it is often enabled as the information is useful. This patch adds a kernel command-line and sysctl tunable to enable or disable schedstats on demand. It is disabled by default as someone who knows they need it can also learn to enable it when necessary.

Quoted 7 lines

So the reason its often enabled in distro configs is (IIRC) that it enables trace_sched_stat_{wait,sleep,iowait,blocked}().

I've not looked at the details of this patch, but I suspect this patch would make these tracepoints available but non-functional unless you poke the magic button.

It's potentially slightly worse than that. The tracepoints are available, functional but produce garbage unless the magic button is poked and do a lot of useful work producing that garbage. I missed a few hunks that are included below. With this, the tracepoints will exist but unless the magic button is poked, they'll never fire. Considering the paths affected, this will require retesting but if it's ok, would you be ok in general with a patch like this that forces a button to be pushed if the user is doing performance analysis?

Quoted 17 lines · Mel Gorman

Quoted 8 lines · Peter Zijlstra

Quoted 6 lines · Mel Gorman

schedstats is very useful during debugging and performance tuning but it incurs overhead. As such, even though it can be disabled at build time, it is often enabled as the information is useful. This patch adds a kernel command-line and sysctl tunable to enable or disable schedstats on demand. It is disabled by default as someone who knows they need it can also learn to enable it when necessary.

Quoted 7 lines

So the reason its often enabled in distro configs is (IIRC) that it enables trace_sched_stat_{wait,sleep,iowait,blocked}().

I've not looked at the details of this patch, but I suspect this patch would make these tracepoints available but non-functional unless you poke the magic button.

Quoted 8 lines

It's potentially slightly worse than that. The tracepoints are available, functional but produce garbage unless the magic button is poked and do a lot of useful work producing that garbage. I missed a few hunks that are included below. With this, the tracepoints will exist but unless the magic button is poked, they'll never fire. Considering the paths affected, this will require retesting but if it's ok, would you be ok in general with a patch like this that forces a button to be pushed if the user is doing performance analysis?

Its rather unintuitive and error prone semantics :/

Ideally we'd auto-magically enable the magic knob if any of these affected tracepoints become active. Or alternatively fail to enable the tracepoints (which would then get us people going: 'WTF this used to work').

One of the things on my TODO is look at how much of sched_stat is required for these tracepoints and see if we can enable just that (hopefully) little bit, while not doing the rest of the accounting.

Of course, it'll be our luck that tracking the data for these tracepoints is the most expensive part of schedstats ...

Ingo?

Quoted 4 lines

Of course, it'll be our luck that tracking the data for these tracepoints is the most expensive part of schedstats ...

Ingo?

IIRC it needed only a small subset of schedstats to make those tracepoints work.

We already have too much overhead in the scheduler as-is - and the extra cache footprint does not even show on the typically cache-rich enterprise CPUs most of the scalability testing goes on.

My minimum requirement for such runtime enablement would be to make it entirely static-branch patched and triggered at the call sites as well - not hidden inside schedstat functions.

Quoted 7 lines · Ingo Molnar

Quoted 4 lines

Of course, it'll be our luck that tracking the data for these tracepoints is the most expensive part of schedstats ...

Ingo?

IIRC it needed only a small subset of schedstats to make those tracepoints work.

Quoted 8 lines

We already have too much overhead in the scheduler as-is - and the extra cache footprint does not even show on the typically cache-rich enterprise CPUs most of the scalability testing goes on.

My minimum requirement for such runtime enablement would be to make it entirely static-branch patched and triggered at the call sites as well - not hidden inside schedstat functions.

As it is, it's static-branch patched but I'm struggling to see why they cannot be hidden in the schedstat_* functions which are just preprocessor macros. The checks could be put in the callsites but it's a lot of updates and I don't think the end result would be very nice to read.

Quoted 17 lines · Mel Gorman

Quoted 7 lines · Ingo Molnar

Quoted 4 lines

Of course, it'll be our luck that tracking the data for these tracepoints is the most expensive part of schedstats ...

Ingo?

IIRC it needed only a small subset of schedstats to make those tracepoints work.

Quoted 8 lines

We already have too much overhead in the scheduler as-is - and the extra cache footprint does not even show on the typically cache-rich enterprise CPUs most of the scalability testing goes on.

My minimum requirement for such runtime enablement would be to make it entirely static-branch patched and triggered at the call sites as well - not hidden inside schedstat functions.

Quoted 4 lines

As it is, it's static-branch patched but I'm struggling to see why they cannot be hidden in the schedstat_* functions which are just preprocessor macros. The checks could be put in the callsites but it's a lot of updates and I don't think the end result would be very nice to read.

So I was judging by:

kernel/sched/fair.c +5 -1

@@ -755,7 +755,12 @@ static void
 update_stats_wait_end(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
        struct task_struct *p;
-       u64 delta = rq_clock(rq_of(cfs_rq)) - se->statistics.wait_start;
+       u64 delta;
+
+       if (static_branch_unlikely(&sched_schedstats))
+               return;
+
 

which puts a static branch inside a real function, not preprocessor macros.

The vast majority of the checks are in the schedstat_[inc|dec|set) helpers. update_stats_wait_end() is an oddity in that it's a large function that only exists in schedstat and does a number of calculations. Ideally update_stats_wait would be updated too. In the next revision, I'll create a schedstats_enabled() helper that is an alias of static_branch_unlikely() and returns 0 if stats are not configured in for use at the call-site of large functions like this. That has the added benefit of avoiding any function call overhead.

Quoted 17 lines · Mel Gorman

Quoted 8 lines · Peter Zijlstra

Quoted 6 lines · Mel Gorman

schedstats is very useful during debugging and performance tuning but it incurs overhead. As such, even though it can be disabled at build time, it is often enabled as the information is useful. This patch adds a kernel command-line and sysctl tunable to enable or disable schedstats on demand. It is disabled by default as someone who knows they need it can also learn to enable it when necessary.

Quoted 7 lines

So the reason its often enabled in distro configs is (IIRC) that it enables trace_sched_stat_{wait,sleep,iowait,blocked}().

I've not looked at the details of this patch, but I suspect this patch would make these tracepoints available but non-functional unless you poke the magic button.

Quoted 8 lines

It's potentially slightly worse than that. The tracepoints are available, functional but produce garbage unless the magic button is poked and do a lot of useful work producing that garbage. I missed a few hunks that are included below. With this, the tracepoints will exist but unless the magic button is poked, they'll never fire. Considering the paths affected, this will require retesting but if it's ok, would you be ok in general with a patch like this that forces a button to be pushed if the user is doing performance analysis?

Its rather unintuitive and error prone semantics :/

Ideally we'd auto-magically enable the magic knob if any of these affected tracepoints become active.

Or alternatively fail to enable the
tracepoints (which would then get us people going: 'WTF this used to work').

One of the things on my TODO is look at how much of sched_stat is required for these tracepoints and see if we can enable just that (hopefully) little bit, while not doing the rest of the accounting.

kernel/sched/core.c +7 -0

+static int __init setup_schedstats(char *str)
+{
+	int ret = 0;
+	if (!str)
+		goto out;
+
+	if (!strcmp(str, "enable")) {

Quoted 11 lines · Mel Gorman

kernel/sched/core.c +11 -0

+static int __init setup_schedstats(char *str)
+{
+	int ret = 0;
+	if (!str)
+		goto out;
+
+	if (!strcmp(str, "enable")) {
+		set_schedstats(true);
+		ret = 1;
+	} else if (!strcmp(str, "disable")) {
+		set_schedstats(false);

kernel/sched/core.c int sysctl_numa_balancing(struct ctl_table *table, int write, · line 2311

Quoted 6 lines

kernel/sched/core.c +6 -0

+		ret = 1;
+	}
+out:
+	if (!ret)
+		pr_warn("Unable to parse numa_balancing=\n");
+

Copy+paste error?