2022-11-15 17:25:37

by Vincent Guittot

Subject: [PATCH v9 0/9] Add latency priority for CFS class

This patchset restarts the work about adding a latency priority to describe
the latency tolerance of cfs tasks.

Patch [1] is a new one that has been added with v6. It fixes an
unfairness for low prio tasks because of wakeup_gran() being bigger
than the maximum vruntime credit that a waking task can keep after
sleeping.

The patches [2-4] have been done by Parth:
https://lore.kernel.org/lkml/[email protected]/

I have just rebased and moved the set of latency priority outside the
priority update. I have removed the reviewed tag because the patches
are 2 years old.

This aims to be a generic interface and the following patches is one use
of it to improve the scheduling latency of cfs tasks.

Patch [5] uses latency nice priority to define a latency offset
and then decide if a cfs task can or should preempt the current
running task. The patch gives some tests results with cyclictests and
hackbench to highlight the benefit of latency priority for short
interactive task or long intensive tasks.

Patch [6] adds the support of latency nice priority to task group by
adding a cpu.latency.nice field. The range is [-20:19] as for setting task
latency priority.

Patch [7] makes sched_core taking into account the latency offset.

Patch [8] adds a rb tree to cover some corner cases where the latency
sensitive task (priority < 0) is preempted by high priority task (RT/DL)
or fails to preempt them. This patch ensures that tasks will have at least
a slice of sched_min_granularity in priority at wakeup.

Patch [9] removes useless check after adding a latency rb tree.

I have also backported the patchset on a dragonboard RB3 with an android
mainline kernel based on v5.18 for a quick test. I have used the
TouchLatency app which is part of AOSP and described to be a very good
test to highlight jitter and jank frame sources of a system [1].
In addition to the app, I have added some short running tasks waking-up
regularly (to use the 8 cpus for 4 ms every 37777us) to stress the system
without overloading it (and disabling EAS). The 1st results shows that the
patchset helps to reduce the missed deadline frames from 5% to less than
0.1% when the cpu.latency.nice of task group are set. I haven't rerun the
test with latest version.

I have also tested the patchset with the modified version of the alsa
latency test that has been shared by Tim. The test quickly xruns with
default latency nice priority 0 but is able to run without underuns with
a latency -20 and hackbench running simultaneously.

While preparing the version 8, I have evaluated the benefit of using an
augmented rbtree instead of adding a rbtree for latency sensitive entities,
which was a relevant suggestion done by PeterZ. Although the augmented
rbtree enables to sort additional information in the tree with a limited
overhead, it has more impact on legacy use cases (latency_nice >= 0)
because the augmented callbacks are always called to maintain this
additional information even when there is no sensitive tasks. In such
cases, the dedicated rbtree remains empty and the overhead is reduced to
loading a cached null node pointer. Nevertheless, we might want to
reconsider the augmented rbtree once the use of negative latency_nice will
be more widlely deployed. At now, the different tests that I have done,
have not shown improvements with augmented rbtree.

Below are some hackbench results:
2 rbtrees augmented rbtree augmented rbtree
sorted by vruntime sorted by wakeup_vruntime
sched pipe
avg 26311,000 25976,667 25839,556
stdev 0,15 % 0,28 % 0,24 %
vs tip 0,50 % -0,78 % -1,31 %
hackbench 1 group
avg 1,315 1,344 1,359
stdev 0,88 % 1,55 % 1,82 %
vs tip -0,47 % -2,68 % -3,87 %
hackbench 4 groups
avg 1,339 1,365 1,367
stdev 2,39 % 2,26 % 3,58 %
vs tip -0,08 % -2,01 % -2,22 %
hackbench 8 groups
avg 1,233 1,286 1,301
stdev 0,74 % 1,09 % 1,52 %
vs tip 0,29 % -4,05 % -5,27 %
hackbench 16 groups
avg 1,268 1,313 1,319
stdev 0,85 % 1,60 % 0,68 %
vs tip -0,02 % -3,56 % -4,01 %

[1] https://source.android.com/docs/core/debug/eval_perf#touchlatency

Change since v8:
- Rename get_sched_latency by get_sleep_latency
- move latency nice defines in sched/prio.h and fix latency_prio init value
- Fix typo and comments

Change since v7:
- Replaced se->on_latency by using RB_CLEAR_NODE() and RB_EMPTY_NODE()
- Clarify the limit behavior fo the cgroup cpu.latenyc_nice

Change since v6:
- Fix compilation error for !CONFIG_SCHED_DEBUG

Change since v5:
- Add patch 1 to fix unfairness for low prio task. This has been
discovered while studying Youssef's tests results with latency nice
which were hitting the same problem.
- Fixed latency_offset computation to take into account
GENTLE_FAIR_SLEEPERS. This has diseappeared with v2and has been raised
by Youssef's tests.
- Reworked and optimized how latency_offset in used to check for
preempting current task at wakeup and tick. This cover more cases too.
- Add patch 9 to remove check_preempt_from_others() which is not needed
anymore with the rb tree.

Change since v4:
- Removed permission checks to set latency priority. This enables user
without elevated privilege like audio application to set their latency
priority as requested by Tim.
- Removed cpu.latency and replaced it by cpu.latency.nice so we keep a
generic interface not tied to latency_offset which can be used to
implement other latency features.
- Added an entry in Documentation/admin-guide/cgroup-v2.rst to describe
cpu.latency.nice.
- Fix some typos.

Change since v3:
- Fix 2 compilation warnings raised by kernel test robot <[email protected]>

Change since v2:
- Set a latency_offset field instead of saving a weight and computing it
on the fly.
- Make latency_offset available for task group: cpu.latency
- Fix some corner cases to make latency sensitive tasks schedule first and
add a rb tree for latency sensitive task.

Change since v1:
- fix typo
- move some codes in the right patch to make bisect happy
- simplify and fixed how the weight is computed
- added support of sched core patch 7

Parth Shah (3):
sched: Introduce latency-nice as a per-task attribute
sched/core: Propagate parent task's latency requirements to the child
task
sched: Allow sched_{get,set}attr to change latency_nice of the task

Vincent Guittot (6):
sched/fair: fix unfairness at wakeup
sched/fair: Take into account latency priority at wakeup
sched/fair: Add sched group latency support
sched/core: Support latency priority with sched core
sched/fair: Add latency list
sched/fair: remove check_preempt_from_others

Documentation/admin-guide/cgroup-v2.rst | 10 ++
include/linux/sched.h | 4 +
include/linux/sched/prio.h | 27 +++
include/uapi/linux/sched.h | 4 +-
include/uapi/linux/sched/types.h | 19 +++
init/init_task.c | 1 +
kernel/sched/core.c | 106 ++++++++++++
kernel/sched/debug.c | 1 +
kernel/sched/fair.c | 209 ++++++++++++++++++++----
kernel/sched/sched.h | 45 ++++-
tools/include/uapi/linux/sched.h | 4 +-
11 files changed, 394 insertions(+), 36 deletions(-)

--
2.17.1

2022-11-15 17:30:15

by Vincent Guittot

Subject: [PATCH v9 2/9] sched: Introduce latency-nice as a per-task attribute

From: Parth Shah <[email protected]>

Latency-nice indicates the latency requirements of a task with respect
to the other tasks in the system. The value of the attribute can be within
the range of [-20, 19] both inclusive to be in-line with the values just
like task nice values.

latency_nice = -20 indicates the task to have the least latency as
compared to the tasks having latency_nice = +19.

The latency_nice may affect only the CFS SCHED_CLASS by getting
latency requirements from the userspace.

Additionally, add debugging bits for newly added latency_nice attribute.

Signed-off-by: Parth Shah <[email protected]>
[rebase, move defines in sched/prio.h]
Signed-off-by: Vincent Guittot <[email protected]>
---
include/linux/sched.h | 1 +
include/linux/sched/prio.h | 18 ++++++++++++++++++
kernel/sched/debug.c | 1 +
3 files changed, 20 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 23de7fe86cc4..856240573300 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -784,6 +784,7 @@ struct task_struct {
int static_prio;
int normal_prio;
unsigned int rt_priority;
+ int latency_nice;

struct sched_entity se;
struct sched_rt_entity rt;
diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
index ab83d85e1183..bfcd7f1d1e11 100644
--- a/include/linux/sched/prio.h
+++ b/include/linux/sched/prio.h
@@ -42,4 +42,22 @@ static inline long rlimit_to_nice(long prio)
return (MAX_NICE - prio + 1);
}

+/*
+ * Latency nice is meant to provide scheduler hints about the relative
+ * latency requirements of a task with respect to other tasks.
+ * Thus a task with latency_nice == 19 can be hinted as the task with no
+ * latency requirements, in contrast to the task with latency_nice == -20
+ * which should be given priority in terms of lower latency.
+ */
+#define MAX_LATENCY_NICE 19
+#define MIN_LATENCY_NICE -20
+
+#define LATENCY_NICE_WIDTH \
+ (MAX_LATENCY_NICE - MIN_LATENCY_NICE + 1)
+
+/*
+ * Default tasks should be treated as a task with latency_nice = 0.
+ */
+#define DEFAULT_LATENCY_NICE 0
+
#endif /* _LINUX_SCHED_PRIO_H */
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 1637b65ba07a..68be7a3e42a3 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -1043,6 +1043,7 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
#endif
P(policy);
P(prio);
+ P(latency_nice);
if (task_has_dl_policy(p)) {
P(dl.runtime);
P(dl.deadline);
--
2.17.1

2022-11-15 17:34:46

by Vincent Guittot

Subject: [PATCH 5/9] sched/fair: Take into account latency priority at wakeup

Take into account the latency priority of a thread when deciding to
preempt the current running thread. We don't want to provide more CPU
bandwidth to a thread but reorder the scheduling to run latency sensitive
task first whenever possible.

As long as a thread didn't use its bandwidth, it will be able to preempt
the current thread.

At the opposite, a thread with a low latency priority will preempt current
thread at wakeup only to keep fair CPU bandwidth sharing. Otherwise it will
wait for the tick to get its sched slice.

curr vruntime
|
sysctl_sched_wakeup_granularity
<-->
----------------------------------|----|-----------------------|---------------
| |<--------------------->
| . sysctl_sched_latency
| .
default/current latency entity | .
| .
1111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-
se preempts curr at wakeup ------>|<- se doesn't preempt curr -----------------
| .
| .
| .
low latency entity | .
---------------------->|
% of sysctl_sched_latency |
1111111111111111111111111111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-
preempt ------------------------------------------------->|<- do not preempt --
| .
| .
| .
high latency entity | .
|<-----------------------|----.
| % of sysctl_sched_latency .
111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1
preempt->|<- se doesn't preempt curr ------------------------------------------

Tests results of nice latency impact on heavy load like hackbench:

hackbench -l (2560 / group) -g group
group latency 0 latency 19
1 1.378(+/- 1%) 1.337(+/- 1%) + 3%
4 1.393(+/- 3%) 1.312(+/- 3%) + 6%
8 1.308(+/- 2%) 1.279(+/- 1%) + 2%
16 1.347(+/- 1%) 1.317(+/- 1%) + 2%

hackbench -p -l (2560 / group) -g group
group
1 1.836(+/- 17%) 1.148(+/- 5%) +37%
4 1.586(+/- 6%) 1.109(+/- 8%) +30%
8 1.209(+/- 4%) 0.780(+/- 4%) +35%
16 0.805(+/- 5%) 0.728(+/- 4%) +10%

By deacreasing the latency prio, we reduce the number of preemption at
wakeup and help hackbench making progress.

Test results of nice latency impact on short live load like cyclictest
while competing with heavy load like hackbench:

hackbench -l 10000 -g $group &
cyclictest --policy other -D 5 -q -n
latency 0 latency -20
group min avg max min avg max
0 16 19 29 17 18 29
1 43 299 7359 63 84 3422
4 56 449 14806 45 83 284
8 63 820 51123 63 83 283
16 64 1326 70684 41 157 26852

group = 0 means that hackbench is not running.

The avg is significantly improved with nice latency -20 especially with
large number of groups but min and max remain quite similar. If we add the
histogram parameter to get details of latency, we have :

hackbench -l 10000 -g 16 &
cyclictest --policy other -D 5 -q -n -H 20000 --histfile data.txt
latency 0 latency -20
Min Latencies: 64 62
Avg Latencies: 1170 107
Max Latencies: 88069 10417
50% latencies: 122 86
75% latencies: 614 91
85% latencies: 961 94
90% latencies: 1225 97
95% latencies: 6120 102
99% latencies: 18328 159

With percentile details, we see the benefit of nice latency -20 as
only 1% of the latencies are above 159us whereas the default latency
has got 15% around ~1ms or above and 5% over the 6ms.

Signed-off-by: Vincent Guittot <[email protected]>
---
include/linux/sched.h | 4 ++-
include/linux/sched/prio.h | 9 ++++++
init/init_task.c | 2 +-
kernel/sched/core.c | 38 +++++++++++++++++++---
kernel/sched/debug.c | 2 +-
kernel/sched/fair.c | 66 ++++++++++++++++++++++++++++++++++----
kernel/sched/sched.h | 6 ++++
7 files changed, 112 insertions(+), 15 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 856240573300..2f33326adb8d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -568,6 +568,8 @@ struct sched_entity {
/* cached value of my_q->h_nr_running */
unsigned long runnable_weight;
#endif
+ /* preemption offset in ns */
+ long latency_offset;

#ifdef CONFIG_SMP
/*
@@ -784,7 +786,7 @@ struct task_struct {
int static_prio;
int normal_prio;
unsigned int rt_priority;
- int latency_nice;
+ int latency_prio;

struct sched_entity se;
struct sched_rt_entity rt;
diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
index bfcd7f1d1e11..be79503d86af 100644
--- a/include/linux/sched/prio.h
+++ b/include/linux/sched/prio.h
@@ -59,5 +59,14 @@ static inline long rlimit_to_nice(long prio)
* Default tasks should be treated as a task with latency_nice = 0.
*/
#define DEFAULT_LATENCY_NICE 0
+#define DEFAULT_LATENCY_PRIO (DEFAULT_LATENCY_NICE + LATENCY_NICE_WIDTH/2)
+
+/*
+ * Convert user-nice values [ -20 ... 0 ... 19 ]
+ * to static latency [ 0..39 ],
+ * and back.
+ */
+#define NICE_TO_LATENCY(nice) ((nice) + DEFAULT_LATENCY_PRIO)
+#define LATENCY_TO_NICE(prio) ((prio) - DEFAULT_LATENCY_PRIO)

#endif /* _LINUX_SCHED_PRIO_H */
diff --git a/init/init_task.c b/init/init_task.c
index 7dd71dd2d261..071deff8dbd1 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -78,7 +78,7 @@ struct task_struct init_task
.prio = MAX_PRIO - 20,
.static_prio = MAX_PRIO - 20,
.normal_prio = MAX_PRIO - 20,
- .latency_nice = DEFAULT_LATENCY_NICE,
+ .latency_prio = DEFAULT_LATENCY_PRIO,
.policy = SCHED_NORMAL,
.cpus_ptr = &init_task.cpus_mask,
.user_cpus_ptr = NULL,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 18c31a68eb18..b2b8cb6c08cd 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1283,6 +1283,16 @@ static void set_load_weight(struct task_struct *p, bool update_load)
}
}

+static void set_latency_offset(struct task_struct *p)
+{
+ long weight = sched_latency_to_weight[p->latency_prio];
+ s64 offset;
+
+ offset = weight * get_sleep_latency(false);
+ offset = div_s64(offset, NICE_LATENCY_WEIGHT_MAX);
+ p->se.latency_offset = (long)offset;
+}
+
#ifdef CONFIG_UCLAMP_TASK
/*
* Serializes updates of utilization clamp values
@@ -4592,7 +4602,9 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
p->prio = p->normal_prio = p->static_prio;
set_load_weight(p, false);

- p->latency_nice = DEFAULT_LATENCY_NICE;
+ p->latency_prio = NICE_TO_LATENCY(0);
+ set_latency_offset(p);
+
/*
* We don't need the reset flag anymore after the fork. It has
* fulfilled its duty:
@@ -7358,8 +7370,10 @@ static void __setscheduler_params(struct task_struct *p,
static void __setscheduler_latency(struct task_struct *p,
const struct sched_attr *attr)
{
- if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE)
- p->latency_nice = attr->sched_latency_nice;
+ if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE) {
+ p->latency_prio = NICE_TO_LATENCY(attr->sched_latency_nice);
+ set_latency_offset(p);
+ }
}

/*
@@ -7544,7 +7558,7 @@ static int __sched_setscheduler(struct task_struct *p,
if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
goto change;
if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE &&
- attr->sched_latency_nice != p->latency_nice)
+ attr->sched_latency_nice != LATENCY_TO_NICE(p->latency_prio))
goto change;

p->sched_reset_on_fork = reset_on_fork;
@@ -8085,7 +8099,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
get_params(p, &kattr);
kattr.sched_flags &= SCHED_FLAG_ALL;

- kattr.sched_latency_nice = p->latency_nice;
+ kattr.sched_latency_nice = LATENCY_TO_NICE(p->latency_prio);

#ifdef CONFIG_UCLAMP_TASK
/*
@@ -11294,6 +11308,20 @@ const u32 sched_prio_to_wmult[40] = {
/* 15 */ 119304647, 148102320, 186737708, 238609294, 286331153,
};

+/*
+ * latency weight for wakeup preemption
+ */
+const int sched_latency_to_weight[40] = {
+ /* -20 */ -1024, -973, -922, -870, -819,
+ /* -15 */ -768, -717, -666, -614, -563,
+ /* -10 */ -512, -461, -410, -358, -307,
+ /* -5 */ -256, -205, -154, -102, -51,
+ /* 0 */ 0, 51, 102, 154, 205,
+ /* 5 */ 256, 307, 358, 410, 461,
+ /* 10 */ 512, 563, 614, 666, 717,
+ /* 15 */ 768, 819, 870, 922, 973,
+};
+
void call_trace_sched_update_nr_running(struct rq *rq, int count)
{
trace_sched_update_nr_running_tp(rq, count);
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 68be7a3e42a3..b3922184af91 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -1043,7 +1043,7 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
#endif
P(policy);
P(prio);
- P(latency_nice);
+ P(latency_prio);
if (task_has_dl_policy(p)) {
P(dl.runtime);
P(dl.deadline);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index c8a697f8db88..0e80e65113bd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4858,6 +4858,8 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
update_idle_cfs_rq_clock_pelt(cfs_rq);
}

+static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se);
+
/*
* Preempt the current task with a newly woken task if needed:
*/
@@ -4866,7 +4868,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
{
unsigned long ideal_runtime, delta_exec;
struct sched_entity *se;
- s64 delta;
+ s64 delta, offset;

ideal_runtime = sched_slice(cfs_rq, curr);
delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
@@ -4891,10 +4893,12 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
se = __pick_first_entity(cfs_rq);
delta = curr->vruntime - se->vruntime;

- if (delta < 0)
+ offset = wakeup_latency_gran(curr, se);
+ if (delta < offset)
return;

- if (delta > ideal_runtime)
+ if ((delta > ideal_runtime) ||
+ (delta > get_latency_max()))
resched_curr(rq_of(cfs_rq));
}

@@ -6019,6 +6023,35 @@ static int sched_idle_cpu(int cpu)
}
#endif

+static void set_next_buddy(struct sched_entity *se);
+
+static void check_preempt_from_others(struct cfs_rq *cfs, struct sched_entity *se)
+{
+ struct sched_entity *next;
+
+ if (se->latency_offset >= 0)
+ return;
+
+ if (cfs->nr_running <= 1)
+ return;
+ /*
+ * When waking from another class, we don't need to check to preempt at
+ * wakeup and don't set next buddy as a candidate for being picked in
+ * priority.
+ * In case of simultaneous wakeup when current is another class, the
+ * latency sensitive tasks lost opportunity to preempt non sensitive
+ * tasks which woke up simultaneously.
+ */
+
+ if (cfs->next)
+ next = cfs->next;
+ else
+ next = __pick_first_entity(cfs);
+
+ if (next && wakeup_preempt_entity(next, se) == 1)
+ set_next_buddy(se);
+}
+
/*
* The enqueue_task method is called before nr_running is
* increased. Here we update the fair scheduling stats and
@@ -6105,14 +6138,15 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
if (!task_new)
update_overutilized_status(rq);

+ if (rq->curr->sched_class != &fair_sched_class)
+ check_preempt_from_others(cfs_rq_of(&p->se), &p->se);
+
enqueue_throttle:
assert_list_leaf_cfs_rq(rq);

hrtick_update(rq);
}

-static void set_next_buddy(struct sched_entity *se);
-
/*
* The dequeue_task method is called before nr_running is
* decreased. We remove the task from the rbtree and
@@ -7461,6 +7495,23 @@ balance_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
}
#endif /* CONFIG_SMP */

