Skip to content

Commit ae79270

Browse files
author
Peter Zijlstra
committed
sched: Optimize finish_lock_switch()
The kernel test robot measured a -1.6% performance regression on will-it-scale/sched_yield due to commit: 2558aac ("sched/hotplug: Ensure only per-cpu kthreads run during hotplug") Even though we were careful to replace a single load with another single load from the same cacheline. Restore finish_lock_switch() to the exact state before the offending patch and solve the problem differently. Fixes: 2558aac ("sched/hotplug: Ensure only per-cpu kthreads run during hotplug") Reported-by: kernel test robot <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent 5b78f2d commit ae79270

File tree

2 files changed

+20
-33
lines changed

2 files changed

+20
-33
lines changed

kernel/sched/core.c

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3985,15 +3985,20 @@ static void do_balance_callbacks(struct rq *rq, struct callback_head *head)
39853985
}
39863986
}
39873987

3988+
static void balance_push(struct rq *rq);
3989+
3990+
struct callback_head balance_push_callback = {
3991+
.next = NULL,
3992+
.func = (void (*)(struct callback_head *))balance_push,
3993+
};
3994+
39883995
static inline struct callback_head *splice_balance_callbacks(struct rq *rq)
39893996
{
39903997
struct callback_head *head = rq->balance_callback;
39913998

39923999
lockdep_assert_held(&rq->lock);
3993-
if (head) {
4000+
if (head)
39944001
rq->balance_callback = NULL;
3995-
rq->balance_flags &= ~BALANCE_WORK;
3996-
}
39974002

39984003
return head;
39994004
}
@@ -4014,21 +4019,6 @@ static inline void balance_callbacks(struct rq *rq, struct callback_head *head)
40144019
}
40154020
}
40164021

4017-
static void balance_push(struct rq *rq);
4018-
4019-
static inline void balance_switch(struct rq *rq)
4020-
{
4021-
if (likely(!rq->balance_flags))
4022-
return;
4023-
4024-
if (rq->balance_flags & BALANCE_PUSH) {
4025-
balance_push(rq);
4026-
return;
4027-
}
4028-
4029-
__balance_callbacks(rq);
4030-
}
4031-
40324022
#else
40334023

40344024
static inline void __balance_callbacks(struct rq *rq)
@@ -4044,10 +4034,6 @@ static inline void balance_callbacks(struct rq *rq, struct callback_head *head)
40444034
{
40454035
}
40464036

4047-
static inline void balance_switch(struct rq *rq)
4048-
{
4049-
}
4050-
40514037
#endif
40524038

40534039
static inline void
@@ -4075,7 +4061,7 @@ static inline void finish_lock_switch(struct rq *rq)
40754061
* prev into current:
40764062
*/
40774063
spin_acquire(&rq->lock.dep_map, 0, 0, _THIS_IP_);
4078-
balance_switch(rq);
4064+
__balance_callbacks(rq);
40794065
raw_spin_unlock_irq(&rq->lock);
40804066
}
40814067

@@ -7256,6 +7242,10 @@ static void balance_push(struct rq *rq)
72567242

72577243
lockdep_assert_held(&rq->lock);
72587244
SCHED_WARN_ON(rq->cpu != smp_processor_id());
7245+
/*
7246+
* Ensure the thing is persistent until balance_push_set(.on = false);
7247+
*/
7248+
rq->balance_callback = &balance_push_callback;
72597249

72607250
/*
72617251
* Both the cpu-hotplug and stop task are in this case and are
@@ -7305,9 +7295,9 @@ static void balance_push_set(int cpu, bool on)
73057295

73067296
rq_lock_irqsave(rq, &rf);
73077297
if (on)
7308-
rq->balance_flags |= BALANCE_PUSH;
7298+
rq->balance_callback = &balance_push_callback;
73097299
else
7310-
rq->balance_flags &= ~BALANCE_PUSH;
7300+
rq->balance_callback = NULL;
73117301
rq_unlock_irqrestore(rq, &rf);
73127302
}
73137303

kernel/sched/sched.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,6 @@ struct rq {
975975
unsigned long cpu_capacity_orig;
976976

977977
struct callback_head *balance_callback;
978-
unsigned char balance_flags;
979978

980979
unsigned char nohz_idle_balance;
981980
unsigned char idle_balance;
@@ -1226,6 +1225,8 @@ struct rq_flags {
12261225
#endif
12271226
};
12281227

1228+
extern struct callback_head balance_push_callback;
1229+
12291230
/*
12301231
* Lockdep annotation that avoids accidental unlocks; it's like a
12311232
* sticky/continuous lockdep_assert_held().
@@ -1243,9 +1244,9 @@ static inline void rq_pin_lock(struct rq *rq, struct rq_flags *rf)
12431244
#ifdef CONFIG_SCHED_DEBUG
12441245
rq->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
12451246
rf->clock_update_flags = 0;
1246-
#endif
12471247
#ifdef CONFIG_SMP
1248-
SCHED_WARN_ON(rq->balance_callback);
1248+
SCHED_WARN_ON(rq->balance_callback && rq->balance_callback != &balance_push_callback);
1249+
#endif
12491250
#endif
12501251
}
12511252

@@ -1408,23 +1409,19 @@ init_numa_balancing(unsigned long clone_flags, struct task_struct *p)
14081409

14091410
#ifdef CONFIG_SMP
14101411

1411-
#define BALANCE_WORK 0x01
1412-
#define BALANCE_PUSH 0x02
1413-
14141412
static inline void
14151413
queue_balance_callback(struct rq *rq,
14161414
struct callback_head *head,
14171415
void (*func)(struct rq *rq))
14181416
{
14191417
lockdep_assert_held(&rq->lock);
14201418

1421-
if (unlikely(head->next || (rq->balance_flags & BALANCE_PUSH)))
1419+
if (unlikely(head->next || rq->balance_callback == &balance_push_callback))
14221420
return;
14231421

14241422
head->func = (void (*)(struct callback_head *))func;
14251423
head->next = rq->balance_callback;
14261424
rq->balance_callback = head;
1427-
rq->balance_flags |= BALANCE_WORK;
14281425
}
14291426

14301427
#define rcu_dereference_check_sched_domain(p) \

0 commit comments

Comments
 (0)