+static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se)
+{
+ long latency_offset = se->latency_offset;
+
+ /*
+ * A negative latency offset means that the sched_entity has latency
+ * requirement that needs to be evaluated versus other entity.
+ * Otherwise, use the latency weight to evaluate how much scheduling
+ * delay is acceptable by se.
+ */
+ if ((latency_offset < 0) || (curr->latency_offset < 0))
+ latency_offset -= curr->latency_offset;
+ latency_offset = min_t(long, latency_offset, get_latency_max());
+
+ return latency_offset;
+}
+
static unsigned long wakeup_gran(struct sched_entity *se)
{
unsigned long gran = sysctl_sched_wakeup_granularity;
@@ -7499,11 +7550,12 @@ static int
wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se)
{
s64 gran, vdiff = curr->vruntime - se->vruntime;
+ s64 offset = wakeup_latency_gran(curr, se);

- if (vdiff <= 0)
+ if (vdiff < offset)
return -1;

- gran = wakeup_gran(se);
+ gran = offset + wakeup_gran(se);

/*
* At wake up, the vruntime of a task is capped to not be older than
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 842ce0094d9c..7292652731d0 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -125,6 +125,11 @@ extern int sched_rr_timeslice;
*/
#define NS_TO_JIFFIES(TIME) ((unsigned long)(TIME) / (NSEC_PER_SEC / HZ))

+/* Maximum nice latency weight used to scale the latency_offset */
+
+#define NICE_LATENCY_SHIFT (SCHED_FIXEDPOINT_SHIFT)
+#define NICE_LATENCY_WEIGHT_MAX (1L << NICE_LATENCY_SHIFT)
+
/*
* Increase resolution of nice-level calculations for 64-bit architectures.
* The extra resolution improves shares distribution and load balancing of
@@ -2115,6 +2120,7 @@ static_assert(WF_TTWU == SD_BALANCE_WAKE);

extern const int sched_prio_to_weight[40];
extern const u32 sched_prio_to_wmult[40];
+extern const int sched_latency_to_weight[40];

/*
* {de,en}queue flags:
--
2.17.1

2022-11-15 17:36:08

by Vincent Guittot

Subject: [PATCH v9 4/9] sched: Allow sched_{get,set}attr to change latency_nice of the task

From: Parth Shah <[email protected]>

Introduce the latency_nice attribute to sched_attr and provide a
mechanism to change the value with the use of sched_setattr/sched_getattr
syscall.

Also add new flag "SCHED_FLAG_LATENCY_NICE" to hint the change in
latency_nice of the task on every sched_setattr syscall.

Signed-off-by: Parth Shah <[email protected]>
[rebase and add a dedicated __setscheduler_latency ]
Signed-off-by: Vincent Guittot <[email protected]>
---
include/uapi/linux/sched.h | 4 +++-
include/uapi/linux/sched/types.h | 19 +++++++++++++++++++
kernel/sched/core.c | 24 ++++++++++++++++++++++++
tools/include/uapi/linux/sched.h | 4 +++-
4 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
index 3bac0a8ceab2..b2e932c25be6 100644
--- a/include/uapi/linux/sched.h
+++ b/include/uapi/linux/sched.h
@@ -132,6 +132,7 @@ struct clone_args {
#define SCHED_FLAG_KEEP_PARAMS 0x10
#define SCHED_FLAG_UTIL_CLAMP_MIN 0x20
#define SCHED_FLAG_UTIL_CLAMP_MAX 0x40
+#define SCHED_FLAG_LATENCY_NICE 0x80

#define SCHED_FLAG_KEEP_ALL (SCHED_FLAG_KEEP_POLICY | \
SCHED_FLAG_KEEP_PARAMS)
@@ -143,6 +144,7 @@ struct clone_args {
SCHED_FLAG_RECLAIM | \
SCHED_FLAG_DL_OVERRUN | \
SCHED_FLAG_KEEP_ALL | \
- SCHED_FLAG_UTIL_CLAMP)
+ SCHED_FLAG_UTIL_CLAMP | \
+ SCHED_FLAG_LATENCY_NICE)

#endif /* _UAPI_LINUX_SCHED_H */
diff --git a/include/uapi/linux/sched/types.h b/include/uapi/linux/sched/types.h
index f2c4589d4dbf..db1e8199e8c8 100644
--- a/include/uapi/linux/sched/types.h
+++ b/include/uapi/linux/sched/types.h
@@ -10,6 +10,7 @@ struct sched_param {

#define SCHED_ATTR_SIZE_VER0 48 /* sizeof first published struct */
#define SCHED_ATTR_SIZE_VER1 56 /* add: util_{min,max} */
+#define SCHED_ATTR_SIZE_VER2 60 /* add: latency_nice */

/*
* Extended scheduling parameters data structure.
@@ -98,6 +99,22 @@ struct sched_param {
* scheduled on a CPU with no more capacity than the specified value.
*
* A task utilization boundary can be reset by setting the attribute to -1.
+ *
+ * Latency Tolerance Attributes
+ * ===========================
+ *
+ * A subset of sched_attr attributes allows to specify the relative latency
+ * requirements of a task with respect to the other tasks running/queued in the
+ * system.
+ *
+ * @ sched_latency_nice task's latency_nice value
+ *
+ * The latency_nice of a task can have any value in a range of
+ * [MIN_LATENCY_NICE..MAX_LATENCY_NICE].
+ *
+ * A task with latency_nice with the value of LATENCY_NICE_MIN can be
+ * taken for a task requiring a lower latency as opposed to the task with
+ * higher latency_nice.
*/
struct sched_attr {
__u32 size;
@@ -120,6 +137,8 @@ struct sched_attr {
__u32 sched_util_min;
__u32 sched_util_max;

+ /* latency requirement hints */
+ __s32 sched_latency_nice;
};

#endif /* _UAPI_LINUX_SCHED_TYPES_H */
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8c84c652853b..18c31a68eb18 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7352,6 +7352,14 @@ static void __setscheduler_params(struct task_struct *p,
p->rt_priority = attr->sched_priority;
p->normal_prio = normal_prio(p);
set_load_weight(p, true);
+
+}
+
+static void __setscheduler_latency(struct task_struct *p,
+ const struct sched_attr *attr)
+{
+ if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE)
+ p->latency_nice = attr->sched_latency_nice;
}

/*
@@ -7494,6 +7502,13 @@ static int __sched_setscheduler(struct task_struct *p,
return retval;
}

+ if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE) {
+ if (attr->sched_latency_nice > MAX_LATENCY_NICE)
+ return -EINVAL;
+ if (attr->sched_latency_nice < MIN_LATENCY_NICE)
+ return -EINVAL;
+ }
+
if (pi)
cpuset_read_lock();

@@ -7528,6 +7543,9 @@ static int __sched_setscheduler(struct task_struct *p,
goto change;
if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
goto change;
+ if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE &&
+ attr->sched_latency_nice != p->latency_nice)
+ goto change;

p->sched_reset_on_fork = reset_on_fork;
retval = 0;
@@ -7616,6 +7634,7 @@ static int __sched_setscheduler(struct task_struct *p,
__setscheduler_params(p, attr);
__setscheduler_prio(p, newprio);
}
+ __setscheduler_latency(p, attr);
__setscheduler_uclamp(p, attr);

if (queued) {
@@ -7826,6 +7845,9 @@ static int sched_copy_attr(struct sched_attr __user *uattr, struct sched_attr *a
size < SCHED_ATTR_SIZE_VER1)
return -EINVAL;

+ if ((attr->sched_flags & SCHED_FLAG_LATENCY_NICE) &&
+ size < SCHED_ATTR_SIZE_VER2)
+ return -EINVAL;
/*
* XXX: Do we want to be lenient like existing syscalls; or do we want
* to be strict and return an error on out-of-bounds values?
@@ -8063,6 +8085,8 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
get_params(p, &kattr);
kattr.sched_flags &= SCHED_FLAG_ALL;

+ kattr.sched_latency_nice = p->latency_nice;
+
#ifdef CONFIG_UCLAMP_TASK
/*
* This could race with another potential updater, but this is fine
diff --git a/tools/include/uapi/linux/sched.h b/tools/include/uapi/linux/sched.h
index 3bac0a8ceab2..b2e932c25be6 100644
--- a/tools/include/uapi/linux/sched.h
+++ b/tools/include/uapi/linux/sched.h
@@ -132,6 +132,7 @@ struct clone_args {
#define SCHED_FLAG_KEEP_PARAMS 0x10
#define SCHED_FLAG_UTIL_CLAMP_MIN 0x20
#define SCHED_FLAG_UTIL_CLAMP_MAX 0x40
+#define SCHED_FLAG_LATENCY_NICE 0x80

#define SCHED_FLAG_KEEP_ALL (SCHED_FLAG_KEEP_POLICY | \
SCHED_FLAG_KEEP_PARAMS)
@@ -143,6 +144,7 @@ struct clone_args {
SCHED_FLAG_RECLAIM | \
SCHED_FLAG_DL_OVERRUN | \
SCHED_FLAG_KEEP_ALL | \
- SCHED_FLAG_UTIL_CLAMP)
+ SCHED_FLAG_UTIL_CLAMP | \
+ SCHED_FLAG_LATENCY_NICE)

#endif /* _UAPI_LINUX_SCHED_H */
--
2.17.1

2022-11-15 17:50:09

by Vincent Guittot

Subject: [PATCH v9 3/9] sched/core: Propagate parent task's latency requirements to the child task

From: Parth Shah <[email protected]>

Clone parent task's latency_nice attribute to the forked child task.

Reset the latency_nice value to default value when the child task is
set to sched_reset_on_fork.

Also, initialize init_task.latency_nice value with DEFAULT_LATENCY_NICE
value

Signed-off-by: Parth Shah <[email protected]>
[rebase]
Signed-off-by: Vincent Guittot <[email protected]>
---
init/init_task.c | 1 +
kernel/sched/core.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/init/init_task.c b/init/init_task.c
index ff6c4b9bfe6b..7dd71dd2d261 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -78,6 +78,7 @@ struct task_struct init_task
.prio = MAX_PRIO - 20,
.static_prio = MAX_PRIO - 20,
.normal_prio = MAX_PRIO - 20,
+ .latency_nice = DEFAULT_LATENCY_NICE,
.policy = SCHED_NORMAL,
.cpus_ptr = &init_task.cpus_mask,
.user_cpus_ptr = NULL,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 07ac08caf019..8c84c652853b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4592,6 +4592,7 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
p->prio = p->normal_prio = p->static_prio;
set_load_weight(p, false);

+ p->latency_nice = DEFAULT_LATENCY_NICE;
/*
* We don't need the reset flag anymore after the fork. It has
* fulfilled its duty:
--
2.17.1

2022-11-15 17:50:46

by Vincent Guittot

Subject: [PATCH v9 6/9] sched/fair: Add sched group latency support

Task can set its latency priority with sched_setattr(), which is then used
to set the latency offset of its sched_enity, but sched group entities
still have the default latency offset value.

Add a latency.nice field in cpu cgroup controller to set the latency
priority of the group similarly to sched_setattr(). The latency priority
is then used to set the offset of the sched_entities of the group.

Signed-off-by: Vincent Guittot <[email protected]>
---
Documentation/admin-guide/cgroup-v2.rst | 10 +++++
kernel/sched/core.c | 52 +++++++++++++++++++++++++
kernel/sched/fair.c | 33 ++++++++++++++++
kernel/sched/sched.h | 4 ++
4 files changed, 99 insertions(+)

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index dc254a3cb956..93a73663a5f7 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -1118,6 +1118,16 @@ All time durations are in microseconds.
values similar to the sched_setattr(2). This maximum utilization
value is used to clamp the task specific maximum utilization clamp.

+ cpu.latency.nice
+ A read-write single value file which exists on non-root
+ cgroups. The default is "0".
+
+ The nice value is in the range [-20, 19].
+
+ This interface file allows reading and setting latency using the
+ same values used by sched_setattr(2). The latency_nice of a group is
+ used to limit the impact of the latency_nice of a task outside the
+ group.


Memory
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b2b8cb6c08cd..9f6700f812ea 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -10967,6 +10967,47 @@ static int cpu_idle_write_s64(struct cgroup_subsys_state *css,
{
return sched_group_set_idle(css_tg(css), idle);
}
+
+static s64 cpu_latency_nice_read_s64(struct cgroup_subsys_state *css,
+ struct cftype *cft)
+{
+ int prio, delta, last_delta = INT_MAX;
+ s64 weight;
+
+ weight = css_tg(css)->latency_offset * NICE_LATENCY_WEIGHT_MAX;
+ weight = div_s64(weight, get_sleep_latency(false));
+
+ /* Find the closest nice value to the current weight */
+ for (prio = 0; prio < ARRAY_SIZE(sched_latency_to_weight); prio++) {
+ delta = abs(sched_latency_to_weight[prio] - weight);
+ if (delta >= last_delta)
+ break;
+ last_delta = delta;
+ }
+
+ return LATENCY_TO_NICE(prio-1);
+}
+
+static int cpu_latency_nice_write_s64(struct cgroup_subsys_state *css,
+ struct cftype *cft, s64 nice)
+{
+ s64 latency_offset;
+ long weight;
+ int idx;
+
+ if (nice < MIN_LATENCY_NICE || nice > MAX_LATENCY_NICE)
+ return -ERANGE;
+
+ idx = NICE_TO_LATENCY(nice);
+ idx = array_index_nospec(idx, LATENCY_NICE_WIDTH);
+ weight = sched_latency_to_weight[idx];
+
+ latency_offset = weight * get_sleep_latency(false);
+ latency_offset = div_s64(latency_offset, NICE_LATENCY_WEIGHT_MAX);
+
+ return sched_group_set_latency(css_tg(css), latency_offset);
+}
+
#endif

static struct cftype cpu_legacy_files[] = {
@@ -10981,6 +11022,11 @@ static struct cftype cpu_legacy_files[] = {
.read_s64 = cpu_idle_read_s64,
.write_s64 = cpu_idle_write_s64,
},
+ {
+ .name = "latency.nice",
+ .read_s64 = cpu_latency_nice_read_s64,
+ .write_s64 = cpu_latency_nice_write_s64,
+ },
#endif
#ifdef CONFIG_CFS_BANDWIDTH
{
@@ -11198,6 +11244,12 @@ static struct cftype cpu_files[] = {
.read_s64 = cpu_idle_read_s64,
.write_s64 = cpu_idle_write_s64,
},
+ {
+ .name = "latency.nice",
+ .flags = CFTYPE_NOT_ON_ROOT,
+ .read_s64 = cpu_latency_nice_read_s64,
+ .write_s64 = cpu_latency_nice_write_s64,
+ },
#endif
#ifdef CONFIG_CFS_BANDWIDTH
{
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0e80e65113bd..75c0a8d203c3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -12158,6 +12158,7 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
goto err;

tg->shares = NICE_0_LOAD;
+ tg->latency_offset = 0;

init_cfs_bandwidth(tg_cfs_bandwidth(tg));

@@ -12256,6 +12257,9 @@ void init_tg_cfs_entry(struct task_group *tg, struct cfs_rq *cfs_rq,
}

se->my_q = cfs_rq;
+
+ se->latency_offset = tg->latency_offset;
+
/* guarantee group entities always have weight */
update_load_set(&se->load, NICE_0_LOAD);
se->parent = parent;
@@ -12386,6 +12390,35 @@ int sched_group_set_idle(struct task_group *tg, long idle)
return 0;
}

+int sched_group_set_latency(struct task_group *tg, s64 latency)
+{
+ int i;
+
+ if (tg == &root_task_group)
+ return -EINVAL;
+
+ if (abs(latency) > sysctl_sched_latency)
+ return -EINVAL;
+
+ mutex_lock(&shares_mutex);
+
+ if (tg->latency_offset == latency) {
+ mutex_unlock(&shares_mutex);
+ return 0;
+ }
+
+ tg->latency_offset = latency;
+
+ for_each_possible_cpu(i) {
+ struct sched_entity *se = tg->se[i];
+
+ WRITE_ONCE(se->latency_offset, latency);
+ }
+
+ mutex_unlock(&shares_mutex);
+ return 0;
+}
+
#else /* CONFIG_FAIR_GROUP_SCHED */

void free_fair_sched_group(struct task_group *tg) { }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 7292652731d0..c3735a34d394 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -383,6 +383,8 @@ struct task_group {

/* A positive value indicates that this is a SCHED_IDLE group. */
int idle;
+ /* latency constraint of the group. */
+ int latency_offset;

#ifdef CONFIG_SMP
/*
@@ -493,6 +495,8 @@ extern int sched_group_set_shares(struct task_group *tg, unsigned long shares);

extern int sched_group_set_idle(struct task_group *tg, long idle);

+extern int sched_group_set_latency(struct task_group *tg, s64 latency);
+
#ifdef CONFIG_SMP
extern void set_task_rq_fair(struct sched_entity *se,
struct cfs_rq *prev, struct cfs_rq *next);
--
2.17.1

2022-11-15 18:12:40

by Vincent Guittot

Subject: [PATCH v9 7/9] sched/core: Support latency priority with sched core

Take into account wakeup_latency_gran() when ordering the cfs threads.

Signed-off-by: Vincent Guittot <[email protected]>
---
kernel/sched/fair.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 75c0a8d203c3..be446dc58be7 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11833,6 +11833,9 @@ bool cfs_prio_less(struct task_struct *a, struct task_struct *b, bool in_fi)
delta = (s64)(sea->vruntime - seb->vruntime) +
(s64)(cfs_rqb->min_vruntime_fi - cfs_rqa->min_vruntime_fi);

+ /* Take into account latency prio */
+ delta -= wakeup_latency_gran(sea, seb);
+
return delta > 0;
}
#else
--
2.17.1

2022-11-28 12:02:45

by K Prateek Nayak

Subject: Re: [PATCH v9 0/9] Add latency priority for CFS class

Hello Vincent,

Following are the test results on dual socket Zen3 machine (2 x 64C/128T)

tl;dr

o All benchmarks with DEFAULT_LATENCY_NICE value are comparable to tip.
There is, however, a noticeable dip for unixbench-spawn test case.

o With the 2 rbtree approach, I do not see much difference in the
hackbench results with varying latency nice value. Tests on v5 did
yield noticeable improvements for hackbench.
(https://lore.kernel.org/lkml/[email protected]/)

o For hackbench + cyclictest and hackbench + schbench, I see the
expected behavior with different latency nice values.

o There are a few cases with hackbench and hackbench + cyclictest where
the results are non-monotonic with different latency nice values.
(Marked with "^").

I'll leave the detailed results below:

On 11/15/2022 10:48 PM, Vincent Guittot wrote:
> This patchset restarts the work about adding a latency priority to describe
> the latency tolerance of cfs tasks.
>
> Patch [1] is a new one that has been added with v6. It fixes an
> unfairness for low prio tasks because of wakeup_gran() being bigger
> than the maximum vruntime credit that a waking task can keep after
> sleeping.
>
> The patches [2-4] have been done by Parth:
> https://lore.kernel.org/lkml/[email protected]/
>
> I have just rebased and moved the set of latency priority outside the
> priority update. I have removed the reviewed tag because the patches
> are 2 years old.
>
> This aims to be a generic interface and the following patches is one use
> of it to improve the scheduling latency of cfs tasks.
>
> Patch [5] uses latency nice priority to define a latency offset
> and then decide if a cfs task can or should preempt the current
> running task. The patch gives some tests results with cyclictests and
> hackbench to highlight the benefit of latency priority for short
> interactive task or long intensive tasks.
>
> Patch [6] adds the support of latency nice priority to task group by
> adding a cpu.latency.nice field. The range is [-20:19] as for setting task
> latency priority.
>
> Patch [7] makes sched_core taking into account the latency offset.
>
> Patch [8] adds a rb tree to cover some corner cases where the latency
> sensitive task (priority < 0) is preempted by high priority task (RT/DL)
> or fails to preempt them. This patch ensures that tasks will have at least
> a slice of sched_min_granularity in priority at wakeup.
>
> Patch [9] removes useless check after adding a latency rb tree.
>
> I have also backported the patchset on a dragonboard RB3 with an android
> mainline kernel based on v5.18 for a quick test. I have used the
> TouchLatency app which is part of AOSP and described to be a very good
> test to highlight jitter and jank frame sources of a system [1].
> In addition to the app, I have added some short running tasks waking-up
> regularly (to use the 8 cpus for 4 ms every 37777us) to stress the system
> without overloading it (and disabling EAS). The 1st results shows that the
> patchset helps to reduce the missed deadline frames from 5% to less than
> 0.1% when the cpu.latency.nice of task group are set. I haven't rerun the
> test with latest version.
>
> I have also tested the patchset with the modified version of the alsa
> latency test that has been shared by Tim. The test quickly xruns with
> default latency nice priority 0 but is able to run without underuns with
> a latency -20 and hackbench running simultaneously.
>
> While preparing the version 8, I have evaluated the benefit of using an
> augmented rbtree instead of adding a rbtree for latency sensitive entities,
> which was a relevant suggestion done by PeterZ. Although the augmented
> rbtree enables to sort additional information in the tree with a limited
> overhead, it has more impact on legacy use cases (latency_nice >= 0)
> because the augmented callbacks are always called to maintain this
> additional information even when there is no sensitive tasks. In such
> cases, the dedicated rbtree remains empty and the overhead is reduced to
> loading a cached null node pointer. Nevertheless, we might want to
> reconsider the augmented rbtree once the use of negative latency_nice will
> be more widlely deployed. At now, the different tests that I have done,
> have not shown improvements with augmented rbtree.
>
> Below are some hackbench results:
> 2 rbtrees augmented rbtree augmented rbtree
> sorted by vruntime sorted by wakeup_vruntime
> sched pipe
> avg 26311,000 25976,667 25839,556
> stdev 0,15 % 0,28 % 0,24 %
> vs tip 0,50 % -0,78 % -1,31 %
> hackbench 1 group
> avg 1,315 1,344 1,359
> stdev 0,88 % 1,55 % 1,82 %
> vs tip -0,47 % -2,68 % -3,87 %
> hackbench 4 groups
> avg 1,339 1,365 1,367
> stdev 2,39 % 2,26 % 3,58 %
> vs tip -0,08 % -2,01 % -2,22 %
> hackbench 8 groups
> avg 1,233 1,286 1,301
> stdev 0,74 % 1,09 % 1,52 %
> vs tip 0,29 % -4,05 % -5,27 %
> hackbench 16 groups
> avg 1,268 1,313 1,319
> stdev 0,85 % 1,60 % 0,68 %
> vs tip -0,02 % -3,56 % -4,01 %

Following are the results from running standard benchmarks on a
dual socket Zen3 (2 x 64C/128T) machine configured in different
NPS modes.

NPS Modes are used to logically divide single socket into
multiple NUMA region.
Following is the NUMA configuration for each NPS mode on the system:

NPS1: Each socket is a NUMA node.
Total 2 NUMA nodes in the dual socket machine.

Node 0: 0-63, 128-191
Node 1: 64-127, 192-255

NPS2: Each socket is further logically divided into 2 NUMA regions.
Total 4 NUMA nodes exist over 2 socket.

Node 0: 0-31, 128-159
Node 1: 32-63, 160-191
Node 2: 64-95, 192-223
Node 3: 96-127, 223-255

NPS4: Each socket is logically divided into 4 NUMA regions.
Total 8 NUMA nodes exist over 2 socket.

Node 0: 0-15, 128-143
Node 1: 16-31, 144-159
Node 2: 32-47, 160-175
Node 3: 48-63, 176-191
Node 4: 64-79, 192-207
Node 5: 80-95, 208-223
Node 6: 96-111, 223-231
Node 7: 112-127, 232-255

Benchmark Results:

Kernel versions:
- tip: 6.1.0 tip sched/core
- latency_nice: 6.1.0 tip sched/core + this series

When we started testing, the tip was at:
commit d6962c4fe8f9 "sched: Clear ttwu_pending after enqueue_task()"


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ hackbench - DEFAULT_LATENCY_NICE ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

NPS1

Test: tip latency_nice
1-groups: 4.25 (0.00 pct) 4.14 (2.58 pct)
2-groups: 4.95 (0.00 pct) 4.92 (0.60 pct)
4-groups: 5.19 (0.00 pct) 5.18 (0.19 pct)
8-groups: 5.45 (0.00 pct) 5.44 (0.18 pct)
16-groups: 7.33 (0.00 pct) 7.32 (0.13 pct)

NPS2

Test: tip latency_nice
1-groups: 4.09 (0.00 pct) 4.08 (0.24 pct)
2-groups: 4.68 (0.00 pct) 4.72 (-0.85 pct)
4-groups: 5.05 (0.00 pct) 4.97 (1.58 pct)
8-groups: 5.37 (0.00 pct) 5.34 (0.55 pct)
16-groups: 6.69 (0.00 pct) 6.74 (-0.74 pct)

NPS4

Test: tip latency_nice
1-groups: 4.28 (0.00 pct) 4.35 (-1.63 pct)
2-groups: 4.78 (0.00 pct) 4.76 (0.41 pct)
4-groups: 5.11 (0.00 pct) 5.06 (0.97 pct)
8-groups: 5.48 (0.00 pct) 5.40 (1.45 pct)
16-groups: 7.07 (0.00 pct) 6.70 (5.23 pct)

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ schbench - DEFAULT_LATENCY_NICE ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

NPS1

#workers: tip latency_nice
1: 31.00 (0.00 pct) 32.00 (-3.22 pct)
2: 33.00 (0.00 pct) 34.00 (-3.03 pct)
4: 39.00 (0.00 pct) 38.00 (2.56 pct)
8: 45.00 (0.00 pct) 46.00 (-2.22 pct)
16: 61.00 (0.00 pct) 66.00 (-8.19 pct)
32: 108.00 (0.00 pct) 110.00 (-1.85 pct)
64: 212.00 (0.00 pct) 216.00 (-1.88 pct)
128: 475.00 (0.00 pct) 701.00 (-47.57 pct) *
128: 429.00 (0.00 pct) 441.00 (-2.79 pct) [Verification Run]
256: 44736.00 (0.00 pct) 45632.00 (-2.00 pct)
512: 77184.00 (0.00 pct) 78720.00 (-1.99 pct)

NPS2

#workers: tip latency_nice
1: 28.00 (0.00 pct) 33.00 (-17.85 pct)
2: 34.00 (0.00 pct) 31.00 (8.82 pct)
4: 36.00 (0.00 pct) 36.00 (0.00 pct)
8: 51.00 (0.00 pct) 49.00 (3.92 pct)
16: 68.00 (0.00 pct) 64.00 (5.88 pct)
32: 113.00 (0.00 pct) 115.00 (-1.76 pct)
64: 221.00 (0.00 pct) 219.00 (0.90 pct)
128: 553.00 (0.00 pct) 531.00 (3.97 pct)
256: 43840.00 (0.00 pct) 48192.00 (-9.92 pct) *
256: 50427.00 (0.00 pct) 48351.00 (4.11 pct) [Verification Run]
512: 76672.00 (0.00 pct) 81024.00 (-5.67 pct)

NPS4

#workers: tip latency_nice
1: 33.00 (0.00 pct) 28.00 (15.15 pct)
2: 29.00 (0.00 pct) 34.00 (-17.24 pct)
4: 39.00 (0.00 pct) 36.00 (7.69 pct)
8: 58.00 (0.00 pct) 55.00 (5.17 pct)
16: 66.00 (0.00 pct) 67.00 (-1.51 pct)
32: 112.00 (0.00 pct) 116.00 (-3.57 pct)
64: 215.00 (0.00 pct) 213.00 (0.93 pct)
128: 689.00 (0.00 pct) 571.00 (17.12 pct)
256: 45120.00 (0.00 pct) 46400.00 (-2.83 pct)
512: 77440.00 (0.00 pct) 76160.00 (1.65 pct)


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ tbench - DEFAULT_LATENCY_NICE ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

NPS1

Clients: tip latency_nice
1 581.75 (0.00 pct) 586.52 (0.81 pct)
2 1145.75 (0.00 pct) 1160.69 (1.30 pct)
4 2127.94 (0.00 pct) 2141.49 (0.63 pct)
8 3838.27 (0.00 pct) 3721.10 (-3.05 pct)
16 6272.71 (0.00 pct) 6539.82 (4.25 pct)
32 11400.12 (0.00 pct) 12079.49 (5.95 pct)
64 21605.96 (0.00 pct) 22908.83 (6.03 pct)
128 30715.43 (0.00 pct) 31736.95 (3.32 pct)
256 55580.78 (0.00 pct) 54786.29 (-1.42 pct)
512 56528.79 (0.00 pct) 56453.54 (-0.13 pct)
1024 56520.40 (0.00 pct) 56369.93 (-0.26 pct)

NPS2

Clients: tip latency_nice
1 584.13 (0.00 pct) 582.53 (-0.27 pct)
2 1153.63 (0.00 pct) 1140.27 (-1.15 pct)
4 2212.89 (0.00 pct) 2159.49 (-2.41 pct)
8 3871.35 (0.00 pct) 3840.77 (-0.78 pct)
16 6216.72 (0.00 pct) 6437.98 (3.55 pct)
32 11766.98 (0.00 pct) 11663.53 (-0.87 pct)
64 22000.93 (0.00 pct) 21882.88 (-0.53 pct)
128 31520.53 (0.00 pct) 31147.05 (-1.18 pct)
256 51420.11 (0.00 pct) 55216.39 (7.38 pct)
512 53935.90 (0.00 pct) 55407.60 (2.72 pct)
1024 55239.73 (0.00 pct) 55997.25 (1.37 pct)

NPS4

Clients: tip latency_nice
1 585.83 (0.00 pct) 578.17 (-1.30 pct)
2 1141.59 (0.00 pct) 1131.14 (-0.91 pct)
4 2174.79 (0.00 pct) 2086.52 (-4.05 pct)
8 3887.56 (0.00 pct) 3778.47 (-2.80 pct)
16 6441.59 (0.00 pct) 6364.30 (-1.19 pct)
32 12133.60 (0.00 pct) 11465.26 (-5.50 pct) *
32 11677.16 (0.00 pct) 12662.09 (8.43 pct) [Verification Run]
64 21769.15 (0.00 pct) 19488.45 (-10.47 pct) *
64 20305.64 (0.00 pct) 21002.90 (3.43 pct) [Verification Run]
128 31396.31 (0.00 pct) 31177.37 (-0.69 pct)
256 52792.39 (0.00 pct) 52890.41 (0.18 pct)
512 55315.44 (0.00 pct) 53572.65 (-3.15 pct)
1024 52150.27 (0.00 pct) 54079.48 (3.69 pct)


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ stream - DEFAULT_LATENCY_NICE ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

NPS1

10 Runs:

Test: tip latency_nice
Copy: 307827.79 (0.00 pct) 330524.48 (7.37 pct)
Scale: 208872.28 (0.00 pct) 215002.06 (2.93 pct)
Add: 239404.64 (0.00 pct) 230334.74 (-3.78 pct)
Triad: 247258.30 (0.00 pct) 238505.06 (-3.54 pct)

100 Runs:

Test: tip latency_nice
Copy: 317217.55 (0.00 pct) 314467.62 (-0.86 pct)
Scale: 208740.82 (0.00 pct) 210452.00 (0.81 pct)
Add: 240550.63 (0.00 pct) 232376.03 (-3.39 pct)
Triad: 249594.21 (0.00 pct) 242460.83 (-2.85 pct)

NPS2

10 Runs:

Test: tip latency_nice
Copy: 340877.18 (0.00 pct) 339441.26 (-0.42 pct)
Scale: 217318.16 (0.00 pct) 216905.49 (-0.18 pct)
Add: 259078.93 (0.00 pct) 261686.67 (1.00 pct)
Triad: 274500.78 (0.00 pct) 271699.83 (-1.02 pct)

100 Runs:

Test: tip latency_nice
Copy: 341860.73 (0.00 pct) 335826.36 (-1.76 pct)
Scale: 218043.00 (0.00 pct) 216451.84 (-0.72 pct)
Add: 253698.22 (0.00 pct) 257317.72 (1.42 pct)
Triad: 265011.84 (0.00 pct) 267769.93 (1.04 pct)

NPS4

10 Runs:

Test: tip latency_nice
Copy: 340877.18 (0.00 pct) 365921.51 (7.34 pct)
Scale: 217318.16 (0.00 pct) 239408.65 (10.16 pct)
Add: 259078.93 (0.00 pct) 264859.31 (2.23 pct)
Triad: 274500.78 (0.00 pct) 281543.65 (2.56 pct)

100 Runs:

Test: tip latency_nice
Copy: 341860.73 (0.00 pct) 359255.16 (5.08 pct)
Scale: 218043.00 (0.00 pct) 238154.15 (9.22 pct)
Add: 253698.22 (0.00 pct) 269223.49 (6.11 pct)
Triad: 265011.84 (0.00 pct) 278473.85 (5.07 pct)

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ ycsb-mongodb - DEFAULT_LATENCY_NICE ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

o NPS1

tip: 131244.00 (var: 2.67%)
latency_nice: 132118.00 (var: 3.62%) (+0.66%)

o NPS2

tip: 127663.33 (var: 2.08%)
latency_nice: 129148.00 (var: 4.29%) (+1.16%)

o NPS4

tip: 133295.00 (var: 1.58%)
latency_nice: 129975.33 (var: 1.10%) (-2.49%)


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ Unixbench - DEFAULT_LATENCY_NICE ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

o NPS1

Test Metric Parallelism tip latency_nice
unixbench-dhry2reg Hmean unixbench-dhry2reg-1 48929419.48 ( 0.00%) 49137039.06 ( 0.42%)
unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6275526953.25 ( 0.00%) 6265580479.15 ( -0.16%)
unixbench-syscall Amean unixbench-syscall-1 2994319.73 ( 0.00%) 3008596.83 * -0.48%*
unixbench-syscall Amean unixbench-syscall-512 7349715.87 ( 0.00%) 7420994.50 * -0.97%*
unixbench-pipe Hmean unixbench-pipe-1 2830206.03 ( 0.00%) 2854405.99 * 0.86%*
unixbench-pipe Hmean unixbench-pipe-512 326207828.01 ( 0.00%) 328997804.52 * 0.86%*
unixbench-spawn Hmean unixbench-spawn-1 6394.21 ( 0.00%) 6367.75 ( -0.41%)
unixbench-spawn Hmean unixbench-spawn-512 72700.64 ( 0.00%) 71454.19 * -1.71%*
unixbench-execl Hmean unixbench-execl-1 4723.61 ( 0.00%) 4750.59 ( 0.57%)
unixbench-execl Hmean unixbench-execl-512 11212.05 ( 0.00%) 11262.13 ( 0.45%)

o NPS2

Test Metric Parallelism tip latency_nice
unixbench-dhry2reg Hmean unixbench-dhry2reg-1 49271512.85 ( 0.00%) 49245260.43 ( -0.05%)
unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6267992483.03 ( 0.00%) 6264951100.67 ( -0.05%)
unixbench-syscall Amean unixbench-syscall-1 2995885.93 ( 0.00%) 3005975.10 * -0.34%*
unixbench-syscall Amean unixbench-syscall-512 7388865.77 ( 0.00%) 7276275.63 * 1.52%*
unixbench-pipe Hmean unixbench-pipe-1 2828971.95 ( 0.00%) 2856578.72 * 0.98%*
unixbench-pipe Hmean unixbench-pipe-512 326225385.37 ( 0.00%) 328941270.81 * 0.83%*
unixbench-spawn Hmean unixbench-spawn-1 6958.71 ( 0.00%) 6954.21 ( -0.06%)
unixbench-spawn Hmean unixbench-spawn-512 85443.56 ( 0.00%) 70536.42 * -17.45%* (0.67% vs 0.93% - CoEff var)
unixbench-execl Hmean unixbench-execl-1 4767.99 ( 0.00%) 4752.63 * -0.32%*
unixbench-execl Hmean unixbench-execl-512 11250.72 ( 0.00%) 11320.97 ( 0.62%)

o NPS4

Test Metric Parallelism tip latency_nice
unixbench-dhry2reg Hmean unixbench-dhry2reg-1 49041932.68 ( 0.00%) 49156671.05 ( 0.23%)
unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6286981589.85 ( 0.00%) 6285248711.40 ( -0.03%)
unixbench-syscall Amean unixbench-syscall-1 2992405.60 ( 0.00%) 3008933.03 * -0.55%*
unixbench-syscall Amean unixbench-syscall-512 7971789.70 ( 0.00%) 7814622.23 * 1.97%*
unixbench-pipe Hmean unixbench-pipe-1 2822892.54 ( 0.00%) 2852615.11 * 1.05%*
unixbench-pipe Hmean unixbench-pipe-512 326408309.83 ( 0.00%) 329617202.56 * 0.98%*
unixbench-spawn Hmean unixbench-spawn-1 7685.31 ( 0.00%) 7243.54 ( -5.75%)
unixbench-spawn Hmean unixbench-spawn-512 72245.56 ( 0.00%) 77000.81 * 6.58%*
unixbench-execl Hmean unixbench-execl-1 4761.42 ( 0.00%) 4733.12 * -0.59%*
unixbench-execl Hmean unixbench-execl-512 11533.53 ( 0.00%) 11660.17 ( 1.10%)

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ Hackbench - Various Latency Nice Values ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

o 100000 loops

- pipe (process)

Test: LN: 0 LN: 19 LN: -20
1-groups: 3.91 (0.00 pct) 3.91 (0.00 pct) 3.81 (2.55 pct)
2-groups: 4.48 (0.00 pct) 4.52 (-0.89 pct) 4.53 (-1.11 pct)
4-groups: 4.83 (0.00 pct) 4.83 (0.00 pct) 4.87 (-0.82 pct)
8-groups: 5.09 (0.00 pct) 5.00 (1.76 pct) 5.07 (0.39 pct)
16-groups: 6.92 (0.00 pct) 6.79 (1.87 pct) 6.96 (-0.57 pct)

- pipe (thread)

1-groups: 4.13 (0.00 pct) 4.08 (1.21 pct) 4.11 (0.48 pct)
2-groups: 4.78 (0.00 pct) 4.90 (-2.51 pct) 4.79 (-0.20 pct)
4-groups: 5.12 (0.00 pct) 5.08 (0.78 pct) 5.16 (-0.78 pct)
8-groups: 5.31 (0.00 pct) 5.28 (0.56 pct) 5.33 (-0.37 pct)
16-groups: 7.34 (0.00 pct) 7.27 (0.95 pct) 7.33 (0.13 pct)

- socket (process)

Test: LN: 0 LN: 19 LN: -20
1-groups: 6.61 (0.00 pct) 6.38 (3.47 pct) 6.54 (1.05 pct)
2-groups: 6.59 (0.00 pct) 6.67 (-1.21 pct) 6.11 (7.28 pct)
4-groups: 6.77 (0.00 pct) 6.78 (-0.14 pct) 6.79 (-0.29 pct)
8-groups: 8.29 (0.00 pct) 8.39 (-1.20 pct) 8.36 (-0.84 pct)
16-groups: 12.21 (0.00 pct) 12.03 (1.47 pct) 12.35 (-1.14 pct)

- socket (thread)

Test: LN: 0 LN: 19 LN: -20
1-groups: 6.50 (0.00 pct) 5.99 (7.84 pct) 6.02 (7.38 pct) ^
2-groups: 6.07 (0.00 pct) 6.20 (-2.14 pct) 6.23 (-2.63 pct)
4-groups: 6.61 (0.00 pct) 6.64 (-0.45 pct) 6.63 (-0.30 pct)
8-groups: 8.87 (0.00 pct) 8.67 (2.25 pct) 8.78 (1.01 pct)
16-groups: 12.63 (0.00 pct) 12.54 (0.71 pct) 12.59 (0.31 pct)

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ Hackbench + Cyclictest - Various Latency Nice Values ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

- Hackbench: 32 Groups

perf bench sched messaging -p -l 100000 -g 32&
cyclictest --policy other -D 5 -q -n -h 2000

o NPS1

----------------------------------------------------------------------------------------------------------
| Hackbench | Cyclictest LN = 19 | Cyclictest LN = 0 | Cyclictest LN = -20 |
| LN |------------------------------|-------------------------------|---------------------------|
| | Min | Avg | Max | Min | Avg | Max | Min | Avg | Max |
|-------------|--------|---------|-----------|--------|---------|------------|--------|-------|----------|
| 19 | 52.00 | 71.00 | 5191.00 | 29.00 | 68.00 | 4477.00 | 53.00 | 60.00 | 753.00 |
| 0 | 53.00 | 150.00 | 7300.00 | 53.00 | 105.00 | 7730.00 | 53.00 | 64.00 | 2067.00 |
| -20 | 33.00 | 159.00 | 98492.00 | 53.00 | 149.00 | 9608.00 | 53.00 | 91.00 | 5349.00 |
----------------------------------------------------------------------------------------------------------

o NPS4

----------------------------------------------------------------------------------------------------------
| Hackbench | Cyclictest LN = 19 | Cyclictest LN = 0 | Cyclictest LN = -20 |
| LN |------------------------------|-------------------------------|---------------------------|
| | Min | Avg | Max | Min | Avg | Max | Min | Avg | Max |
|-------------|--------|---------|-----------|--------|---------|------------|--------|-------|----------|
| 19 | 53.00 | 84.00 | 4790.00 | 53.00 | 72.00 | 3456.00 | 53.00 | 58.00 | 1271.00 |
| 0 | 53.00 | 99.00 | 5494.00 | 52.00 | 74.00 | 5813.00 | 53.00 | 59.00 | 1004.00 |
| -20 | 45.00 | 84.00 | 3592.00 | 53.00 | 91.00 | 15222.00 | 53.00 | 74.00 | 5232.00 | ^
----------------------------------------------------------------------------------------------------------

- Hackbench: 128 Groups

perf bench sched messaging -p -l 500000 -g 128&
cyclictest --policy other -D 5 -q -n -h 2000

o NPS1

----------------------------------------------------------------------------------------------------------
| Hackbench | Cyclictest LN = 19 | Cyclictest LN = 0 | Cyclictest LN = -20 |
| LN |------------------------------|-------------------------------|---------------------------|
| | Min | Avg | Max | Min | Avg | Max | Min | Avg | Max |
|-------------|--------|---------|-----------|--------|---------|------------|--------|-------|----------|
| 19 | 53.00 | 274.00 | 11294.00 | 33.00 | 130.00 | 20071.00 | 53.00 | 56.00 | 244.00 | ^
| 0 | 53.00 | 125.00 | 10014.00 | 53.00 | 113.00 | 15857.00 | 53.00 | 57.00 | 250.00 |
| -20 | 53.00 | 187.00 | 49565.00 | 53.00 | 230.00 | 73353.00 | 53.00 | 118.00| 8816.00 |
----------------------------------------------------------------------------------------------------------

o NPS4

----------------------------------------------------------------------------------------------------------
| Hackbench | Cyclictest LN = 19 | Cyclictest LN = 0 | Cyclictest LN = -20 |
| LN |------------------------------|-------------------------------|---------------------------|
| | Min | Avg | Max | Min | Avg | Max | Min | Avg | Max |
|-------------|--------|---------|-----------|--------|---------|------------|--------|-------|----------|
| 19 | 53.00 | 271.00 | 11411.00 | 53.00 | 82.00 | 5486.00 | 25.00 | 57.00 | 1256.00 |
| 0 | 53.00 | 148.00 | 8374.00 | 52.00 | 109.00 | 11074.00 | 52.00 | 59.00 | 1068.00 |
| -20 | 53.00 | 202.00 | 52537.00 | 53.00 | 205.00 | 22265.00 | 52.00 | 87.00 | 14151.00 |
----------------------------------------------------------------------------------------------------------

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ Hackbench + schbench - Various Latency Nice Values ~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

perf bench sched messaging -p -l 400000 -g 128
schbench -m 2 -t 1 -s 30

o NPS1

-------------------------------------------------------------------------------------------------
| Hackbench | schbench LN = 19 | schbench LN = 0 | schbench LN = -20 |
| LN |----------------------------|---------------------------|--------------------------|
| | 90th | 95th | 99th | 90th | 95th | 99th | 90th | 95th | 99th |
|-----------|--------|--------|----------|--------|--------|---------|--------|--------|--------|
| 19 | 38 | 131 | 1458 | 46 | 151 | 2636 | 11 | 19 | 410 | ^
| 0 | 45 | 98 | 1758 | 25 | 50 | 1670 | 16 | 30 | 1042 |
| -20 | 47 | 348 | 29280 | 40 | 109 | 16144 | 35 | 63 | 9104 |
-------------------------------------------------------------------------------------------------

o NPS4

-------------------------------------------------------------------------------------------------
| Hackbench | schbench LN = 19 | schbench LN = 0 | schbench LN = -20 |
| LN |----------------------------|---------------------------|--------------------------|
| | 90th | 95th | 99th | 90th | 95th | 99th | 90th | 95th | 99th |
|-----------|--------|--------|----------|--------|--------|---------|--------|--------|--------|
| 19 | 19 | 60 | 1886 | 17 | 29 | 621 | 10 | 18 | 227 |
| 0 | 51 | 141 | 8120 | 37 | 78 | 8880 | 33 | 55 | 474 | ^
| -20 | 48 | 1494 | 27296 | 51 | 469 | 40384 | 31 | 64 | 4092 | ^
-------------------------------------------------------------------------------------------------

^ Note: There are cases where the Max, 99th percentile latency is
non-monotonic but I've also seen a good amount of run to run variation
there with a single bad sample polluting the results. In such cases,
the averages are more representative.

>
> [1] https://source.android.com/docs/core/debug/eval_perf#touchlatency
>
> [..snip..]
>

Apart from couple of anomalies, latency nice reduces wait time, especially
when the system is heavily loaded. If there is any data, or any specific
workload you would like me to run on the test system, please do let me know.
Meanwhile, I'll try to get some numbers for larger workloads like SpecJBB
that did see improvements with latency nice on v5.
--
Thanks and Regards,
Prateek

2022-11-28 17:31:30

by Vincent Guittot

Subject: Re: [PATCH v9 0/9] Add latency priority for CFS class

Hi Prateek,

On Mon, 28 Nov 2022 at 12:52, K Prateek Nayak <[email protected]> wrote:
>
> Hello Vincent,
>
> Following are the test results on dual socket Zen3 machine (2 x 64C/128T)
>
> tl;dr
>
> o All benchmarks with DEFAULT_LATENCY_NICE value are comparable to tip.
> There is, however, a noticeable dip for unixbench-spawn test case.
>
> o With the 2 rbtree approach, I do not see much difference in the
> hackbench results with varying latency nice value. Tests on v5 did
> yield noticeable improvements for hackbench.
> (https://lore.kernel.org/lkml/[email protected]/)

The 2 rbtree approach is the one that was already used in v5. I just
rerun hackbench tests with latest tip and v6.2-rc7 and I can see large
performance improvement for pipe tests on my system (8 cores system).
Could you try witha larger number of group ? like 64, 128 and 256
groups

>
> o For hackbench + cyclictest and hackbench + schbench, I see the
> expected behavior with different latency nice values.
>
> o There are a few cases with hackbench and hackbench + cyclictest where
> the results are non-monotonic with different latency nice values.
> (Marked with "^").
>
> I'll leave the detailed results below:
>
> On 11/15/2022 10:48 PM, Vincent Guittot wrote:
> > This patchset restarts the work about adding a latency priority to describe
> > the latency tolerance of cfs tasks.
> >
> > Patch [1] is a new one that has been added with v6. It fixes an
> > unfairness for low prio tasks because of wakeup_gran() being bigger
> > than the maximum vruntime credit that a waking task can keep after
> > sleeping.
> >
> > The patches [2-4] have been done by Parth:
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > I have just rebased and moved the set of latency priority outside the
> > priority update. I have removed the reviewed tag because the patches
> > are 2 years old.
> >
> > This aims to be a generic interface and the following patches is one use
> > of it to improve the scheduling latency of cfs tasks.
> >
> > Patch [5] uses latency nice priority to define a latency offset
> > and then decide if a cfs task can or should preempt the current
> > running task. The patch gives some tests results with cyclictests and
> > hackbench to highlight the benefit of latency priority for short
> > interactive task or long intensive tasks.
> >
> > Patch [6] adds the support of latency nice priority to task group by
> > adding a cpu.latency.nice field. The range is [-20:19] as for setting task
> > latency priority.
> >
> > Patch [7] makes sched_core taking into account the latency offset.
> >
> > Patch [8] adds a rb tree to cover some corner cases where the latency
> > sensitive task (priority < 0) is preempted by high priority task (RT/DL)
> > or fails to preempt them. This patch ensures that tasks will have at least
> > a slice of sched_min_granularity in priority at wakeup.
> >
> > Patch [9] removes useless check after adding a latency rb tree.
> >
> > I have also backported the patchset on a dragonboard RB3 with an android
> > mainline kernel based on v5.18 for a quick test. I have used the
> > TouchLatency app which is part of AOSP and described to be a very good
> > test to highlight jitter and jank frame sources of a system [1].
> > In addition to the app, I have added some short running tasks waking-up
> > regularly (to use the 8 cpus for 4 ms every 37777us) to stress the system
> > without overloading it (and disabling EAS). The 1st results shows that the
> > patchset helps to reduce the missed deadline frames from 5% to less than
> > 0.1% when the cpu.latency.nice of task group are set. I haven't rerun the
> > test with latest version.
> >
> > I have also tested the patchset with the modified version of the alsa
> > latency test that has been shared by Tim. The test quickly xruns with
> > default latency nice priority 0 but is able to run without underuns with
> > a latency -20 and hackbench running simultaneously.
> >
> > While preparing the version 8, I have evaluated the benefit of using an
> > augmented rbtree instead of adding a rbtree for latency sensitive entities,
> > which was a relevant suggestion done by PeterZ. Although the augmented
> > rbtree enables to sort additional information in the tree with a limited
> > overhead, it has more impact on legacy use cases (latency_nice >= 0)
> > because the augmented callbacks are always called to maintain this
> > additional information even when there is no sensitive tasks. In such
> > cases, the dedicated rbtree remains empty and the overhead is reduced to
> > loading a cached null node pointer. Nevertheless, we might want to
> > reconsider the augmented rbtree once the use of negative latency_nice will
> > be more widlely deployed. At now, the different tests that I have done,
> > have not shown improvements with augmented rbtree.
> >
> > Below are some hackbench results:
> > 2 rbtrees augmented rbtree augmented rbtree
> > sorted by vruntime sorted by wakeup_vruntime
> > sched pipe
> > avg 26311,000 25976,667 25839,556
> > stdev 0,15 % 0,28 % 0,24 %
> > vs tip 0,50 % -0,78 % -1,31 %
> > hackbench 1 group
> > avg 1,315 1,344 1,359
> > stdev 0,88 % 1,55 % 1,82 %
> > vs tip -0,47 % -2,68 % -3,87 %
> > hackbench 4 groups
> > avg 1,339 1,365 1,367
> > stdev 2,39 % 2,26 % 3,58 %
> > vs tip -0,08 % -2,01 % -2,22 %
> > hackbench 8 groups
> > avg 1,233 1,286 1,301
> > stdev 0,74 % 1,09 % 1,52 %
> > vs tip 0,29 % -4,05 % -5,27 %
> > hackbench 16 groups
> > avg 1,268 1,313 1,319
> > stdev 0,85 % 1,60 % 0,68 %
> > vs tip -0,02 % -3,56 % -4,01 %
>
> Following are the results from running standard benchmarks on a
> dual socket Zen3 (2 x 64C/128T) machine configured in different
> NPS modes.
>
> NPS Modes are used to logically divide single socket into
> multiple NUMA region.
> Following is the NUMA configuration for each NPS mode on the system:
>
> NPS1: Each socket is a NUMA node.
> Total 2 NUMA nodes in the dual socket machine.
>
> Node 0: 0-63, 128-191
> Node 1: 64-127, 192-255
>
> NPS2: Each socket is further logically divided into 2 NUMA regions.
> Total 4 NUMA nodes exist over 2 socket.
>
> Node 0: 0-31, 128-159
> Node 1: 32-63, 160-191
> Node 2: 64-95, 192-223
> Node 3: 96-127, 223-255
>
> NPS4: Each socket is logically divided into 4 NUMA regions.
> Total 8 NUMA nodes exist over 2 socket.
>
> Node 0: 0-15, 128-143
> Node 1: 16-31, 144-159
> Node 2: 32-47, 160-175
> Node 3: 48-63, 176-191
> Node 4: 64-79, 192-207
> Node 5: 80-95, 208-223
> Node 6: 96-111, 223-231
> Node 7: 112-127, 232-255
>
> Benchmark Results:
>
> Kernel versions:
> - tip: 6.1.0 tip sched/core
> - latency_nice: 6.1.0 tip sched/core + this series
>
> When we started testing, the tip was at:
> commit d6962c4fe8f9 "sched: Clear ttwu_pending after enqueue_task()"
>
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ hackbench - DEFAULT_LATENCY_NICE ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> NPS1
>
> Test: tip latency_nice
> 1-groups: 4.25 (0.00 pct) 4.14 (2.58 pct)
> 2-groups: 4.95 (0.00 pct) 4.92 (0.60 pct)
> 4-groups: 5.19 (0.00 pct) 5.18 (0.19 pct)
> 8-groups: 5.45 (0.00 pct) 5.44 (0.18 pct)
> 16-groups: 7.33 (0.00 pct) 7.32 (0.13 pct)
>
> NPS2
>
> Test: tip latency_nice
> 1-groups: 4.09 (0.00 pct) 4.08 (0.24 pct)
> 2-groups: 4.68 (0.00 pct) 4.72 (-0.85 pct)
> 4-groups: 5.05 (0.00 pct) 4.97 (1.58 pct)
> 8-groups: 5.37 (0.00 pct) 5.34 (0.55 pct)
> 16-groups: 6.69 (0.00 pct) 6.74 (-0.74 pct)
>
> NPS4
>
> Test: tip latency_nice
> 1-groups: 4.28 (0.00 pct) 4.35 (-1.63 pct)
> 2-groups: 4.78 (0.00 pct) 4.76 (0.41 pct)
> 4-groups: 5.11 (0.00 pct) 5.06 (0.97 pct)
> 8-groups: 5.48 (0.00 pct) 5.40 (1.45 pct)
> 16-groups: 7.07 (0.00 pct) 6.70 (5.23 pct)
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ schbench - DEFAULT_LATENCY_NICE ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> NPS1
>
> #workers: tip latency_nice
> 1: 31.00 (0.00 pct) 32.00 (-3.22 pct)
> 2: 33.00 (0.00 pct) 34.00 (-3.03 pct)
> 4: 39.00 (0.00 pct) 38.00 (2.56 pct)
> 8: 45.00 (0.00 pct) 46.00 (-2.22 pct)
> 16: 61.00 (0.00 pct) 66.00 (-8.19 pct)
> 32: 108.00 (0.00 pct) 110.00 (-1.85 pct)
> 64: 212.00 (0.00 pct) 216.00 (-1.88 pct)
> 128: 475.00 (0.00 pct) 701.00 (-47.57 pct) *
> 128: 429.00 (0.00 pct) 441.00 (-2.79 pct) [Verification Run]
> 256: 44736.00 (0.00 pct) 45632.00 (-2.00 pct)
> 512: 77184.00 (0.00 pct) 78720.00 (-1.99 pct)
>
> NPS2
>
> #workers: tip latency_nice
> 1: 28.00 (0.00 pct) 33.00 (-17.85 pct)
> 2: 34.00 (0.00 pct) 31.00 (8.82 pct)
> 4: 36.00 (0.00 pct) 36.00 (0.00 pct)
> 8: 51.00 (0.00 pct) 49.00 (3.92 pct)
> 16: 68.00 (0.00 pct) 64.00 (5.88 pct)
> 32: 113.00 (0.00 pct) 115.00 (-1.76 pct)
> 64: 221.00 (0.00 pct) 219.00 (0.90 pct)
> 128: 553.00 (0.00 pct) 531.00 (3.97 pct)
> 256: 43840.00 (0.00 pct) 48192.00 (-9.92 pct) *
> 256: 50427.00 (0.00 pct) 48351.00 (4.11 pct) [Verification Run]
> 512: 76672.00 (0.00 pct) 81024.00 (-5.67 pct)
>
> NPS4
>
> #workers: tip latency_nice
> 1: 33.00 (0.00 pct) 28.00 (15.15 pct)
> 2: 29.00 (0.00 pct) 34.00 (-17.24 pct)
> 4: 39.00 (0.00 pct) 36.00 (7.69 pct)
> 8: 58.00 (0.00 pct) 55.00 (5.17 pct)
> 16: 66.00 (0.00 pct) 67.00 (-1.51 pct)
> 32: 112.00 (0.00 pct) 116.00 (-3.57 pct)
> 64: 215.00 (0.00 pct) 213.00 (0.93 pct)
> 128: 689.00 (0.00 pct) 571.00 (17.12 pct)
> 256: 45120.00 (0.00 pct) 46400.00 (-2.83 pct)
> 512: 77440.00 (0.00 pct) 76160.00 (1.65 pct)
>
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ tbench - DEFAULT_LATENCY_NICE ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> NPS1
>
> Clients: tip latency_nice
> 1 581.75 (0.00 pct) 586.52 (0.81 pct)
> 2 1145.75 (0.00 pct) 1160.69 (1.30 pct)
> 4 2127.94 (0.00 pct) 2141.49 (0.63 pct)
> 8 3838.27 (0.00 pct) 3721.10 (-3.05 pct)
> 16 6272.71 (0.00 pct) 6539.82 (4.25 pct)
> 32 11400.12 (0.00 pct) 12079.49 (5.95 pct)
> 64 21605.96 (0.00 pct) 22908.83 (6.03 pct)
> 128 30715.43 (0.00 pct) 31736.95 (3.32 pct)
> 256 55580.78 (0.00 pct) 54786.29 (-1.42 pct)
> 512 56528.79 (0.00 pct) 56453.54 (-0.13 pct)
> 1024 56520.40 (0.00 pct) 56369.93 (-0.26 pct)
>
> NPS2
>
> Clients: tip latency_nice
> 1 584.13 (0.00 pct) 582.53 (-0.27 pct)
> 2 1153.63 (0.00 pct) 1140.27 (-1.15 pct)
> 4 2212.89 (0.00 pct) 2159.49 (-2.41 pct)
> 8 3871.35 (0.00 pct) 3840.77 (-0.78 pct)
> 16 6216.72 (0.00 pct) 6437.98 (3.55 pct)
> 32 11766.98 (0.00 pct) 11663.53 (-0.87 pct)
> 64 22000.93 (0.00 pct) 21882.88 (-0.53 pct)
> 128 31520.53 (0.00 pct) 31147.05 (-1.18 pct)
> 256 51420.11 (0.00 pct) 55216.39 (7.38 pct)
> 512 53935.90 (0.00 pct) 55407.60 (2.72 pct)
> 1024 55239.73 (0.00 pct) 55997.25 (1.37 pct)
>
> NPS4
>
> Clients: tip latency_nice
> 1 585.83 (0.00 pct) 578.17 (-1.30 pct)
> 2 1141.59 (0.00 pct) 1131.14 (-0.91 pct)
> 4 2174.79 (0.00 pct) 2086.52 (-4.05 pct)
> 8 3887.56 (0.00 pct) 3778.47 (-2.80 pct)
> 16 6441.59 (0.00 pct) 6364.30 (-1.19 pct)
> 32 12133.60 (0.00 pct) 11465.26 (-5.50 pct) *
> 32 11677.16 (0.00 pct) 12662.09 (8.43 pct) [Verification Run]
> 64 21769.15 (0.00 pct) 19488.45 (-10.47 pct) *
> 64 20305.64 (0.00 pct) 21002.90 (3.43 pct) [Verification Run]
> 128 31396.31 (0.00 pct) 31177.37 (-0.69 pct)
> 256 52792.39 (0.00 pct) 52890.41 (0.18 pct)
> 512 55315.44 (0.00 pct) 53572.65 (-3.15 pct)
> 1024 52150.27 (0.00 pct) 54079.48 (3.69 pct)
>
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ stream - DEFAULT_LATENCY_NICE ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> NPS1
>
> 10 Runs:
>
> Test: tip latency_nice
> Copy: 307827.79 (0.00 pct) 330524.48 (7.37 pct)
> Scale: 208872.28 (0.00 pct) 215002.06 (2.93 pct)
> Add: 239404.64 (0.00 pct) 230334.74 (-3.78 pct)
> Triad: 247258.30 (0.00 pct) 238505.06 (-3.54 pct)
>
> 100 Runs:
>
> Test: tip latency_nice
> Copy: 317217.55 (0.00 pct) 314467.62 (-0.86 pct)
> Scale: 208740.82 (0.00 pct) 210452.00 (0.81 pct)
> Add: 240550.63 (0.00 pct) 232376.03 (-3.39 pct)
> Triad: 249594.21 (0.00 pct) 242460.83 (-2.85 pct)
>
> NPS2
>
> 10 Runs:
>
> Test: tip latency_nice
> Copy: 340877.18 (0.00 pct) 339441.26 (-0.42 pct)
> Scale: 217318.16 (0.00 pct) 216905.49 (-0.18 pct)
> Add: 259078.93 (0.00 pct) 261686.67 (1.00 pct)
> Triad: 274500.78 (0.00 pct) 271699.83 (-1.02 pct)
>
> 100 Runs:
>
> Test: tip latency_nice
> Copy: 341860.73 (0.00 pct) 335826.36 (-1.76 pct)
> Scale: 218043.00 (0.00 pct) 216451.84 (-0.72 pct)
> Add: 253698.22 (0.00 pct) 257317.72 (1.42 pct)
> Triad: 265011.84 (0.00 pct) 267769.93 (1.04 pct)
>
> NPS4
>
> 10 Runs:
>
> Test: tip latency_nice
> Copy: 340877.18 (0.00 pct) 365921.51 (7.34 pct)
> Scale: 217318.16 (0.00 pct) 239408.65 (10.16 pct)
> Add: 259078.93 (0.00 pct) 264859.31 (2.23 pct)
> Triad: 274500.78 (0.00 pct) 281543.65 (2.56 pct)
>
> 100 Runs:
>
> Test: tip latency_nice
> Copy: 341860.73 (0.00 pct) 359255.16 (5.08 pct)
> Scale: 218043.00 (0.00 pct) 238154.15 (9.22 pct)
> Add: 253698.22 (0.00 pct) 269223.49 (6.11 pct)
> Triad: 265011.84 (0.00 pct) 278473.85 (5.07 pct)
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ ycsb-mongodb - DEFAULT_LATENCY_NICE ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> o NPS1
>
> tip: 131244.00 (var: 2.67%)
> latency_nice: 132118.00 (var: 3.62%) (+0.66%)
>
> o NPS2
>
> tip: 127663.33 (var: 2.08%)
> latency_nice: 129148.00 (var: 4.29%) (+1.16%)
>
> o NPS4
>
> tip: 133295.00 (var: 1.58%)
> latency_nice: 129975.33 (var: 1.10%) (-2.49%)
>
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ Unixbench - DEFAULT_LATENCY_NICE ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> o NPS1
>
> Test Metric Parallelism tip latency_nice
> unixbench-dhry2reg Hmean unixbench-dhry2reg-1 48929419.48 ( 0.00%) 49137039.06 ( 0.42%)
> unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6275526953.25 ( 0.00%) 6265580479.15 ( -0.16%)
> unixbench-syscall Amean unixbench-syscall-1 2994319.73 ( 0.00%) 3008596.83 * -0.48%*
> unixbench-syscall Amean unixbench-syscall-512 7349715.87 ( 0.00%) 7420994.50 * -0.97%*
> unixbench-pipe Hmean unixbench-pipe-1 2830206.03 ( 0.00%) 2854405.99 * 0.86%*
> unixbench-pipe Hmean unixbench-pipe-512 326207828.01 ( 0.00%) 328997804.52 * 0.86%*
> unixbench-spawn Hmean unixbench-spawn-1 6394.21 ( 0.00%) 6367.75 ( -0.41%)
> unixbench-spawn Hmean unixbench-spawn-512 72700.64 ( 0.00%) 71454.19 * -1.71%*
> unixbench-execl Hmean unixbench-execl-1 4723.61 ( 0.00%) 4750.59 ( 0.57%)
> unixbench-execl Hmean unixbench-execl-512 11212.05 ( 0.00%) 11262.13 ( 0.45%)
>
> o NPS2
>
> Test Metric Parallelism tip latency_nice
> unixbench-dhry2reg Hmean unixbench-dhry2reg-1 49271512.85 ( 0.00%) 49245260.43 ( -0.05%)
> unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6267992483.03 ( 0.00%) 6264951100.67 ( -0.05%)
> unixbench-syscall Amean unixbench-syscall-1 2995885.93 ( 0.00%) 3005975.10 * -0.34%*
> unixbench-syscall Amean unixbench-syscall-512 7388865.77 ( 0.00%) 7276275.63 * 1.52%*
> unixbench-pipe Hmean unixbench-pipe-1 2828971.95 ( 0.00%) 2856578.72 * 0.98%*
> unixbench-pipe Hmean unixbench-pipe-512 326225385.37 ( 0.00%) 328941270.81 * 0.83%*
> unixbench-spawn Hmean unixbench-spawn-1 6958.71 ( 0.00%) 6954.21 ( -0.06%)
> unixbench-spawn Hmean unixbench-spawn-512 85443.56 ( 0.00%) 70536.42 * -17.45%* (0.67% vs 0.93% - CoEff var)

I don't expect any perf improvement or regression when the latency
nice is not changed

> unixbench-execl Hmean unixbench-execl-1 4767.99 ( 0.00%) 4752.63 * -0.32%*
> unixbench-execl Hmean unixbench-execl-512 11250.72 ( 0.00%) 11320.97 ( 0.62%)
>
> o NPS4
>
> Test Metric Parallelism tip latency_nice
> unixbench-dhry2reg Hmean unixbench-dhry2reg-1 49041932.68 ( 0.00%) 49156671.05 ( 0.23%)
> unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6286981589.85 ( 0.00%) 6285248711.40 ( -0.03%)
> unixbench-syscall Amean unixbench-syscall-1 2992405.60 ( 0.00%) 3008933.03 * -0.55%*
> unixbench-syscall Amean unixbench-syscall-512 7971789.70 ( 0.00%) 7814622.23 * 1.97%*
> unixbench-pipe Hmean unixbench-pipe-1 2822892.54 ( 0.00%) 2852615.11 * 1.05%*
> unixbench-pipe Hmean unixbench-pipe-512 326408309.83 ( 0.00%) 329617202.56 * 0.98%*
> unixbench-spawn Hmean unixbench-spawn-1 7685.31 ( 0.00%) 7243.54 ( -5.75%)
> unixbench-spawn Hmean unixbench-spawn-512 72245.56 ( 0.00%) 77000.81 * 6.58%*
> unixbench-execl Hmean unixbench-execl-1 4761.42 ( 0.00%) 4733.12 * -0.59%*
> unixbench-execl Hmean unixbench-execl-512 11533.53 ( 0.00%) 11660.17 ( 1.10%)
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ Hackbench - Various Latency Nice Values ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> o 100000 loops
>
> - pipe (process)
>
> Test: LN: 0 LN: 19 LN: -20
> 1-groups: 3.91 (0.00 pct) 3.91 (0.00 pct) 3.81 (2.55 pct)
> 2-groups: 4.48 (0.00 pct) 4.52 (-0.89 pct) 4.53 (-1.11 pct)
> 4-groups: 4.83 (0.00 pct) 4.83 (0.00 pct) 4.87 (-0.82 pct)
> 8-groups: 5.09 (0.00 pct) 5.00 (1.76 pct) 5.07 (0.39 pct)
> 16-groups: 6.92 (0.00 pct) 6.79 (1.87 pct) 6.96 (-0.57 pct)
>
> - pipe (thread)
>
> 1-groups: 4.13 (0.00 pct) 4.08 (1.21 pct) 4.11 (0.48 pct)
> 2-groups: 4.78 (0.00 pct) 4.90 (-2.51 pct) 4.79 (-0.20 pct)
> 4-groups: 5.12 (0.00 pct) 5.08 (0.78 pct) 5.16 (-0.78 pct)
> 8-groups: 5.31 (0.00 pct) 5.28 (0.56 pct) 5.33 (-0.37 pct)
> 16-groups: 7.34 (0.00 pct) 7.27 (0.95 pct) 7.33 (0.13 pct)
>
> - socket (process)
>
> Test: LN: 0 LN: 19 LN: -20
> 1-groups: 6.61 (0.00 pct) 6.38 (3.47 pct) 6.54 (1.05 pct)
> 2-groups: 6.59 (0.00 pct) 6.67 (-1.21 pct) 6.11 (7.28 pct)
> 4-groups: 6.77 (0.00 pct) 6.78 (-0.14 pct) 6.79 (-0.29 pct)
> 8-groups: 8.29 (0.00 pct) 8.39 (-1.20 pct) 8.36 (-0.84 pct)
> 16-groups: 12.21 (0.00 pct) 12.03 (1.47 pct) 12.35 (-1.14 pct)
>
> - socket (thread)
>
> Test: LN: 0 LN: 19 LN: -20
> 1-groups: 6.50 (0.00 pct) 5.99 (7.84 pct) 6.02 (7.38 pct) ^
> 2-groups: 6.07 (0.00 pct) 6.20 (-2.14 pct) 6.23 (-2.63 pct)
> 4-groups: 6.61 (0.00 pct) 6.64 (-0.45 pct) 6.63 (-0.30 pct)
> 8-groups: 8.87 (0.00 pct) 8.67 (2.25 pct) 8.78 (1.01 pct)
> 16-groups: 12.63 (0.00 pct) 12.54 (0.71 pct) 12.59 (0.31 pct)
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ Hackbench + Cyclictest - Various Latency Nice Values ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> - Hackbench: 32 Groups
>
> perf bench sched messaging -p -l 100000 -g 32&
> cyclictest --policy other -D 5 -q -n -h 2000
>
> o NPS1
>
> ----------------------------------------------------------------------------------------------------------
> | Hackbench | Cyclictest LN = 19 | Cyclictest LN = 0 | Cyclictest LN = -20 |
> | LN |------------------------------|-------------------------------|---------------------------|
> | | Min | Avg | Max | Min | Avg | Max | Min | Avg | Max |
> |-------------|--------|---------|-----------|--------|---------|------------|--------|-------|----------|
> | 19 | 52.00 | 71.00 | 5191.00 | 29.00 | 68.00 | 4477.00 | 53.00 | 60.00 | 753.00 |
> | 0 | 53.00 | 150.00 | 7300.00 | 53.00 | 105.00 | 7730.00 | 53.00 | 64.00 | 2067.00 |
> | -20 | 33.00 | 159.00 | 98492.00 | 53.00 | 149.00 | 9608.00 | 53.00 | 91.00 | 5349.00 |
> ----------------------------------------------------------------------------------------------------------
>
> o NPS4
>
> ----------------------------------------------------------------------------------------------------------
> | Hackbench | Cyclictest LN = 19 | Cyclictest LN = 0 | Cyclictest LN = -20 |
> | LN |------------------------------|-------------------------------|---------------------------|
> | | Min | Avg | Max | Min | Avg | Max | Min | Avg | Max |
> |-------------|--------|---------|-----------|--------|---------|------------|--------|-------|----------|
> | 19 | 53.00 | 84.00 | 4790.00 | 53.00 | 72.00 | 3456.00 | 53.00 | 58.00 | 1271.00 |
> | 0 | 53.00 | 99.00 | 5494.00 | 52.00 | 74.00 | 5813.00 | 53.00 | 59.00 | 1004.00 |
> | -20 | 45.00 | 84.00 | 3592.00 | 53.00 | 91.00 | 15222.00 | 53.00 | 74.00 | 5232.00 | ^
> ----------------------------------------------------------------------------------------------------------
>
> - Hackbench: 128 Groups
>
> perf bench sched messaging -p -l 500000 -g 128&
> cyclictest --policy other -D 5 -q -n -h 2000
>
> o NPS1
>
> ----------------------------------------------------------------------------------------------------------
> | Hackbench | Cyclictest LN = 19 | Cyclictest LN = 0 | Cyclictest LN = -20 |
> | LN |------------------------------|-------------------------------|---------------------------|
> | | Min | Avg | Max | Min | Avg | Max | Min | Avg | Max |
> |-------------|--------|---------|-----------|--------|---------|------------|--------|-------|----------|
> | 19 | 53.00 | 274.00 | 11294.00 | 33.00 | 130.00 | 20071.00 | 53.00 | 56.00 | 244.00 | ^
> | 0 | 53.00 | 125.00 | 10014.00 | 53.00 | 113.00 | 15857.00 | 53.00 | 57.00 | 250.00 |
> | -20 | 53.00 | 187.00 | 49565.00 | 53.00 | 230.00 | 73353.00 | 53.00 | 118.00| 8816.00 |
> ----------------------------------------------------------------------------------------------------------
>
> o NPS4
>
> ----------------------------------------------------------------------------------------------------------
> | Hackbench | Cyclictest LN = 19 | Cyclictest LN = 0 | Cyclictest LN = -20 |
> | LN |------------------------------|-------------------------------|---------------------------|
> | | Min | Avg | Max | Min | Avg | Max | Min | Avg | Max |
> |-------------|--------|---------|-----------|--------|---------|------------|--------|-------|----------|
> | 19 | 53.00 | 271.00 | 11411.00 | 53.00 | 82.00 | 5486.00 | 25.00 | 57.00 | 1256.00 |
> | 0 | 53.00 | 148.00 | 8374.00 | 52.00 | 109.00 | 11074.00 | 52.00 | 59.00 | 1068.00 |
> | -20 | 53.00 | 202.00 | 52537.00 | 53.00 | 205.00 | 22265.00 | 52.00 | 87.00 | 14151.00 |
> ----------------------------------------------------------------------------------------------------------
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~ Hackbench + schbench - Various Latency Nice Values ~
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> perf bench sched messaging -p -l 400000 -g 128
> schbench -m 2 -t 1 -s 30
>
> o NPS1
>
> -------------------------------------------------------------------------------------------------
> | Hackbench | schbench LN = 19 | schbench LN = 0 | schbench LN = -20 |
> | LN |----------------------------|---------------------------|--------------------------|
> | | 90th | 95th | 99th | 90th | 95th | 99th | 90th | 95th | 99th |
> |-----------|--------|--------|----------|--------|--------|---------|--------|--------|--------|
> | 19 | 38 | 131 | 1458 | 46 | 151 | 2636 | 11 | 19 | 410 | ^
> | 0 | 45 | 98 | 1758 | 25 | 50 | 1670 | 16 | 30 | 1042 |
> | -20 | 47 | 348 | 29280 | 40 | 109 | 16144 | 35 | 63 | 9104 |
> -------------------------------------------------------------------------------------------------
>
> o NPS4
>
> -------------------------------------------------------------------------------------------------
> | Hackbench | schbench LN = 19 | schbench LN = 0 | schbench LN = -20 |
> | LN |----------------------------|---------------------------|--------------------------|
> | | 90th | 95th | 99th | 90th | 95th | 99th | 90th | 95th | 99th |
> |-----------|--------|--------|----------|--------|--------|---------|--------|--------|--------|
> | 19 | 19 | 60 | 1886 | 17 | 29 | 621 | 10 | 18 | 227 |
> | 0 | 51 | 141 | 8120 | 37 | 78 | 8880 | 33 | 55 | 474 | ^
> | -20 | 48 | 1494 | 27296 | 51 | 469 | 40384 | 31 | 64 | 4092 | ^
> -------------------------------------------------------------------------------------------------
>
> ^ Note: There are cases where the Max, 99th percentile latency is
> non-monotonic but I've also seen a good amount of run to run variation
> there with a single bad sample polluting the results. In such cases,
> the averages are more representative.
>
> >
> > [1] https://source.android.com/docs/core/debug/eval_perf#touchlatency
> >
> > [..snip..]
> >
>
> Apart from couple of anomalies, latency nice reduces wait time, especially
> when the system is heavily loaded. If there is any data, or any specific
> workload you would like me to run on the test system, please do let me know.
> Meanwhile, I'll try to get some numbers for larger workloads like SpecJBB
> that did see improvements with latency nice on v5.

Thanks for your tests

Vincent

> --
> Thanks and Regards,
> Prateek

2022-11-29 04:32:21

by Joel Fernandes

Subject: Re: [PATCH 5/9] sched/fair: Take into account latency priority at wakeup

On Tue, Nov 15, 2022 at 12:19 PM Vincent Guittot
<[email protected]> wrote:
>
> Take into account the latency priority of a thread when deciding to
> preempt the current running thread. We don't want to provide more CPU
> bandwidth to a thread but reorder the scheduling to run latency sensitive
> task first whenever possible.
>
> As long as a thread didn't use its bandwidth, it will be able to preempt
> the current thread.
>
> At the opposite, a thread with a low latency priority will preempt current
> thread at wakeup only to keep fair CPU bandwidth sharing. Otherwise it will
> wait for the tick to get its sched slice.
>
> curr vruntime
> |
> sysctl_sched_wakeup_granularity
> <-->
> ----------------------------------|----|-----------------------|---------------
> | |<--------------------->
> | . sysctl_sched_latency
> | .
> default/current latency entity | .
> | .
> 1111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-
> se preempts curr at wakeup ------>|<- se doesn't preempt curr -----------------
> | .
> | .
> | .
> low latency entity | .
> ---------------------->|
> % of sysctl_sched_latency |
> 1111111111111111111111111111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-
> preempt ------------------------------------------------->|<- do not preempt --
> | .
> | .
> | .
> high latency entity | .
> |<-----------------------|----.
> | % of sysctl_sched_latency .
> 111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1
> preempt->|<- se doesn't preempt curr ------------------------------------------
> Tests results of nice latency impact on heavy load like hackbench:
>
> hackbench -l (2560 / group) -g group
> group latency 0 latency 19
> 1 1.378(+/- 1%) 1.337(+/- 1%) + 3%
> 4 1.393(+/- 3%) 1.312(+/- 3%) + 6%
> 8 1.308(+/- 2%) 1.279(+/- 1%) + 2%
> 16 1.347(+/- 1%) 1.317(+/- 1%) + 2%
>
> hackbench -p -l (2560 / group) -g group
> group
> 1 1.836(+/- 17%) 1.148(+/- 5%) +37%
> 4 1.586(+/- 6%) 1.109(+/- 8%) +30%
> 8 1.209(+/- 4%) 0.780(+/- 4%) +35%
> 16 0.805(+/- 5%) 0.728(+/- 4%) +10%
>
> By deacreasing the latency prio, we reduce the number of preemption at
> wakeup and help hackbench making progress.
>
> Test results of nice latency impact on short live load like cyclictest
> while competing with heavy load like hackbench:
>
> hackbench -l 10000 -g $group &
> cyclictest --policy other -D 5 -q -n
> latency 0 latency -20
> group min avg max min avg max
> 0 16 19 29 17 18 29
> 1 43 299 7359 63 84 3422
> 4 56 449 14806 45 83 284
> 8 63 820 51123 63 83 283
> 16 64 1326 70684 41 157 26852
>
> group = 0 means that hackbench is not running.
>
> The avg is significantly improved with nice latency -20 especially with
> large number of groups but min and max remain quite similar. If we add the
> histogram parameter to get details of latency, we have :
>
> hackbench -l 10000 -g 16 &
> cyclictest --policy other -D 5 -q -n -H 20000 --histfile data.txt
> latency 0 latency -20
> Min Latencies: 64 62
> Avg Latencies: 1170 107
> Max Latencies: 88069 10417
> 50% latencies: 122 86
> 75% latencies: 614 91
> 85% latencies: 961 94
> 90% latencies: 1225 97
> 95% latencies: 6120 102
> 99% latencies: 18328 159
>
> With percentile details, we see the benefit of nice latency -20 as
> only 1% of the latencies are above 159us whereas the default latency
> has got 15% around ~1ms or above and 5% over the 6ms.
>
> Signed-off-by: Vincent Guittot <[email protected]>
> ---
> include/linux/sched.h | 4 ++-
> include/linux/sched/prio.h | 9 ++++++
> init/init_task.c | 2 +-
> kernel/sched/core.c | 38 +++++++++++++++++++---
> kernel/sched/debug.c | 2 +-
> kernel/sched/fair.c | 66 ++++++++++++++++++++++++++++++++++----
> kernel/sched/sched.h | 6 ++++
> 7 files changed, 112 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 856240573300..2f33326adb8d 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -568,6 +568,8 @@ struct sched_entity {
> /* cached value of my_q->h_nr_running */
> unsigned long runnable_weight;
> #endif
> + /* preemption offset in ns */
> + long latency_offset;
>
> #ifdef CONFIG_SMP
> /*
> @@ -784,7 +786,7 @@ struct task_struct {
> int static_prio;
> int normal_prio;
> unsigned int rt_priority;
> - int latency_nice;
> + int latency_prio;
>
> struct sched_entity se;
> struct sched_rt_entity rt;
> diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
> index bfcd7f1d1e11..be79503d86af 100644
> --- a/include/linux/sched/prio.h
> +++ b/include/linux/sched/prio.h
> @@ -59,5 +59,14 @@ static inline long rlimit_to_nice(long prio)
> * Default tasks should be treated as a task with latency_nice = 0.
> */
> #define DEFAULT_LATENCY_NICE 0
> +#define DEFAULT_LATENCY_PRIO (DEFAULT_LATENCY_NICE + LATENCY_NICE_WIDTH/2)
> +
> +/*
> + * Convert user-nice values [ -20 ... 0 ... 19 ]
> + * to static latency [ 0..39 ],
> + * and back.
> + */
> +#define NICE_TO_LATENCY(nice) ((nice) + DEFAULT_LATENCY_PRIO)
> +#define LATENCY_TO_NICE(prio) ((prio) - DEFAULT_LATENCY_PRIO)
>
> #endif /* _LINUX_SCHED_PRIO_H */
> diff --git a/init/init_task.c b/init/init_task.c
> index 7dd71dd2d261..071deff8dbd1 100644
> --- a/init/init_task.c
> +++ b/init/init_task.c
> @@ -78,7 +78,7 @@ struct task_struct init_task
> .prio = MAX_PRIO - 20,
> .static_prio = MAX_PRIO - 20,
> .normal_prio = MAX_PRIO - 20,
> - .latency_nice = DEFAULT_LATENCY_NICE,
> + .latency_prio = DEFAULT_LATENCY_PRIO,
> .policy = SCHED_NORMAL,
> .cpus_ptr = &init_task.cpus_mask,
> .user_cpus_ptr = NULL,
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 18c31a68eb18..b2b8cb6c08cd 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1283,6 +1283,16 @@ static void set_load_weight(struct task_struct *p, bool update_load)
> }
> }
>
> +static void set_latency_offset(struct task_struct *p)
> +{
> + long weight = sched_latency_to_weight[p->latency_prio];
> + s64 offset;
> +
> + offset = weight * get_sleep_latency(false);
> + offset = div_s64(offset, NICE_LATENCY_WEIGHT_MAX);
> + p->se.latency_offset = (long)offset;
> +}
> +
> #ifdef CONFIG_UCLAMP_TASK
> /*
> * Serializes updates of utilization clamp values
> @@ -4592,7 +4602,9 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
> p->prio = p->normal_prio = p->static_prio;
> set_load_weight(p, false);
>
> - p->latency_nice = DEFAULT_LATENCY_NICE;
> + p->latency_prio = NICE_TO_LATENCY(0);
> + set_latency_offset(p);
> +
> /*
> * We don't need the reset flag anymore after the fork. It has
> * fulfilled its duty:
> @@ -7358,8 +7370,10 @@ static void __setscheduler_params(struct task_struct *p,
> static void __setscheduler_latency(struct task_struct *p,
> const struct sched_attr *attr)
> {
> - if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE)
> - p->latency_nice = attr->sched_latency_nice;
> + if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE) {
> + p->latency_prio = NICE_TO_LATENCY(attr->sched_latency_nice);
> + set_latency_offset(p);
> + }
> }
>
> /*
> @@ -7544,7 +7558,7 @@ static int __sched_setscheduler(struct task_struct *p,
> if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
> goto change;
> if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE &&
> - attr->sched_latency_nice != p->latency_nice)
> + attr->sched_latency_nice != LATENCY_TO_NICE(p->latency_prio))
> goto change;
>
> p->sched_reset_on_fork = reset_on_fork;
> @@ -8085,7 +8099,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
> get_params(p, &kattr);
> kattr.sched_flags &= SCHED_FLAG_ALL;
>
> - kattr.sched_latency_nice = p->latency_nice;
> + kattr.sched_latency_nice = LATENCY_TO_NICE(p->latency_prio);
>
> #ifdef CONFIG_UCLAMP_TASK
> /*
> @@ -11294,6 +11308,20 @@ const u32 sched_prio_to_wmult[40] = {
> /* 15 */ 119304647, 148102320, 186737708, 238609294, 286331153,
> };
>
> +/*
> + * latency weight for wakeup preemption
> + */
> +const int sched_latency_to_weight[40] = {
> + /* -20 */ -1024, -973, -922, -870, -819,
> + /* -15 */ -768, -717, -666, -614, -563,
> + /* -10 */ -512, -461, -410, -358, -307,
> + /* -5 */ -256, -205, -154, -102, -51,
> + /* 0 */ 0, 51, 102, 154, 205,
> + /* 5 */ 256, 307, 358, 410, 461,
> + /* 10 */ 512, 563, 614, 666, 717,
> + /* 15 */ 768, 819, 870, 922, 973,
> +};
> +

The table is linear. You could approximate this as: weight = nice * 51
since it is a linear scale and do the conversion in place.

Or, since the only place you are using the latency_to_weight is in
set_latency_offset(), can we drop the sched_latency_to_weight array
and simplify as follows?

static void set_latency_offset(struct task_struct *p)
{
s64 offset = p->latency_prio * get_sleep_latency(false);
p->latency_prio = (long)div_s64(offset, 40);
}

> void call_trace_sched_update_nr_running(struct rq *rq, int count)
> {
> trace_sched_update_nr_running_tp(rq, count);
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index 68be7a3e42a3..b3922184af91 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -1043,7 +1043,7 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
> #endif
> P(policy);
> P(prio);
> - P(latency_nice);
> + P(latency_prio);
> if (task_has_dl_policy(p)) {
> P(dl.runtime);
> P(dl.deadline);
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index c8a697f8db88..0e80e65113bd 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4858,6 +4858,8 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> update_idle_cfs_rq_clock_pelt(cfs_rq);
> }
>
> +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se);
> +
> /*
> * Preempt the current task with a newly woken task if needed:
> */
> @@ -4866,7 +4868,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> {
> unsigned long ideal_runtime, delta_exec;
> struct sched_entity *se;
> - s64 delta;
> + s64 delta, offset;
>
> ideal_runtime = sched_slice(cfs_rq, curr);
> delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
> @@ -4891,10 +4893,12 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> se = __pick_first_entity(cfs_rq);
> delta = curr->vruntime - se->vruntime;
>
> - if (delta < 0)
> + offset = wakeup_latency_gran(curr, se);
> + if (delta < offset)
> return;

Agreed.

> - if (delta > ideal_runtime)
> + if ((delta > ideal_runtime) ||
> + (delta > get_latency_max()))
> resched_curr(rq_of(cfs_rq));
> }
>
> @@ -6019,6 +6023,35 @@ static int sched_idle_cpu(int cpu)
> }
> #endif
>
> +static void set_next_buddy(struct sched_entity *se);
> +
> +static void check_preempt_from_others(struct cfs_rq *cfs, struct sched_entity *se)
> +{
> + struct sched_entity *next;
> +
> + if (se->latency_offset >= 0)
> + return;
> +
> + if (cfs->nr_running <= 1)
> + return;
> + /*
> + * When waking from another class, we don't need to check to preempt at
> + * wakeup and don't set next buddy as a candidate for being picked in
> + * priority.
> + * In case of simultaneous wakeup when current is another class, the
> + * latency sensitive tasks lost opportunity to preempt non sensitive
> + * tasks which woke up simultaneously.
> + */
> +
> + if (cfs->next)
> + next = cfs->next;
> + else
> + next = __pick_first_entity(cfs);
> +
> + if (next && wakeup_preempt_entity(next, se) == 1)
> + set_next_buddy(se);
> +}
> +
> /*
> * The enqueue_task method is called before nr_running is
> * increased. Here we update the fair scheduling stats and
> @@ -6105,14 +6138,15 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> if (!task_new)
> update_overutilized_status(rq);
>
> + if (rq->curr->sched_class != &fair_sched_class)
> + check_preempt_from_others(cfs_rq_of(&p->se), &p->se);
> +
> enqueue_throttle:
> assert_list_leaf_cfs_rq(rq);
>
> hrtick_update(rq);
> }
>
> -static void set_next_buddy(struct sched_entity *se);
> -
> /*
> * The dequeue_task method is called before nr_running is
> * decreased. We remove the task from the rbtree and
> @@ -7461,6 +7495,23 @@ balance_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
> }
> #endif /* CONFIG_SMP */
>
> +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se)
> +{
> + long latency_offset = se->latency_offset;
> +
> + /*
> + * A negative latency offset means that the sched_entity has latency
> + * requirement that needs to be evaluated versus other entity.
> + * Otherwise, use the latency weight to evaluate how much scheduling
> + * delay is acceptable by se.
> + */
> + if ((latency_offset < 0) || (curr->latency_offset < 0))
> + latency_offset -= curr->latency_offset;
> + latency_offset = min_t(long, latency_offset, get_latency_max());

Over here can we make positive latency offsets also be evaluated
"versus other entity"?

It feels strange to have different rules for positive latency_offset
when comparing curr and se. IMO we should also factor in latency
requirements by comparing 2 positive nice values. It should be
relative even for positive values, just like regular nice IMO and not
have hidden meaning. If there is hidden meaning, it confuses the user
and requires documentation that most users will not read. Especially
because latency_nice shares the word "nice" with regular nice values.

Thanks,

- Joel
.

> +
> + return latency_offset;
> +}
> +
> static unsigned long wakeup_gran(struct sched_entity *se)
> {
> unsigned long gran = sysctl_sched_wakeup_granularity;
> @@ -7499,11 +7550,12 @@ static int
> wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se)
> {
> s64 gran, vdiff = curr->vruntime - se->vruntime;
> + s64 offset = wakeup_latency_gran(curr, se);
>
> - if (vdiff <= 0)
> + if (vdiff < offset)
> return -1;
>
> - gran = wakeup_gran(se);
> + gran = offset + wakeup_gran(se);
>
> /*
> * At wake up, the vruntime of a task is capped to not be older than
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 842ce0094d9c..7292652731d0 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -125,6 +125,11 @@ extern int sched_rr_timeslice;
> */
> #define NS_TO_JIFFIES(TIME) ((unsigned long)(TIME) / (NSEC_PER_SEC / HZ))
>
> +/* Maximum nice latency weight used to scale the latency_offset */
> +
> +#define NICE_LATENCY_SHIFT (SCHED_FIXEDPOINT_SHIFT)
> +#define NICE_LATENCY_WEIGHT_MAX (1L << NICE_LATENCY_SHIFT)
> +
> /*
> * Increase resolution of nice-level calculations for 64-bit architectures.
> * The extra resolution improves shares distribution and load balancing of
> @@ -2115,6 +2120,7 @@ static_assert(WF_TTWU == SD_BALANCE_WAKE);
>
> extern const int sched_prio_to_weight[40];
> extern const u32 sched_prio_to_wmult[40];
> +extern const int sched_latency_to_weight[40];
>
> /*
> * {de,en}queue flags:
> --
> 2.17.1
>

2022-11-29 09:42:47

by Vincent Guittot

Subject: Re: [PATCH 5/9] sched/fair: Take into account latency priority at wakeup

On Tue, 29 Nov 2022 at 05:25, Joel Fernandes <[email protected]> wrote:
>
> On Tue, Nov 15, 2022 at 12:19 PM Vincent Guittot
> <[email protected]> wrote:
> >
> > Take into account the latency priority of a thread when deciding to
> > preempt the current running thread. We don't want to provide more CPU
> > bandwidth to a thread but reorder the scheduling to run latency sensitive
> > task first whenever possible.
> >
> > As long as a thread didn't use its bandwidth, it will be able to preempt
> > the current thread.
> >
> > At the opposite, a thread with a low latency priority will preempt current
> > thread at wakeup only to keep fair CPU bandwidth sharing. Otherwise it will
> > wait for the tick to get its sched slice.
> >
> > curr vruntime
> > |
> > sysctl_sched_wakeup_granularity
> > <-->
> > ----------------------------------|----|-----------------------|---------------
> > | |<--------------------->
> > | . sysctl_sched_latency
> > | .
> > default/current latency entity | .
> > | .
> > 1111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-
> > se preempts curr at wakeup ------>|<- se doesn't preempt curr -----------------
> > | .
> > | .
> > | .
> > low latency entity | .
> > ---------------------->|
> > % of sysctl_sched_latency |
> > 1111111111111111111111111111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-
> > preempt ------------------------------------------------->|<- do not preempt --
> > | .
> > | .
> > | .
> > high latency entity | .
> > |<-----------------------|----.
> > | % of sysctl_sched_latency .
> > 111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1
> > preempt->|<- se doesn't preempt curr ------------------------------------------
> > Tests results of nice latency impact on heavy load like hackbench:
> >
> > hackbench -l (2560 / group) -g group
> > group latency 0 latency 19
> > 1 1.378(+/- 1%) 1.337(+/- 1%) + 3%
> > 4 1.393(+/- 3%) 1.312(+/- 3%) + 6%
> > 8 1.308(+/- 2%) 1.279(+/- 1%) + 2%
> > 16 1.347(+/- 1%) 1.317(+/- 1%) + 2%
> >
> > hackbench -p -l (2560 / group) -g group
> > group
> > 1 1.836(+/- 17%) 1.148(+/- 5%) +37%
> > 4 1.586(+/- 6%) 1.109(+/- 8%) +30%
> > 8 1.209(+/- 4%) 0.780(+/- 4%) +35%
> > 16 0.805(+/- 5%) 0.728(+/- 4%) +10%
> >
> > By deacreasing the latency prio, we reduce the number of preemption at
> > wakeup and help hackbench making progress.
> >
> > Test results of nice latency impact on short live load like cyclictest
> > while competing with heavy load like hackbench:
> >
> > hackbench -l 10000 -g $group &
> > cyclictest --policy other -D 5 -q -n
> > latency 0 latency -20
> > group min avg max min avg max
> > 0 16 19 29 17 18 29
> > 1 43 299 7359 63 84 3422
> > 4 56 449 14806 45 83 284
> > 8 63 820 51123 63 83 283
> > 16 64 1326 70684 41 157 26852
> >
> > group = 0 means that hackbench is not running.
> >
> > The avg is significantly improved with nice latency -20 especially with
> > large number of groups but min and max remain quite similar. If we add the
> > histogram parameter to get details of latency, we have :
> >
> > hackbench -l 10000 -g 16 &
> > cyclictest --policy other -D 5 -q -n -H 20000 --histfile data.txt
> > latency 0 latency -20
> > Min Latencies: 64 62
> > Avg Latencies: 1170 107
> > Max Latencies: 88069 10417
> > 50% latencies: 122 86
> > 75% latencies: 614 91
> > 85% latencies: 961 94
> > 90% latencies: 1225 97
> > 95% latencies: 6120 102
> > 99% latencies: 18328 159
> >
> > With percentile details, we see the benefit of nice latency -20 as
> > only 1% of the latencies are above 159us whereas the default latency
> > has got 15% around ~1ms or above and 5% over the 6ms.
> >
> > Signed-off-by: Vincent Guittot <[email protected]>
> > ---
> > include/linux/sched.h | 4 ++-
> > include/linux/sched/prio.h | 9 ++++++
> > init/init_task.c | 2 +-
> > kernel/sched/core.c | 38 +++++++++++++++++++---
> > kernel/sched/debug.c | 2 +-
> > kernel/sched/fair.c | 66 ++++++++++++++++++++++++++++++++++----
> > kernel/sched/sched.h | 6 ++++
> > 7 files changed, 112 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 856240573300..2f33326adb8d 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -568,6 +568,8 @@ struct sched_entity {
> > /* cached value of my_q->h_nr_running */
> > unsigned long runnable_weight;
> > #endif
> > + /* preemption offset in ns */
> > + long latency_offset;
> >
> > #ifdef CONFIG_SMP
> > /*
> > @@ -784,7 +786,7 @@ struct task_struct {
> > int static_prio;
> > int normal_prio;
> > unsigned int rt_priority;
> > - int latency_nice;
> > + int latency_prio;
> >
> > struct sched_entity se;
> > struct sched_rt_entity rt;
> > diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
> > index bfcd7f1d1e11..be79503d86af 100644
> > --- a/include/linux/sched/prio.h
> > +++ b/include/linux/sched/prio.h
> > @@ -59,5 +59,14 @@ static inline long rlimit_to_nice(long prio)
> > * Default tasks should be treated as a task with latency_nice = 0.
> > */
> > #define DEFAULT_LATENCY_NICE 0
> > +#define DEFAULT_LATENCY_PRIO (DEFAULT_LATENCY_NICE + LATENCY_NICE_WIDTH/2)
> > +
> > +/*
> > + * Convert user-nice values [ -20 ... 0 ... 19 ]
> > + * to static latency [ 0..39 ],
> > + * and back.
> > + */
> > +#define NICE_TO_LATENCY(nice) ((nice) + DEFAULT_LATENCY_PRIO)
> > +#define LATENCY_TO_NICE(prio) ((prio) - DEFAULT_LATENCY_PRIO)
> >
> > #endif /* _LINUX_SCHED_PRIO_H */
> > diff --git a/init/init_task.c b/init/init_task.c
> > index 7dd71dd2d261..071deff8dbd1 100644
> > --- a/init/init_task.c
> > +++ b/init/init_task.c
> > @@ -78,7 +78,7 @@ struct task_struct init_task
> > .prio = MAX_PRIO - 20,
> > .static_prio = MAX_PRIO - 20,
> > .normal_prio = MAX_PRIO - 20,
> > - .latency_nice = DEFAULT_LATENCY_NICE,
> > + .latency_prio = DEFAULT_LATENCY_PRIO,
> > .policy = SCHED_NORMAL,
> > .cpus_ptr = &init_task.cpus_mask,
> > .user_cpus_ptr = NULL,
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 18c31a68eb18..b2b8cb6c08cd 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -1283,6 +1283,16 @@ static void set_load_weight(struct task_struct *p, bool update_load)
> > }
> > }
> >
> > +static void set_latency_offset(struct task_struct *p)
> > +{
> > + long weight = sched_latency_to_weight[p->latency_prio];
> > + s64 offset;
> > +
> > + offset = weight * get_sleep_latency(false);
> > + offset = div_s64(offset, NICE_LATENCY_WEIGHT_MAX);
> > + p->se.latency_offset = (long)offset;
> > +}
> > +
> > #ifdef CONFIG_UCLAMP_TASK
> > /*
> > * Serializes updates of utilization clamp values
> > @@ -4592,7 +4602,9 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
> > p->prio = p->normal_prio = p->static_prio;
> > set_load_weight(p, false);
> >
> > - p->latency_nice = DEFAULT_LATENCY_NICE;
> > + p->latency_prio = NICE_TO_LATENCY(0);
> > + set_latency_offset(p);
> > +
> > /*
> > * We don't need the reset flag anymore after the fork. It has
> > * fulfilled its duty:
> > @@ -7358,8 +7370,10 @@ static void __setscheduler_params(struct task_struct *p,
> > static void __setscheduler_latency(struct task_struct *p,
> > const struct sched_attr *attr)
> > {
> > - if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE)
> > - p->latency_nice = attr->sched_latency_nice;
> > + if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE) {
> > + p->latency_prio = NICE_TO_LATENCY(attr->sched_latency_nice);
> > + set_latency_offset(p);
> > + }
> > }
> >
> > /*
> > @@ -7544,7 +7558,7 @@ static int __sched_setscheduler(struct task_struct *p,
> > if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
> > goto change;
> > if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE &&
> > - attr->sched_latency_nice != p->latency_nice)
> > + attr->sched_latency_nice != LATENCY_TO_NICE(p->latency_prio))
> > goto change;
> >
> > p->sched_reset_on_fork = reset_on_fork;
> > @@ -8085,7 +8099,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
> > get_params(p, &kattr);
> > kattr.sched_flags &= SCHED_FLAG_ALL;
> >
> > - kattr.sched_latency_nice = p->latency_nice;
> > + kattr.sched_latency_nice = LATENCY_TO_NICE(p->latency_prio);
> >
> > #ifdef CONFIG_UCLAMP_TASK
> > /*
> > @@ -11294,6 +11308,20 @@ const u32 sched_prio_to_wmult[40] = {
> > /* 15 */ 119304647, 148102320, 186737708, 238609294, 286331153,
> > };
> >
> > +/*
> > + * latency weight for wakeup preemption
> > + */
> > +const int sched_latency_to_weight[40] = {
> > + /* -20 */ -1024, -973, -922, -870, -819,
> > + /* -15 */ -768, -717, -666, -614, -563,
> > + /* -10 */ -512, -461, -410, -358, -307,
> > + /* -5 */ -256, -205, -154, -102, -51,
> > + /* 0 */ 0, 51, 102, 154, 205,
> > + /* 5 */ 256, 307, 358, 410, 461,
> > + /* 10 */ 512, 563, 614, 666, 717,
> > + /* 15 */ 768, 819, 870, 922, 973,
> > +};
> > +
>
> The table is linear. You could approximate this as: weight = nice * 51
> since it is a linear scale and do the conversion in place.
>
> Or, since the only place you are using the latency_to_weight is in
> set_latency_offset(), can we drop the sched_latency_to_weight array
> and simplify as follows?

It's also used in cgroup patch and keeps a coherency between
nice/weight an latency_nice/offset so I prefer keeping current
implementation

>
> static void set_latency_offset(struct task_struct *p)
> {
> s64 offset = p->latency_prio * get_sleep_latency(false);
> p->latency_prio = (long)div_s64(offset, 40);
> }
>
> > void call_trace_sched_update_nr_running(struct rq *rq, int count)
> > {
> > trace_sched_update_nr_running_tp(rq, count);
> > diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> > index 68be7a3e42a3..b3922184af91 100644
> > --- a/kernel/sched/debug.c
> > +++ b/kernel/sched/debug.c
> > @@ -1043,7 +1043,7 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
> > #endif
> > P(policy);
> > P(prio);
> > - P(latency_nice);
> > + P(latency_prio);
> > if (task_has_dl_policy(p)) {
> > P(dl.runtime);
> > P(dl.deadline);
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index c8a697f8db88..0e80e65113bd 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -4858,6 +4858,8 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> > update_idle_cfs_rq_clock_pelt(cfs_rq);
> > }
> >
> > +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se);
> > +
> > /*
> > * Preempt the current task with a newly woken task if needed:
> > */
> > @@ -4866,7 +4868,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> > {
> > unsigned long ideal_runtime, delta_exec;
> > struct sched_entity *se;
> > - s64 delta;
> > + s64 delta, offset;
> >
> > ideal_runtime = sched_slice(cfs_rq, curr);
> > delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
> > @@ -4891,10 +4893,12 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> > se = __pick_first_entity(cfs_rq);
> > delta = curr->vruntime - se->vruntime;
> >
> > - if (delta < 0)
> > + offset = wakeup_latency_gran(curr, se);
> > + if (delta < offset)
> > return;
>
> Agreed.
>
> > - if (delta > ideal_runtime)
> > + if ((delta > ideal_runtime) ||
> > + (delta > get_latency_max()))
> > resched_curr(rq_of(cfs_rq));
> > }
> >
> > @@ -6019,6 +6023,35 @@ static int sched_idle_cpu(int cpu)
> > }
> > #endif
> >
> > +static void set_next_buddy(struct sched_entity *se);
> > +
> > +static void check_preempt_from_others(struct cfs_rq *cfs, struct sched_entity *se)
> > +{
> > + struct sched_entity *next;
> > +
> > + if (se->latency_offset >= 0)
> > + return;
> > +
> > + if (cfs->nr_running <= 1)
> > + return;
> > + /*
> > + * When waking from another class, we don't need to check to preempt at
> > + * wakeup and don't set next buddy as a candidate for being picked in
> > + * priority.
> > + * In case of simultaneous wakeup when current is another class, the
> > + * latency sensitive tasks lost opportunity to preempt non sensitive
> > + * tasks which woke up simultaneously.
> > + */
> > +
> > + if (cfs->next)
> > + next = cfs->next;
> > + else
> > + next = __pick_first_entity(cfs);
> > +
> > + if (next && wakeup_preempt_entity(next, se) == 1)
> > + set_next_buddy(se);
> > +}
> > +
> > /*
> > * The enqueue_task method is called before nr_running is
> > * increased. Here we update the fair scheduling stats and
> > @@ -6105,14 +6138,15 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > if (!task_new)
> > update_overutilized_status(rq);
> >
> > + if (rq->curr->sched_class != &fair_sched_class)
> > + check_preempt_from_others(cfs_rq_of(&p->se), &p->se);
> > +
> > enqueue_throttle:
> > assert_list_leaf_cfs_rq(rq);
> >
> > hrtick_update(rq);
> > }
> >
> > -static void set_next_buddy(struct sched_entity *se);
> > -
> > /*
> > * The dequeue_task method is called before nr_running is
> > * decreased. We remove the task from the rbtree and
> > @@ -7461,6 +7495,23 @@ balance_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
> > }
> > #endif /* CONFIG_SMP */
> >
> > +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se)
> > +{
> > + long latency_offset = se->latency_offset;
> > +
> > + /*
> > + * A negative latency offset means that the sched_entity has latency
> > + * requirement that needs to be evaluated versus other entity.
> > + * Otherwise, use the latency weight to evaluate how much scheduling
> > + * delay is acceptable by se.
> > + */
> > + if ((latency_offset < 0) || (curr->latency_offset < 0))
> > + latency_offset -= curr->latency_offset;
> > + latency_offset = min_t(long, latency_offset, get_latency_max());
>
> Over here can we make positive latency offsets also be evaluated
> "versus other entity"?
>
> It feels strange to have different rules for positive latency_offset
> when comparing curr and se. IMO we should also factor in latency
> requirements by comparing 2 positive nice values. It should be
> relative even for positive values, just like regular nice IMO and not
> have hidden meaning. If there is hidden meaning, it confuses the user
> and requires documentation that most users will not read. Especially
> because latency_nice shares the word "nice" with regular nice values.

This has already been discussed in the previous revisions. This is not
a hidden behavior but the normal behavior.

A negative latency nice, means that the task are not tolerant to
scheduling delay and it want to preempt current and run now. Or, if
the task is current, it doesn't want to be preempted and finish its
slice. In this case, we compare current and wake up task in case there
is 2 latency sensitive tasks that are fighting to run 1st.

Whereas a positive latency nice means that the task is tolerant to
scheduling delay and you don't care preempting current as long as it's
in an acceptable vruntime range. Why would the latency nice of the
current task make the wakeup task less tolerant to the scheduling
delay ? As an example, If current is latency_nice 19 and the wakeup
task is latency nice 19 too, both are tolerant to scheduling delay and
the waking up task should preempt current only if there is an
unfairness problem. By comparing their positive latency nice values,
you are back to the normal behavior which defeats the purpose of the
feature.

Thanks
Vincent

>
> Thanks,
>
> - Joel
> .
>
> > +
> > + return latency_offset;
> > +}
> > +
> > static unsigned long wakeup_gran(struct sched_entity *se)
> > {
> > unsigned long gran = sysctl_sched_wakeup_granularity;
> > @@ -7499,11 +7550,12 @@ static int
> > wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se)
> > {
> > s64 gran, vdiff = curr->vruntime - se->vruntime;
> > + s64 offset = wakeup_latency_gran(curr, se);
> >
> > - if (vdiff <= 0)
> > + if (vdiff < offset)
> > return -1;
> >
> > - gran = wakeup_gran(se);
> > + gran = offset + wakeup_gran(se);
> >
> > /*
> > * At wake up, the vruntime of a task is capped to not be older than
> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > index 842ce0094d9c..7292652731d0 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -125,6 +125,11 @@ extern int sched_rr_timeslice;
> > */
> > #define NS_TO_JIFFIES(TIME) ((unsigned long)(TIME) / (NSEC_PER_SEC / HZ))
> >
> > +/* Maximum nice latency weight used to scale the latency_offset */
> > +
> > +#define NICE_LATENCY_SHIFT (SCHED_FIXEDPOINT_SHIFT)
> > +#define NICE_LATENCY_WEIGHT_MAX (1L << NICE_LATENCY_SHIFT)
> > +
> > /*
> > * Increase resolution of nice-level calculations for 64-bit architectures.
> > * The extra resolution improves shares distribution and load balancing of
> > @@ -2115,6 +2120,7 @@ static_assert(WF_TTWU == SD_BALANCE_WAKE);
> >
> > extern const int sched_prio_to_weight[40];
> > extern const u32 sched_prio_to_wmult[40];
> > +extern const int sched_latency_to_weight[40];
> >
> > /*
> > * {de,en}queue flags:
> > --
> > 2.17.1
> >

2022-11-29 15:58:16

by Joel Fernandes

Subject: Re: [PATCH 5/9] sched/fair: Take into account latency priority at wakeup

Hi Vincent,
Thank you so much for your quick reply.

> On Nov 29, 2022, at 3:58 AM, Vincent Guittot <[email protected]> wrote:
>
> On Tue, 29 Nov 2022 at 05:25, Joel Fernandes <[email protected]> wrote:
>>
>>> On Tue, Nov 15, 2022 at 12:19 PM Vincent Guittot
>>> <[email protected]> wrote:
>>>
>>> Take into account the latency priority of a thread when deciding to
>>> preempt the current running thread. We don't want to provide more CPU
>>> bandwidth to a thread but reorder the scheduling to run latency sensitive
>>> task first whenever possible.
>>>
>>> As long as a thread didn't use its bandwidth, it will be able to preempt
>>> the current thread.
>>>
>>> At the opposite, a thread with a low latency priority will preempt current
>>> thread at wakeup only to keep fair CPU bandwidth sharing. Otherwise it will
>>> wait for the tick to get its sched slice.
>>>
>>> curr vruntime
>>> |
>>> sysctl_sched_wakeup_granularity
>>> <-->
>>> ----------------------------------|----|-----------------------|---------------
>>> | |<--------------------->
>>> | . sysctl_sched_latency
>>> | .
>>> default/current latency entity | .
>>> | .
>>> 1111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-
>>> se preempts curr at wakeup ------>|<- se doesn't preempt curr -----------------
>>> | .
>>> | .
>>> | .
>>> low latency entity | .
>>> ---------------------->|
>>> % of sysctl_sched_latency |
>>> 1111111111111111111111111111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-
>>> preempt ------------------------------------------------->|<- do not preempt --
>>> | .
>>> | .
>>> | .
>>> high latency entity | .
>>> |<-----------------------|----.
>>> | % of sysctl_sched_latency .
>>> 111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1
>>> preempt->|<- se doesn't preempt curr ------------------------------------------
>>> Tests results of nice latency impact on heavy load like hackbench:
>>>
>>> hackbench -l (2560 / group) -g group
>>> group latency 0 latency 19
>>> 1 1.378(+/- 1%) 1.337(+/- 1%) + 3%
>>> 4 1.393(+/- 3%) 1.312(+/- 3%) + 6%
>>> 8 1.308(+/- 2%) 1.279(+/- 1%) + 2%
>>> 16 1.347(+/- 1%) 1.317(+/- 1%) + 2%
>>>
>>> hackbench -p -l (2560 / group) -g group
>>> group
>>> 1 1.836(+/- 17%) 1.148(+/- 5%) +37%
>>> 4 1.586(+/- 6%) 1.109(+/- 8%) +30%
>>> 8 1.209(+/- 4%) 0.780(+/- 4%) +35%
>>> 16 0.805(+/- 5%) 0.728(+/- 4%) +10%
>>>
>>> By deacreasing the latency prio, we reduce the number of preemption at
>>> wakeup and help hackbench making progress.
>>>
>>> Test results of nice latency impact on short live load like cyclictest
>>> while competing with heavy load like hackbench:
>>>
>>> hackbench -l 10000 -g $group &
>>> cyclictest --policy other -D 5 -q -n
>>> latency 0 latency -20
>>> group min avg max min avg max
>>> 0 16 19 29 17 18 29
>>> 1 43 299 7359 63 84 3422
>>> 4 56 449 14806 45 83 284
>>> 8 63 820 51123 63 83 283
>>> 16 64 1326 70684 41 157 26852
>>>
>>> group = 0 means that hackbench is not running.
>>>
>>> The avg is significantly improved with nice latency -20 especially with
>>> large number of groups but min and max remain quite similar. If we add the
>>> histogram parameter to get details of latency, we have :
>>>
>>> hackbench -l 10000 -g 16 &
>>> cyclictest --policy other -D 5 -q -n -H 20000 --histfile data.txt
>>> latency 0 latency -20
>>> Min Latencies: 64 62
>>> Avg Latencies: 1170 107
>>> Max Latencies: 88069 10417
>>> 50% latencies: 122 86
>>> 75% latencies: 614 91
>>> 85% latencies: 961 94
>>> 90% latencies: 1225 97
>>> 95% latencies: 6120 102
>>> 99% latencies: 18328 159
>>>
>>> With percentile details, we see the benefit of nice latency -20 as
>>> only 1% of the latencies are above 159us whereas the default latency
>>> has got 15% around ~1ms or above and 5% over the 6ms.
>>>
>>> Signed-off-by: Vincent Guittot <[email protected]>
>>> ---
>>> include/linux/sched.h | 4 ++-
>>> include/linux/sched/prio.h | 9 ++++++
>>> init/init_task.c | 2 +-
>>> kernel/sched/core.c | 38 +++++++++++++++++++---
>>> kernel/sched/debug.c | 2 +-
>>> kernel/sched/fair.c | 66 ++++++++++++++++++++++++++++++++++----
>>> kernel/sched/sched.h | 6 ++++
>>> 7 files changed, 112 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>>> index 856240573300..2f33326adb8d 100644
>>> --- a/include/linux/sched.h
>>> +++ b/include/linux/sched.h
>>> @@ -568,6 +568,8 @@ struct sched_entity {
>>> /* cached value of my_q->h_nr_running */
>>> unsigned long runnable_weight;
>>> #endif
>>> + /* preemption offset in ns */
>>> + long latency_offset;
>>>
>>> #ifdef CONFIG_SMP
>>> /*
>>> @@ -784,7 +786,7 @@ struct task_struct {
>>> int static_prio;
>>> int normal_prio;
>>> unsigned int rt_priority;
>>> - int latency_nice;
>>> + int latency_prio;
>>>
>>> struct sched_entity se;
>>> struct sched_rt_entity rt;
>>> diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
>>> index bfcd7f1d1e11..be79503d86af 100644
>>> --- a/include/linux/sched/prio.h
>>> +++ b/include/linux/sched/prio.h
>>> @@ -59,5 +59,14 @@ static inline long rlimit_to_nice(long prio)
>>> * Default tasks should be treated as a task with latency_nice = 0.
>>> */
>>> #define DEFAULT_LATENCY_NICE 0
>>> +#define DEFAULT_LATENCY_PRIO (DEFAULT_LATENCY_NICE + LATENCY_NICE_WIDTH/2)
>>> +
>>> +/*
>>> + * Convert user-nice values [ -20 ... 0 ... 19 ]
>>> + * to static latency [ 0..39 ],
>>> + * and back.
>>> + */
>>> +#define NICE_TO_LATENCY(nice) ((nice) + DEFAULT_LATENCY_PRIO)
>>> +#define LATENCY_TO_NICE(prio) ((prio) - DEFAULT_LATENCY_PRIO)
>>>
>>> #endif /* _LINUX_SCHED_PRIO_H */
>>> diff --git a/init/init_task.c b/init/init_task.c
>>> index 7dd71dd2d261..071deff8dbd1 100644
>>> --- a/init/init_task.c
>>> +++ b/init/init_task.c
>>> @@ -78,7 +78,7 @@ struct task_struct init_task
>>> .prio = MAX_PRIO - 20,
>>> .static_prio = MAX_PRIO - 20,
>>> .normal_prio = MAX_PRIO - 20,
>>> - .latency_nice = DEFAULT_LATENCY_NICE,
>>> + .latency_prio = DEFAULT_LATENCY_PRIO,
>>> .policy = SCHED_NORMAL,
>>> .cpus_ptr = &init_task.cpus_mask,
>>> .user_cpus_ptr = NULL,
>>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>>> index 18c31a68eb18..b2b8cb6c08cd 100644
>>> --- a/kernel/sched/core.c
>>> +++ b/kernel/sched/core.c
>>> @@ -1283,6 +1283,16 @@ static void set_load_weight(struct task_struct *p, bool update_load)
>>> }
>>> }
>>>
>>> +static void set_latency_offset(struct task_struct *p)
>>> +{
>>> + long weight = sched_latency_to_weight[p->latency_prio];
>>> + s64 offset;
>>> +
>>> + offset = weight * get_sleep_latency(false);
>>> + offset = div_s64(offset, NICE_LATENCY_WEIGHT_MAX);
>>> + p->se.latency_offset = (long)offset;
>>> +}
>>> +
>>> #ifdef CONFIG_UCLAMP_TASK
>>> /*
>>> * Serializes updates of utilization clamp values
>>> @@ -4592,7 +4602,9 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
>>> p->prio = p->normal_prio = p->static_prio;
>>> set_load_weight(p, false);
>>>
>>> - p->latency_nice = DEFAULT_LATENCY_NICE;
>>> + p->latency_prio = NICE_TO_LATENCY(0);
>>> + set_latency_offset(p);
>>> +
>>> /*
>>> * We don't need the reset flag anymore after the fork. It has
>>> * fulfilled its duty:
>>> @@ -7358,8 +7370,10 @@ static void __setscheduler_params(struct task_struct *p,
>>> static void __setscheduler_latency(struct task_struct *p,
>>> const struct sched_attr *attr)
>>> {
>>> - if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE)
>>> - p->latency_nice = attr->sched_latency_nice;
>>> + if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE) {
>>> + p->latency_prio = NICE_TO_LATENCY(attr->sched_latency_nice);
>>> + set_latency_offset(p);
>>> + }
>>> }
>>>
>>> /*
>>> @@ -7544,7 +7558,7 @@ static int __sched_setscheduler(struct task_struct *p,
>>> if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
>>> goto change;
>>> if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE &&
>>> - attr->sched_latency_nice != p->latency_nice)
>>> + attr->sched_latency_nice != LATENCY_TO_NICE(p->latency_prio))
>>> goto change;
>>>
>>> p->sched_reset_on_fork = reset_on_fork;
>>> @@ -8085,7 +8099,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
>>> get_params(p, &kattr);
>>> kattr.sched_flags &= SCHED_FLAG_ALL;
>>>
>>> - kattr.sched_latency_nice = p->latency_nice;
>>> + kattr.sched_latency_nice = LATENCY_TO_NICE(p->latency_prio);
>>>
>>> #ifdef CONFIG_UCLAMP_TASK
>>> /*
>>> @@ -11294,6 +11308,20 @@ const u32 sched_prio_to_wmult[40] = {
>>> /* 15 */ 119304647, 148102320, 186737708, 238609294, 286331153,
>>> };
>>>
>>> +/*
>>> + * latency weight for wakeup preemption
>>> + */
>>> +const int sched_latency_to_weight[40] = {
>>> + /* -20 */ -1024, -973, -922, -870, -819,
>>> + /* -15 */ -768, -717, -666, -614, -563,
>>> + /* -10 */ -512, -461, -410, -358, -307,
>>> + /* -5 */ -256, -205, -154, -102, -51,
>>> + /* 0 */ 0, 51, 102, 154, 205,
>>> + /* 5 */ 256, 307, 358, 410, 461,
>>> + /* 10 */ 512, 563, 614, 666, 717,
>>> + /* 15 */ 768, 819, 870, 922, 973,
>>> +};
>>> +
>>
>> The table is linear. You could approximate this as: weight = nice * 51
>> since it is a linear scale and do the conversion in place.
>>
>> Or, since the only place you are using the latency_to_weight is in
>> set_latency_offset(), can we drop the sched_latency_to_weight array
>> and simplify as follows?
>
> It's also used in cgroup patch and keeps a coherency between
> nice/weight an latency_nice/offset so I prefer

I dont think it’s a valid comparison as nice/weight conversion are non linear and over there a table makes sense: weight = 1024 / 1.25 ^ nice

> keeping current
> implementation

I could be missing something, but, since its a linear scale, why does cgroup need weight at all? Just store nice directly. Why would that not work?

In the end the TG and SE has the latency offset in the struct, that is all you care about. All the conversion back and forth is unnecessary, as it is a linear scale and just increases LOC and takes more memory to store linear arrays.

Again I could be missing something and I will try to play with your series and see if I can show you what I mean (or convince myself it’s needed).

>> static void set_latency_offset(struct task_struct *p)
>> {
>> s64 offset = p->latency_prio * get_sleep_latency(false);
>> p->latency_prio = (long)div_s64(offset, 40);
>> }
>>
>>> void call_trace_sched_update_nr_running(struct rq *rq, int count)
>>> {
>>> trace_sched_update_nr_running_tp(rq, count);
>>> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
>>> index 68be7a3e42a3..b3922184af91 100644
>>> --- a/kernel/sched/debug.c
>>> +++ b/kernel/sched/debug.c
>>> @@ -1043,7 +1043,7 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
>>> #endif
>>> P(policy);
>>> P(prio);
>>> - P(latency_nice);
>>> + P(latency_prio);
>>> if (task_has_dl_policy(p)) {
>>> P(dl.runtime);
>>> P(dl.deadline);
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index c8a697f8db88..0e80e65113bd 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -4858,6 +4858,8 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>>> update_idle_cfs_rq_clock_pelt(cfs_rq);
>>> }
>>>
>>> +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se);
>>> +
>>> /*
>>> * Preempt the current task with a newly woken task if needed:
>>> */
>>> @@ -4866,7 +4868,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
>>> {
>>> unsigned long ideal_runtime, delta_exec;
>>> struct sched_entity *se;
>>> - s64 delta;
>>> + s64 delta, offset;
>>>
>>> ideal_runtime = sched_slice(cfs_rq, curr);
>>> delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
>>> @@ -4891,10 +4893,12 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
>>> se = __pick_first_entity(cfs_rq);
>>> delta = curr->vruntime - se->vruntime;
>>>
>>> - if (delta < 0)
>>> + offset = wakeup_latency_gran(curr, se);
>>> + if (delta < offset)
>>> return;
>>
>> Agreed.
>>
>>> - if (delta > ideal_runtime)
>>> + if ((delta > ideal_runtime) ||
>>> + (delta > get_latency_max()))
>>> resched_curr(rq_of(cfs_rq));
>>> }
>>>
>>> @@ -6019,6 +6023,35 @@ static int sched_idle_cpu(int cpu)
>>> }
>>> #endif
>>>
>>> +static void set_next_buddy(struct sched_entity *se);
>>> +
>>> +static void check_preempt_from_others(struct cfs_rq *cfs, struct sched_entity *se)
>>> +{
>>> + struct sched_entity *next;
>>> +
>>> + if (se->latency_offset >= 0)
>>> + return;
>>> +
>>> + if (cfs->nr_running <= 1)
>>> + return;
>>> + /*
>>> + * When waking from another class, we don't need to check to preempt at
>>> + * wakeup and don't set next buddy as a candidate for being picked in
>>> + * priority.
>>> + * In case of simultaneous wakeup when current is another class, the
>>> + * latency sensitive tasks lost opportunity to preempt non sensitive
>>> + * tasks which woke up simultaneously.
>>> + */
>>> +
>>> + if (cfs->next)
>>> + next = cfs->next;
>>> + else
>>> + next = __pick_first_entity(cfs);
>>> +
>>> + if (next && wakeup_preempt_entity(next, se) == 1)
>>> + set_next_buddy(se);
>>> +}
>>> +
>>> /*
>>> * The enqueue_task method is called before nr_running is
>>> * increased. Here we update the fair scheduling stats and
>>> @@ -6105,14 +6138,15 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>>> if (!task_new)
>>> update_overutilized_status(rq);
>>>
>>> + if (rq->curr->sched_class != &fair_sched_class)
>>> + check_preempt_from_others(cfs_rq_of(&p->se), &p->se);
>>> +
>>> enqueue_throttle:
>>> assert_list_leaf_cfs_rq(rq);
>>>
>>> hrtick_update(rq);
>>> }
>>>
>>> -static void set_next_buddy(struct sched_entity *se);
>>> -
>>> /*
>>> * The dequeue_task method is called before nr_running is
>>> * decreased. We remove the task from the rbtree and
>>> @@ -7461,6 +7495,23 @@ balance_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
>>> }
>>> #endif /* CONFIG_SMP */
>>>
>>> +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se)
>>> +{
>>> + long latency_offset = se->latency_offset;
>>> +
>>> + /*
>>> + * A negative latency offset means that the sched_entity has latency
>>> + * requirement that needs to be evaluated versus other entity.
>>> + * Otherwise, use the latency weight to evaluate how much scheduling
>>> + * delay is acceptable by se.
>>> + */
>>> + if ((latency_offset < 0) || (curr->latency_offset < 0))
>>> + latency_offset -= curr->latency_offset;
>>> + latency_offset = min_t(long, latency_offset, get_latency_max());
>>
>> Over here can we make positive latency offsets also be evaluated
>> "versus other entity"?
>>
>> It feels strange to have different rules for positive latency_offset
>> when comparing curr and se. IMO we should also factor in latency
>> requirements by comparing 2 positive nice values. It should be
>> relative even for positive values, just like regular nice IMO and not
>> have hidden meaning. If there is hidden meaning, it confuses the user
>> and requires documentation that most users will not read. Especially
>> because latency_nice shares the word "nice" with regular nice values.
>
> This has already been discussed in the previous revisions.

Sorry to be late to the party.

> This is not
> a hidden behavior but the normal behavior.
>
> A negative latency nice, means that the task are not tolerant to
> scheduling delay and it want to preempt current and run now. Or, if
> the task is current, it doesn't want to be preempted and finish its
> slice. In this case, we compare current and wake up task in case there
> is 2 latency sensitive tasks that are fighting to run 1st.
>
> Whereas a positive latency nice means that the task is tolerant to
> scheduling delay and you don't care preempting current as long as it's
> in an acceptable vruntime range. Why would the latency nice of the
> current task make the wakeup task less tolerant to the scheduling
> delay ? As an example, If current is latency_nice 19 and the wakeup
> task is latency nice 19 too, both are tolerant to scheduling delay and
> the waking up task should preempt current only if there is an
> unfairness problem. By comparing their positive latency nice values,
> you are back to the normal behavior which defeats the purpose of the
> feature.

I see it as, if 2 tasks are latency tolerant, then they will have higher latency with respect to a third tasks that is latency in tolerant. But I am ok with your definition as well…

Thanks!

- Joel

>
> Thanks
> Vincent
>
>>
>> Thanks,
>>
>> - Joel
>> .
>>
>>> +
>>> + return latency_offset;
>>> +}
>>> +
>>> static unsigned long wakeup_gran(struct sched_entity *se)
>>> {
>>> unsigned long gran = sysctl_sched_wakeup_granularity;
>>> @@ -7499,11 +7550,12 @@ static int
>>> wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se)
>>> {
>>> s64 gran, vdiff = curr->vruntime - se->vruntime;
>>> + s64 offset = wakeup_latency_gran(curr, se);
>>>
>>> - if (vdiff <= 0)
>>> + if (vdiff < offset)
>>> return -1;
>>>
>>> - gran = wakeup_gran(se);
>>> + gran = offset + wakeup_gran(se);
>>>
>>> /*
>>> * At wake up, the vruntime of a task is capped to not be older than
>>> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
>>> index 842ce0094d9c..7292652731d0 100644
>>> --- a/kernel/sched/sched.h
>>> +++ b/kernel/sched/sched.h
>>> @@ -125,6 +125,11 @@ extern int sched_rr_timeslice;
>>> */
>>> #define NS_TO_JIFFIES(TIME) ((unsigned long)(TIME) / (NSEC_PER_SEC / HZ))
>>>
>>> +/* Maximum nice latency weight used to scale the latency_offset */
>>> +
>>> +#define NICE_LATENCY_SHIFT (SCHED_FIXEDPOINT_SHIFT)
>>> +#define NICE_LATENCY_WEIGHT_MAX (1L << NICE_LATENCY_SHIFT)
>>> +
>>> /*
>>> * Increase resolution of nice-level calculations for 64-bit architectures.
>>> * The extra resolution improves shares distribution and load balancing of
>>> @@ -2115,6 +2120,7 @@ static_assert(WF_TTWU == SD_BALANCE_WAKE);
>>>
>>> extern const int sched_prio_to_weight[40];
>>> extern const u32 sched_prio_to_wmult[40];
>>> +extern const int sched_latency_to_weight[40];
>>>
>>> /*
>>> * {de,en}queue flags:
>>> --
>>> 2.17.1
>>>

2022-11-29 18:03:05

by Vincent Guittot

Subject: Re: [PATCH 5/9] sched/fair: Take into account latency priority at wakeup

On Tue, 29 Nov 2022 at 16:45, Joel Fernandes <[email protected]> wrote:
>
> Hi Vincent,
> Thank you so much for your quick reply.
>
> > On Nov 29, 2022, at 3:58 AM, Vincent Guittot <[email protected]> wrote:
> >
> > On Tue, 29 Nov 2022 at 05:25, Joel Fernandes <[email protected]> wrote:
> >>
> >>> On Tue, Nov 15, 2022 at 12:19 PM Vincent Guittot
> >>> <[email protected]> wrote:
> >>>
> >>> Take into account the latency priority of a thread when deciding to
> >>> preempt the current running thread. We don't want to provide more CPU
> >>> bandwidth to a thread but reorder the scheduling to run latency sensitive
> >>> task first whenever possible.
> >>>
> >>> As long as a thread didn't use its bandwidth, it will be able to preempt
> >>> the current thread.
> >>>
> >>> At the opposite, a thread with a low latency priority will preempt current
> >>> thread at wakeup only to keep fair CPU bandwidth sharing. Otherwise it will
> >>> wait for the tick to get its sched slice.
> >>>
> >>> curr vruntime
> >>> |
> >>> sysctl_sched_wakeup_granularity
> >>> <-->
> >>> ----------------------------------|----|-----------------------|---------------
> >>> | |<--------------------->
> >>> | . sysctl_sched_latency
> >>> | .
> >>> default/current latency entity | .
> >>> | .
> >>> 1111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-
> >>> se preempts curr at wakeup ------>|<- se doesn't preempt curr -----------------
> >>> | .
> >>> | .
> >>> | .
> >>> low latency entity | .
> >>> ---------------------->|
> >>> % of sysctl_sched_latency |
> >>> 1111111111111111111111111111111111111111111111111111111111|0000|-1-1-1-1-1-1-1-
> >>> preempt ------------------------------------------------->|<- do not preempt --
> >>> | .
> >>> | .
> >>> | .
> >>> high latency entity | .
> >>> |<-----------------------|----.
> >>> | % of sysctl_sched_latency .
> >>> 111111111|0000|-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1-1
> >>> preempt->|<- se doesn't preempt curr ------------------------------------------
> >>> Tests results of nice latency impact on heavy load like hackbench:
> >>>
> >>> hackbench -l (2560 / group) -g group
> >>> group latency 0 latency 19
> >>> 1 1.378(+/- 1%) 1.337(+/- 1%) + 3%
> >>> 4 1.393(+/- 3%) 1.312(+/- 3%) + 6%
> >>> 8 1.308(+/- 2%) 1.279(+/- 1%) + 2%
> >>> 16 1.347(+/- 1%) 1.317(+/- 1%) + 2%
> >>>
> >>> hackbench -p -l (2560 / group) -g group
> >>> group
> >>> 1 1.836(+/- 17%) 1.148(+/- 5%) +37%
> >>> 4 1.586(+/- 6%) 1.109(+/- 8%) +30%
> >>> 8 1.209(+/- 4%) 0.780(+/- 4%) +35%
> >>> 16 0.805(+/- 5%) 0.728(+/- 4%) +10%
> >>>
> >>> By deacreasing the latency prio, we reduce the number of preemption at
> >>> wakeup and help hackbench making progress.
> >>>
> >>> Test results of nice latency impact on short live load like cyclictest
> >>> while competing with heavy load like hackbench:
> >>>
> >>> hackbench -l 10000 -g $group &
> >>> cyclictest --policy other -D 5 -q -n
> >>> latency 0 latency -20
> >>> group min avg max min avg max
> >>> 0 16 19 29 17 18 29
> >>> 1 43 299 7359 63 84 3422
> >>> 4 56 449 14806 45 83 284
> >>> 8 63 820 51123 63 83 283
> >>> 16 64 1326 70684 41 157 26852
> >>>
> >>> group = 0 means that hackbench is not running.
> >>>
> >>> The avg is significantly improved with nice latency -20 especially with
> >>> large number of groups but min and max remain quite similar. If we add the
> >>> histogram parameter to get details of latency, we have :
> >>>
> >>> hackbench -l 10000 -g 16 &
> >>> cyclictest --policy other -D 5 -q -n -H 20000 --histfile data.txt
> >>> latency 0 latency -20
> >>> Min Latencies: 64 62
> >>> Avg Latencies: 1170 107
> >>> Max Latencies: 88069 10417
> >>> 50% latencies: 122 86
> >>> 75% latencies: 614 91
> >>> 85% latencies: 961 94
> >>> 90% latencies: 1225 97
> >>> 95% latencies: 6120 102
> >>> 99% latencies: 18328 159
> >>>
> >>> With percentile details, we see the benefit of nice latency -20 as
> >>> only 1% of the latencies are above 159us whereas the default latency
> >>> has got 15% around ~1ms or above and 5% over the 6ms.
> >>>
> >>> Signed-off-by: Vincent Guittot <[email protected]>
> >>> ---
> >>> include/linux/sched.h | 4 ++-
> >>> include/linux/sched/prio.h | 9 ++++++
> >>> init/init_task.c | 2 +-
> >>> kernel/sched/core.c | 38 +++++++++++++++++++---
> >>> kernel/sched/debug.c | 2 +-
> >>> kernel/sched/fair.c | 66 ++++++++++++++++++++++++++++++++++----
> >>> kernel/sched/sched.h | 6 ++++
> >>> 7 files changed, 112 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/include/linux/sched.h b/include/linux/sched.h
> >>> index 856240573300..2f33326adb8d 100644
> >>> --- a/include/linux/sched.h
> >>> +++ b/include/linux/sched.h
> >>> @@ -568,6 +568,8 @@ struct sched_entity {
> >>> /* cached value of my_q->h_nr_running */
> >>> unsigned long runnable_weight;
> >>> #endif
> >>> + /* preemption offset in ns */
> >>> + long latency_offset;
> >>>
> >>> #ifdef CONFIG_SMP
> >>> /*
> >>> @@ -784,7 +786,7 @@ struct task_struct {
> >>> int static_prio;
> >>> int normal_prio;
> >>> unsigned int rt_priority;
> >>> - int latency_nice;
> >>> + int latency_prio;
> >>>
> >>> struct sched_entity se;
> >>> struct sched_rt_entity rt;
> >>> diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
> >>> index bfcd7f1d1e11..be79503d86af 100644
> >>> --- a/include/linux/sched/prio.h
> >>> +++ b/include/linux/sched/prio.h
> >>> @@ -59,5 +59,14 @@ static inline long rlimit_to_nice(long prio)
> >>> * Default tasks should be treated as a task with latency_nice = 0.
> >>> */
> >>> #define DEFAULT_LATENCY_NICE 0
> >>> +#define DEFAULT_LATENCY_PRIO (DEFAULT_LATENCY_NICE + LATENCY_NICE_WIDTH/2)
> >>> +
> >>> +/*
> >>> + * Convert user-nice values [ -20 ... 0 ... 19 ]
> >>> + * to static latency [ 0..39 ],
> >>> + * and back.
> >>> + */
> >>> +#define NICE_TO_LATENCY(nice) ((nice) + DEFAULT_LATENCY_PRIO)
> >>> +#define LATENCY_TO_NICE(prio) ((prio) - DEFAULT_LATENCY_PRIO)
> >>>
> >>> #endif /* _LINUX_SCHED_PRIO_H */
> >>> diff --git a/init/init_task.c b/init/init_task.c
> >>> index 7dd71dd2d261..071deff8dbd1 100644
> >>> --- a/init/init_task.c
> >>> +++ b/init/init_task.c
> >>> @@ -78,7 +78,7 @@ struct task_struct init_task
> >>> .prio = MAX_PRIO - 20,
> >>> .static_prio = MAX_PRIO - 20,
> >>> .normal_prio = MAX_PRIO - 20,
> >>> - .latency_nice = DEFAULT_LATENCY_NICE,
> >>> + .latency_prio = DEFAULT_LATENCY_PRIO,
> >>> .policy = SCHED_NORMAL,
> >>> .cpus_ptr = &init_task.cpus_mask,
> >>> .user_cpus_ptr = NULL,
> >>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> >>> index 18c31a68eb18..b2b8cb6c08cd 100644
> >>> --- a/kernel/sched/core.c
> >>> +++ b/kernel/sched/core.c
> >>> @@ -1283,6 +1283,16 @@ static void set_load_weight(struct task_struct *p, bool update_load)
> >>> }
> >>> }
> >>>
> >>> +static void set_latency_offset(struct task_struct *p)
> >>> +{
> >>> + long weight = sched_latency_to_weight[p->latency_prio];
> >>> + s64 offset;
> >>> +
> >>> + offset = weight * get_sleep_latency(false);
> >>> + offset = div_s64(offset, NICE_LATENCY_WEIGHT_MAX);
> >>> + p->se.latency_offset = (long)offset;
> >>> +}
> >>> +
> >>> #ifdef CONFIG_UCLAMP_TASK
> >>> /*
> >>> * Serializes updates of utilization clamp values
> >>> @@ -4592,7 +4602,9 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
> >>> p->prio = p->normal_prio = p->static_prio;
> >>> set_load_weight(p, false);
> >>>
> >>> - p->latency_nice = DEFAULT_LATENCY_NICE;
> >>> + p->latency_prio = NICE_TO_LATENCY(0);
> >>> + set_latency_offset(p);
> >>> +
> >>> /*
> >>> * We don't need the reset flag anymore after the fork. It has
> >>> * fulfilled its duty:
> >>> @@ -7358,8 +7370,10 @@ static void __setscheduler_params(struct task_struct *p,
> >>> static void __setscheduler_latency(struct task_struct *p,
> >>> const struct sched_attr *attr)
> >>> {
> >>> - if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE)
> >>> - p->latency_nice = attr->sched_latency_nice;
> >>> + if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE) {
> >>> + p->latency_prio = NICE_TO_LATENCY(attr->sched_latency_nice);
> >>> + set_latency_offset(p);
> >>> + }
> >>> }
> >>>
> >>> /*
> >>> @@ -7544,7 +7558,7 @@ static int __sched_setscheduler(struct task_struct *p,
> >>> if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
> >>> goto change;
> >>> if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE &&
> >>> - attr->sched_latency_nice != p->latency_nice)
> >>> + attr->sched_latency_nice != LATENCY_TO_NICE(p->latency_prio))
> >>> goto change;
> >>>
> >>> p->sched_reset_on_fork = reset_on_fork;
> >>> @@ -8085,7 +8099,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
> >>> get_params(p, &kattr);
> >>> kattr.sched_flags &= SCHED_FLAG_ALL;
> >>>
> >>> - kattr.sched_latency_nice = p->latency_nice;
> >>> + kattr.sched_latency_nice = LATENCY_TO_NICE(p->latency_prio);
> >>>
> >>> #ifdef CONFIG_UCLAMP_TASK
> >>> /*
> >>> @@ -11294,6 +11308,20 @@ const u32 sched_prio_to_wmult[40] = {
> >>> /* 15 */ 119304647, 148102320, 186737708, 238609294, 286331153,
> >>> };
> >>>
> >>> +/*
> >>> + * latency weight for wakeup preemption
> >>> + */
> >>> +const int sched_latency_to_weight[40] = {
> >>> + /* -20 */ -1024, -973, -922, -870, -819,
> >>> + /* -15 */ -768, -717, -666, -614, -563,
> >>> + /* -10 */ -512, -461, -410, -358, -307,
> >>> + /* -5 */ -256, -205, -154, -102, -51,
> >>> + /* 0 */ 0, 51, 102, 154, 205,
> >>> + /* 5 */ 256, 307, 358, 410, 461,
> >>> + /* 10 */ 512, 563, 614, 666, 717,
> >>> + /* 15 */ 768, 819, 870, 922, 973,
> >>> +};
> >>> +
> >>
> >> The table is linear. You could approximate this as: weight = nice * 51
> >> since it is a linear scale and do the conversion in place.
> >>
> >> Or, since the only place you are using the latency_to_weight is in
> >> set_latency_offset(), can we drop the sched_latency_to_weight array
> >> and simplify as follows?
> >
> > It's also used in cgroup patch and keeps a coherency between
> > nice/weight an latency_nice/offset so I prefer
>
> I dont think it’s a valid comparison as nice/weight conversion are non linear and over there a table makes sense: weight = 1024 / 1.25 ^ nice
>
> > keeping current
> > implementation
>
> I could be missing something, but, since its a linear scale, why does cgroup need weight at all? Just store nice directly. Why would that not work?
>
> In the end the TG and SE has the latency offset in the struct, that is all you care about. All the conversion back and forth is unnecessary, as it is a linear scale and just increases LOC and takes more memory to store linear arrays.
>
> Again I could be missing something and I will try to play with your series and see if I can show you what I mean (or convince myself it’s needed).

I get what you mean but I think that having an array gives latitude to
adjust this internal offset mapping at a minimum cost of a const array

>
> >> static void set_latency_offset(struct task_struct *p)
> >> {
> >> s64 offset = p->latency_prio * get_sleep_latency(false);
> >> p->latency_prio = (long)div_s64(offset, 40);
> >> }
> >>
> >>> void call_trace_sched_update_nr_running(struct rq *rq, int count)
> >>> {
> >>> trace_sched_update_nr_running_tp(rq, count);
> >>> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> >>> index 68be7a3e42a3..b3922184af91 100644
> >>> --- a/kernel/sched/debug.c
> >>> +++ b/kernel/sched/debug.c
> >>> @@ -1043,7 +1043,7 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
> >>> #endif
> >>> P(policy);
> >>> P(prio);
> >>> - P(latency_nice);
> >>> + P(latency_prio);
> >>> if (task_has_dl_policy(p)) {
> >>> P(dl.runtime);
> >>> P(dl.deadline);
> >>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >>> index c8a697f8db88..0e80e65113bd 100644
> >>> --- a/kernel/sched/fair.c
> >>> +++ b/kernel/sched/fair.c
> >>> @@ -4858,6 +4858,8 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> >>> update_idle_cfs_rq_clock_pelt(cfs_rq);
> >>> }
> >>>
> >>> +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se);
> >>> +
> >>> /*
> >>> * Preempt the current task with a newly woken task if needed:
> >>> */
> >>> @@ -4866,7 +4868,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> >>> {
> >>> unsigned long ideal_runtime, delta_exec;
> >>> struct sched_entity *se;
> >>> - s64 delta;
> >>> + s64 delta, offset;
> >>>
> >>> ideal_runtime = sched_slice(cfs_rq, curr);
> >>> delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
> >>> @@ -4891,10 +4893,12 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> >>> se = __pick_first_entity(cfs_rq);
> >>> delta = curr->vruntime - se->vruntime;
> >>>
> >>> - if (delta < 0)
> >>> + offset = wakeup_latency_gran(curr, se);
> >>> + if (delta < offset)
> >>> return;
> >>
> >> Agreed.
> >>
> >>> - if (delta > ideal_runtime)
> >>> + if ((delta > ideal_runtime) ||
> >>> + (delta > get_latency_max()))
> >>> resched_curr(rq_of(cfs_rq));
> >>> }
> >>>
> >>> @@ -6019,6 +6023,35 @@ static int sched_idle_cpu(int cpu)
> >>> }
> >>> #endif
> >>>
> >>> +static void set_next_buddy(struct sched_entity *se);
> >>> +
> >>> +static void check_preempt_from_others(struct cfs_rq *cfs, struct sched_entity *se)
> >>> +{
> >>> + struct sched_entity *next;
> >>> +
> >>> + if (se->latency_offset >= 0)
> >>> + return;
> >>> +
> >>> + if (cfs->nr_running <= 1)
> >>> + return;
> >>> + /*
> >>> + * When waking from another class, we don't need to check to preempt at
> >>> + * wakeup and don't set next buddy as a candidate for being picked in
> >>> + * priority.
> >>> + * In case of simultaneous wakeup when current is another class, the
> >>> + * latency sensitive tasks lost opportunity to preempt non sensitive
> >>> + * tasks which woke up simultaneously.
> >>> + */
> >>> +
> >>> + if (cfs->next)
> >>> + next = cfs->next;
> >>> + else
> >>> + next = __pick_first_entity(cfs);
> >>> +
> >>> + if (next && wakeup_preempt_entity(next, se) == 1)
> >>> + set_next_buddy(se);
> >>> +}
> >>> +
> >>> /*
> >>> * The enqueue_task method is called before nr_running is
> >>> * increased. Here we update the fair scheduling stats and
> >>> @@ -6105,14 +6138,15 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> >>> if (!task_new)
> >>> update_overutilized_status(rq);
> >>>
> >>> + if (rq->curr->sched_class != &fair_sched_class)
> >>> + check_preempt_from_others(cfs_rq_of(&p->se), &p->se);
> >>> +
> >>> enqueue_throttle:
> >>> assert_list_leaf_cfs_rq(rq);
> >>>
> >>> hrtick_update(rq);
> >>> }
> >>>
> >>> -static void set_next_buddy(struct sched_entity *se);
> >>> -
> >>> /*
> >>> * The dequeue_task method is called before nr_running is
> >>> * decreased. We remove the task from the rbtree and
> >>> @@ -7461,6 +7495,23 @@ balance_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
> >>> }
> >>> #endif /* CONFIG_SMP */
> >>>
> >>> +static long wakeup_latency_gran(struct sched_entity *curr, struct sched_entity *se)
> >>> +{
> >>> + long latency_offset = se->latency_offset;
> >>> +
> >>> + /*
> >>> + * A negative latency offset means that the sched_entity has latency
> >>> + * requirement that needs to be evaluated versus other entity.
> >>> + * Otherwise, use the latency weight to evaluate how much scheduling
> >>> + * delay is acceptable by se.
> >>> + */
> >>> + if ((latency_offset < 0) || (curr->latency_offset < 0))
> >>> + latency_offset -= curr->latency_offset;
> >>> + latency_offset = min_t(long, latency_offset, get_latency_max());
> >>
> >> Over here can we make positive latency offsets also be evaluated
> >> "versus other entity"?
> >>
> >> It feels strange to have different rules for positive latency_offset
> >> when comparing curr and se. IMO we should also factor in latency
> >> requirements by comparing 2 positive nice values. It should be
> >> relative even for positive values, just like regular nice IMO and not
> >> have hidden meaning. If there is hidden meaning, it confuses the user
> >> and requires documentation that most users will not read. Especially
> >> because latency_nice shares the word "nice" with regular nice values.
> >
> > This has already been discussed in the previous revisions.
>
> Sorry to be late to the party.
>
> > This is not
> > a hidden behavior but the normal behavior.
> >
> > A negative latency nice, means that the task are not tolerant to
> > scheduling delay and it want to preempt current and run now. Or, if
> > the task is current, it doesn't want to be preempted and finish its
> > slice. In this case, we compare current and wake up task in case there
> > is 2 latency sensitive tasks that are fighting to run 1st.
> >
> > Whereas a positive latency nice means that the task is tolerant to
> > scheduling delay and you don't care preempting current as long as it's
> > in an acceptable vruntime range. Why would the latency nice of the
> > current task make the wakeup task less tolerant to the scheduling
> > delay ? As an example, If current is latency_nice 19 and the wakeup
> > task is latency nice 19 too, both are tolerant to scheduling delay and
> > the waking up task should preempt current only if there is an
> > unfairness problem. By comparing their positive latency nice values,
> > you are back to the normal behavior which defeats the purpose of the
> > feature.
>
> I see it as, if 2 tasks are latency tolerant, then they will have higher latency with respect to a third tasks that is latency in tolerant. But I am ok with your definition as well…
>
> Thanks!
>
> - Joel
>
> >
> > Thanks
> > Vincent
> >
> >>
> >> Thanks,
> >>
> >> - Joel
> >> .
> >>
> >>> +
> >>> + return latency_offset;
> >>> +}
> >>> +
> >>> static unsigned long wakeup_gran(struct sched_entity *se)
> >>> {
> >>> unsigned long gran = sysctl_sched_wakeup_granularity;
> >>> @@ -7499,11 +7550,12 @@ static int
> >>> wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se)
> >>> {
> >>> s64 gran, vdiff = curr->vruntime - se->vruntime;
> >>> + s64 offset = wakeup_latency_gran(curr, se);
> >>>
> >>> - if (vdiff <= 0)
> >>> + if (vdiff < offset)
> >>> return -1;
> >>>
> >>> - gran = wakeup_gran(se);
> >>> + gran = offset + wakeup_gran(se);
> >>>
> >>> /*
> >>> * At wake up, the vruntime of a task is capped to not be older than
> >>> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> >>> index 842ce0094d9c..7292652731d0 100644
> >>> --- a/kernel/sched/sched.h
> >>> +++ b/kernel/sched/sched.h
> >>> @@ -125,6 +125,11 @@ extern int sched_rr_timeslice;
> >>> */
> >>> #define NS_TO_JIFFIES(TIME) ((unsigned long)(TIME) / (NSEC_PER_SEC / HZ))
> >>>
> >>> +/* Maximum nice latency weight used to scale the latency_offset */
> >>> +
> >>> +#define NICE_LATENCY_SHIFT (SCHED_FIXEDPOINT_SHIFT)
> >>> +#define NICE_LATENCY_WEIGHT_MAX (1L << NICE_LATENCY_SHIFT)
> >>> +
> >>> /*
> >>> * Increase resolution of nice-level calculations for 64-bit architectures.
> >>> * The extra resolution improves shares distribution and load balancing of
> >>> @@ -2115,6 +2120,7 @@ static_assert(WF_TTWU == SD_BALANCE_WAKE);
> >>>
> >>> extern const int sched_prio_to_weight[40];
> >>> extern const u32 sched_prio_to_wmult[40];
> >>> +extern const int sched_latency_to_weight[40];
> >>>
> >>> /*
> >>> * {de,en}queue flags:
> >>> --
> >>> 2.17.1
> >>>

2022-11-30 03:44:46

by Joel Fernandes

Subject: Re: [PATCH 5/9] sched/fair: Take into account latency priority at wakeup

Hi Vincent,

On Tue, Nov 29, 2022 at 5:21 PM Vincent Guittot
<[email protected]> wrote:
[...]
> > >>> }
> > >>>
> > >>> /*
> > >>> @@ -7544,7 +7558,7 @@ static int __sched_setscheduler(struct task_struct *p,
> > >>> if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
> > >>> goto change;
> > >>> if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE &&
> > >>> - attr->sched_latency_nice != p->latency_nice)
> > >>> + attr->sched_latency_nice != LATENCY_TO_NICE(p->latency_prio))
> > >>> goto change;
> > >>>
> > >>> p->sched_reset_on_fork = reset_on_fork;
> > >>> @@ -8085,7 +8099,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
> > >>> get_params(p, &kattr);
> > >>> kattr.sched_flags &= SCHED_FLAG_ALL;
> > >>>
> > >>> - kattr.sched_latency_nice = p->latency_nice;
> > >>> + kattr.sched_latency_nice = LATENCY_TO_NICE(p->latency_prio);
> > >>>
> > >>> #ifdef CONFIG_UCLAMP_TASK
> > >>> /*
> > >>> @@ -11294,6 +11308,20 @@ const u32 sched_prio_to_wmult[40] = {
> > >>> /* 15 */ 119304647, 148102320, 186737708, 238609294, 286331153,
> > >>> };
> > >>>
> > >>> +/*
> > >>> + * latency weight for wakeup preemption
> > >>> + */
> > >>> +const int sched_latency_to_weight[40] = {
> > >>> + /* -20 */ -1024, -973, -922, -870, -819,
> > >>> + /* -15 */ -768, -717, -666, -614, -563,
> > >>> + /* -10 */ -512, -461, -410, -358, -307,
> > >>> + /* -5 */ -256, -205, -154, -102, -51,
> > >>> + /* 0 */ 0, 51, 102, 154, 205,
> > >>> + /* 5 */ 256, 307, 358, 410, 461,
> > >>> + /* 10 */ 512, 563, 614, 666, 717,
> > >>> + /* 15 */ 768, 819, 870, 922, 973,
> > >>> +};
> > >>> +
> > >>
> > >> The table is linear. You could approximate this as: weight = nice * 51
> > >> since it is a linear scale and do the conversion in place.
> > >>
> > >> Or, since the only place you are using the latency_to_weight is in
> > >> set_latency_offset(), can we drop the sched_latency_to_weight array
> > >> and simplify as follows?
> > >
> > > It's also used in cgroup patch and keeps a coherency between
> > > nice/weight an latency_nice/offset so I prefer
> >
> > I dont think it’s a valid comparison as nice/weight conversion are non linear and over there a table makes sense: weight = 1024 / 1.25 ^ nice
> >
> > > keeping current
> > > implementation
> >
> > I could be missing something, but, since its a linear scale, why does cgroup need weight at all? Just store nice directly. Why would that not work?
> >
> > In the end the TG and SE has the latency offset in the struct, that is all you care about. All the conversion back and forth is unnecessary, as it is a linear scale and just increases LOC and takes more memory to store linear arrays.
> >
> > Again I could be missing something and I will try to play with your series and see if I can show you what I mean (or convince myself it’s needed).
>
> I get what you mean but I think that having an array gives latitude to
> adjust this internal offset mapping at a minimum cost of a const array

Ok that makes sense. If you feel like there might be updates in the
future to this mapping array (like changing the constants as you
mentioned), then I am Ok with us keeping it.

Reviewed-by: Joel Fernandes (Google) <[email protected]>

I am excited about your series, the CFS latency issues have been
thorny. This feels like a step forward in the right direction. Cheers,

- Joel

2022-11-30 14:06:08

by Vincent Guittot

Subject: Re: [PATCH 5/9] sched/fair: Take into account latency priority at wakeup

On Wed, 30 Nov 2022 at 04:10, Joel Fernandes <[email protected]> wrote:
>
> Hi Vincent,
>
> On Tue, Nov 29, 2022 at 5:21 PM Vincent Guittot
> <[email protected]> wrote:
> [...]
> > > >>> }
> > > >>>
> > > >>> /*
> > > >>> @@ -7544,7 +7558,7 @@ static int __sched_setscheduler(struct task_struct *p,
> > > >>> if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
> > > >>> goto change;
> > > >>> if (attr->sched_flags & SCHED_FLAG_LATENCY_NICE &&
> > > >>> - attr->sched_latency_nice != p->latency_nice)
> > > >>> + attr->sched_latency_nice != LATENCY_TO_NICE(p->latency_prio))
> > > >>> goto change;
> > > >>>
> > > >>> p->sched_reset_on_fork = reset_on_fork;
> > > >>> @@ -8085,7 +8099,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
> > > >>> get_params(p, &kattr);
> > > >>> kattr.sched_flags &= SCHED_FLAG_ALL;
> > > >>>
> > > >>> - kattr.sched_latency_nice = p->latency_nice;
> > > >>> + kattr.sched_latency_nice = LATENCY_TO_NICE(p->latency_prio);
> > > >>>
> > > >>> #ifdef CONFIG_UCLAMP_TASK
> > > >>> /*
> > > >>> @@ -11294,6 +11308,20 @@ const u32 sched_prio_to_wmult[40] = {
> > > >>> /* 15 */ 119304647, 148102320, 186737708, 238609294, 286331153,
> > > >>> };
> > > >>>
> > > >>> +/*
> > > >>> + * latency weight for wakeup preemption
> > > >>> + */
> > > >>> +const int sched_latency_to_weight[40] = {
> > > >>> + /* -20 */ -1024, -973, -922, -870, -819,
> > > >>> + /* -15 */ -768, -717, -666, -614, -563,
> > > >>> + /* -10 */ -512, -461, -410, -358, -307,
> > > >>> + /* -5 */ -256, -205, -154, -102, -51,
> > > >>> + /* 0 */ 0, 51, 102, 154, 205,
> > > >>> + /* 5 */ 256, 307, 358, 410, 461,
> > > >>> + /* 10 */ 512, 563, 614, 666, 717,
> > > >>> + /* 15 */ 768, 819, 870, 922, 973,
> > > >>> +};
> > > >>> +
> > > >>
> > > >> The table is linear. You could approximate this as: weight = nice * 51
> > > >> since it is a linear scale and do the conversion in place.
> > > >>
> > > >> Or, since the only place you are using the latency_to_weight is in
> > > >> set_latency_offset(), can we drop the sched_latency_to_weight array
> > > >> and simplify as follows?
> > > >
> > > > It's also used in cgroup patch and keeps a coherency between
> > > > nice/weight an latency_nice/offset so I prefer
> > >
> > > I dont think it’s a valid comparison as nice/weight conversion are non linear and over there a table makes sense: weight = 1024 / 1.25 ^ nice
> > >
> > > > keeping current
> > > > implementation
> > >
> > > I could be missing something, but, since its a linear scale, why does cgroup need weight at all? Just store nice directly. Why would that not work?
> > >
> > > In the end the TG and SE has the latency offset in the struct, that is all you care about. All the conversion back and forth is unnecessary, as it is a linear scale and just increases LOC and takes more memory to store linear arrays.
> > >
> > > Again I could be missing something and I will try to play with your series and see if I can show you what I mean (or convince myself it’s needed).
> >
> > I get what you mean but I think that having an array gives latitude to
> > adjust this internal offset mapping at a minimum cost of a const array
>
> Ok that makes sense. If you feel like there might be updates in the
> future to this mapping array (like changing the constants as you
> mentioned), then I am Ok with us keeping it.
>
> Reviewed-by: Joel Fernandes (Google) <[email protected]>
>
> I am excited about your series, the CFS latency issues have been
> thorny. This feels like a step forward in the right direction. Cheers,

Thanks
Vincent

>
> - Joel

2022-12-07 16:58:14

by K Prateek Nayak

Subject: Re: [PATCH v9 0/9] Add latency priority for CFS class

Hello Vincent,

Thank you for taking a look at the report.

On 11/28/2022 10:49 PM, Vincent Guittot wrote:
> Hi Prateek,
>
> On Mon, 28 Nov 2022 at 12:52, K Prateek Nayak <[email protected]> wrote:
>>
>> Hello Vincent,
>>
>> Following are the test results on dual socket Zen3 machine (2 x 64C/128T)
>>
>> tl;dr
>>
>> o All benchmarks with DEFAULT_LATENCY_NICE value are comparable to tip.
>> There is, however, a noticeable dip for unixbench-spawn test case.
>>
>> o With the 2 rbtree approach, I do not see much difference in the
>> hackbench results with varying latency nice value. Tests on v5 did
>> yield noticeable improvements for hackbench.
>> (https://lore.kernel.org/lkml/[email protected]/)
>
> The 2 rbtree approach is the one that was already used in v5. I just
> rerun hackbench tests with latest tip and v6.2-rc7 and I can see large
> performance improvement for pipe tests on my system (8 cores system).
> Could you try witha larger number of group ? like 64, 128 and 256
> groups

Ah! My bad. I've rerun hackbench with larger number of groups and I see a
clear win for pipes with latency nice 19. Hackbench with sockets too see a
small win.

o pipes

$ perf bench sched messaging -p -l 50000 -g <groups>

latency_nice: 0 19 -20
32-groups: 9.43 (0.00 pct) 6.42 (31.91 pct) 9.75 (-3.39 pct)
64-groups: 21.55 (0.00 pct) 12.97 (39.81 pct) 21.48 (0.32 pct)
128-groups: 41.15 (0.00 pct) 24.18 (41.23 pct) 46.69 (-13.46 pct)
256-groups: 78.87 (0.00 pct) 43.65 (44.65 pct) 78.84 (0.03 pct)
512-groups: 125.48 (0.00 pct) 78.91 (37.11 pct) 136.21 (-8.55 pct)
1024-groups: 292.81 (0.00 pct) 151.36 (48.30 pct) 323.57 (-10.50 pct)

o sockets

$ perf bench sched messaging -l 100000 -g <groups>

latency_nice: 0 19 -20
32-groups: 27.23 (0.00 pct) 27.00 (0.84 pct) 26.92 (1.13 pct)
64-groups: 45.71 (0.00 pct) 44.58 (2.47 pct) 45.86 (-0.32 pct)
128-groups: 79.55 (0.00 pct) 78.22 (1.67 pct) 80.01 (-0.57 pct)
256-groups: 161.41 (0.00 pct) 164.04 (-1.62 pct) 169.57 (-5.05 pct)
512-groups: 326.41 (0.00 pct) 310.00 (5.02 pct) 342.17 (-4.82 pct)
1024-groups: 634.36 (0.00 pct) 633.59 (0.12 pct) 640.05 (-0.89 pct)

Note: All tests were done in NPS1 mode.

>
>>
>> [..snip..]
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~ Unixbench - DEFAULT_LATENCY_NICE ~
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> o NPS1
>>
>> Test Metric Parallelism tip latency_nice
>> unixbench-dhry2reg Hmean unixbench-dhry2reg-1 48929419.48 ( 0.00%) 49137039.06 ( 0.42%)
>> unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6275526953.25 ( 0.00%) 6265580479.15 ( -0.16%)
>> unixbench-syscall Amean unixbench-syscall-1 2994319.73 ( 0.00%) 3008596.83 * -0.48%*
>> unixbench-syscall Amean unixbench-syscall-512 7349715.87 ( 0.00%) 7420994.50 * -0.97%*
>> unixbench-pipe Hmean unixbench-pipe-1 2830206.03 ( 0.00%) 2854405.99 * 0.86%*
>> unixbench-pipe Hmean unixbench-pipe-512 326207828.01 ( 0.00%) 328997804.52 * 0.86%*
>> unixbench-spawn Hmean unixbench-spawn-1 6394.21 ( 0.00%) 6367.75 ( -0.41%)
>> unixbench-spawn Hmean unixbench-spawn-512 72700.64 ( 0.00%) 71454.19 * -1.71%*
>> unixbench-execl Hmean unixbench-execl-1 4723.61 ( 0.00%) 4750.59 ( 0.57%)
>> unixbench-execl Hmean unixbench-execl-512 11212.05 ( 0.00%) 11262.13 ( 0.45%)
>>
>> o NPS2
>>
>> Test Metric Parallelism tip latency_nice
>> unixbench-dhry2reg Hmean unixbench-dhry2reg-1 49271512.85 ( 0.00%) 49245260.43 ( -0.05%)
>> unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6267992483.03 ( 0.00%) 6264951100.67 ( -0.05%)
>> unixbench-syscall Amean unixbench-syscall-1 2995885.93 ( 0.00%) 3005975.10 * -0.34%*
>> unixbench-syscall Amean unixbench-syscall-512 7388865.77 ( 0.00%) 7276275.63 * 1.52%*
>> unixbench-pipe Hmean unixbench-pipe-1 2828971.95 ( 0.00%) 2856578.72 * 0.98%*
>> unixbench-pipe Hmean unixbench-pipe-512 326225385.37 ( 0.00%) 328941270.81 * 0.83%*
>> unixbench-spawn Hmean unixbench-spawn-1 6958.71 ( 0.00%) 6954.21 ( -0.06%)
>> unixbench-spawn Hmean unixbench-spawn-512 85443.56 ( 0.00%) 70536.42 * -17.45%* (0.67% vs 0.93% - CoEff var)
>
> I don't expect any perf improvement or regression when the latency
> nice is not changed

This regression can be ignored. Although the results from back to
back runs are very stable, I see the results vary when I rebuild
the unixbench binaries on my test setup.

tip latency_nice
unixbench-spawn-512 73489.0 78260.4 (kexec)
unixbench-spawn-512 73332.7 77821.2 (reboot)
unixbench-spawn-512 86207.4 82281.2 (rebuilt + reboot)

I'll go back and look more into the spawn test because there is
something else at play there but other Unixbench results seem to
be stable looking at the rerun.

>
>> unixbench-execl Hmean unixbench-execl-1 4767.99 ( 0.00%) 4752.63 * -0.32%*
>> unixbench-execl Hmean unixbench-execl-512 11250.72 ( 0.00%) 11320.97 ( 0.62%)
>>
>> o NPS4
>>
>> Test Metric Parallelism tip latency_nice
>> unixbench-dhry2reg Hmean unixbench-dhry2reg-1 49041932.68 ( 0.00%) 49156671.05 ( 0.23%)
>> unixbench-dhry2reg Hmean unixbench-dhry2reg-512 6286981589.85 ( 0.00%) 6285248711.40 ( -0.03%)
>> unixbench-syscall Amean unixbench-syscall-1 2992405.60 ( 0.00%) 3008933.03 * -0.55%*
>> unixbench-syscall Amean unixbench-syscall-512 7971789.70 ( 0.00%) 7814622.23 * 1.97%*
>> unixbench-pipe Hmean unixbench-pipe-1 2822892.54 ( 0.00%) 2852615.11 * 1.05%*
>> unixbench-pipe Hmean unixbench-pipe-512 326408309.83 ( 0.00%) 329617202.56 * 0.98%*
>> unixbench-spawn Hmean unixbench-spawn-1 7685.31 ( 0.00%) 7243.54 ( -5.75%)
>> unixbench-spawn Hmean unixbench-spawn-512 72245.56 ( 0.00%) 77000.81 * 6.58%*
>> unixbench-execl Hmean unixbench-execl-1 4761.42 ( 0.00%) 4733.12 * -0.59%*
>> unixbench-execl Hmean unixbench-execl-512 11533.53 ( 0.00%) 11660.17 ( 1.10%)
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~ Hackbench - Various Latency Nice Values ~
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> o 100000 loops
>>
>> - pipe (process)
>>
>> Test: LN: 0 LN: 19 LN: -20
>> 1-groups: 3.91 (0.00 pct) 3.91 (0.00 pct) 3.81 (2.55 pct)
>> 2-groups: 4.48 (0.00 pct) 4.52 (-0.89 pct) 4.53 (-1.11 pct)
>> 4-groups: 4.83 (0.00 pct) 4.83 (0.00 pct) 4.87 (-0.82 pct)
>> 8-groups: 5.09 (0.00 pct) 5.00 (1.76 pct) 5.07 (0.39 pct)
>> 16-groups: 6.92 (0.00 pct) 6.79 (1.87 pct) 6.96 (-0.57 pct)
>>
>> - pipe (thread)
>>
>> 1-groups: 4.13 (0.00 pct) 4.08 (1.21 pct) 4.11 (0.48 pct)
>> 2-groups: 4.78 (0.00 pct) 4.90 (-2.51 pct) 4.79 (-0.20 pct)
>> 4-groups: 5.12 (0.00 pct) 5.08 (0.78 pct) 5.16 (-0.78 pct)
>> 8-groups: 5.31 (0.00 pct) 5.28 (0.56 pct) 5.33 (-0.37 pct)
>> 16-groups: 7.34 (0.00 pct) 7.27 (0.95 pct) 7.33 (0.13 pct)
>>
>> - socket (process)
>>
>> Test: LN: 0 LN: 19 LN: -20
>> 1-groups: 6.61 (0.00 pct) 6.38 (3.47 pct) 6.54 (1.05 pct)
>> 2-groups: 6.59 (0.00 pct) 6.67 (-1.21 pct) 6.11 (7.28 pct)
>> 4-groups: 6.77 (0.00 pct) 6.78 (-0.14 pct) 6.79 (-0.29 pct)
>> 8-groups: 8.29 (0.00 pct) 8.39 (-1.20 pct) 8.36 (-0.84 pct)
>> 16-groups: 12.21 (0.00 pct) 12.03 (1.47 pct) 12.35 (-1.14 pct)
>>
>> - socket (thread)
>>
>> Test: LN: 0 LN: 19 LN: -20
>> 1-groups: 6.50 (0.00 pct) 5.99 (7.84 pct) 6.02 (7.38 pct) ^
>> 2-groups: 6.07 (0.00 pct) 6.20 (-2.14 pct) 6.23 (-2.63 pct)
>> 4-groups: 6.61 (0.00 pct) 6.64 (-0.45 pct) 6.63 (-0.30 pct)
>> 8-groups: 8.87 (0.00 pct) 8.67 (2.25 pct) 8.78 (1.01 pct)
>> 16-groups: 12.63 (0.00 pct) 12.54 (0.71 pct) 12.59 (0.31 pct)
>>
>>> [..snip..]
>>>
>>
>> Apart from couple of anomalies, latency nice reduces wait time, especially
>> when the system is heavily loaded. If there is any data, or any specific
>> workload you would like me to run on the test system, please do let me know.
>> Meanwhile, I'll try to get some numbers for larger workloads like SpecJBB
>> that did see improvements with latency nice on v5.

Following are results for SpecJBB in NPS1 mode:

+----------------------------------------------+
| | Latency Nice | |
| Metric |-------------------| tip |
| | 0 | 19 | |
|----------------|-------------------|---------|
| Max jOPS | 100.00% | 102.19% | 101.02% |
| Criritcal jOPS | 100.00% | 122.41% | 100.41% |
+----------------------------------------------+

SpecJBB throughput for Max-jOPS is similar across the board
but Critical-jOPS throughput sees a good uplift again with
latency nice 19.

>
> [..snip..]
>

If there is any specific workload you would like me to test,
please do let me know. I'll try to test more workloads I come
across with different latency nice values and update you
with the results on this thread.

Tested-by: K Prateek Nayak <[email protected]>
--
Thanks and Regards,
Prateek