mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	sched: Optimize finish_lock_switch()
The kernel test robot measured a -1.6% performance regression on will-it-scale/sched_yield due to commit:2558aacff8("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:2558aacff8("sched/hotplug: Ensure only per-cpu kthreads run during hotplug") Reported-by: kernel test robot <oliver.sang@intel.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20201210161408.GX3021@hirez.programming.kicks-ass.net
This commit is contained in:
		
							parent
							
								
									5b78f2dc31
								
							
						
					
					
						commit
						ae79270232
					
				
					 2 changed files with 20 additions and 33 deletions
				
			
		| 
						 | 
					@ -3985,15 +3985,20 @@ static void do_balance_callbacks(struct rq *rq, struct callback_head *head)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					static void balance_push(struct rq *rq);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					struct callback_head balance_push_callback = {
 | 
				
			||||||
 | 
						.next = NULL,
 | 
				
			||||||
 | 
						.func = (void (*)(struct callback_head *))balance_push,
 | 
				
			||||||
 | 
					};
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static inline struct callback_head *splice_balance_callbacks(struct rq *rq)
 | 
					static inline struct callback_head *splice_balance_callbacks(struct rq *rq)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	struct callback_head *head = rq->balance_callback;
 | 
						struct callback_head *head = rq->balance_callback;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	lockdep_assert_held(&rq->lock);
 | 
						lockdep_assert_held(&rq->lock);
 | 
				
			||||||
	if (head) {
 | 
						if (head)
 | 
				
			||||||
		rq->balance_callback = NULL;
 | 
							rq->balance_callback = NULL;
 | 
				
			||||||
		rq->balance_flags &= ~BALANCE_WORK;
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
	return head;
 | 
						return head;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					@ -4014,21 +4019,6 @@ static inline void balance_callbacks(struct rq *rq, struct callback_head *head)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static void balance_push(struct rq *rq);
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
static inline void balance_switch(struct rq *rq)
 | 
					 | 
				
			||||||
{
 | 
					 | 
				
			||||||
	if (likely(!rq->balance_flags))
 | 
					 | 
				
			||||||
		return;
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	if (rq->balance_flags & BALANCE_PUSH) {
 | 
					 | 
				
			||||||
		balance_push(rq);
 | 
					 | 
				
			||||||
		return;
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	__balance_callbacks(rq);
 | 
					 | 
				
			||||||
}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
#else
 | 
					#else
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static inline void __balance_callbacks(struct rq *rq)
 | 
					static inline void __balance_callbacks(struct rq *rq)
 | 
				
			||||||
| 
						 | 
					@ -4044,10 +4034,6 @@ static inline void balance_callbacks(struct rq *rq, struct callback_head *head)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static inline void balance_switch(struct rq *rq)
 | 
					 | 
				
			||||||
{
 | 
					 | 
				
			||||||
}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
#endif
 | 
					#endif
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static inline void
 | 
					static inline void
 | 
				
			||||||
| 
						 | 
					@ -4075,7 +4061,7 @@ static inline void finish_lock_switch(struct rq *rq)
 | 
				
			||||||
	 * prev into current:
 | 
						 * prev into current:
 | 
				
			||||||
	 */
 | 
						 */
 | 
				
			||||||
	spin_acquire(&rq->lock.dep_map, 0, 0, _THIS_IP_);
 | 
						spin_acquire(&rq->lock.dep_map, 0, 0, _THIS_IP_);
 | 
				
			||||||
	balance_switch(rq);
 | 
						__balance_callbacks(rq);
 | 
				
			||||||
	raw_spin_unlock_irq(&rq->lock);
 | 
						raw_spin_unlock_irq(&rq->lock);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -7256,6 +7242,10 @@ static void balance_push(struct rq *rq)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	lockdep_assert_held(&rq->lock);
 | 
						lockdep_assert_held(&rq->lock);
 | 
				
			||||||
	SCHED_WARN_ON(rq->cpu != smp_processor_id());
 | 
						SCHED_WARN_ON(rq->cpu != smp_processor_id());
 | 
				
			||||||
 | 
						/*
 | 
				
			||||||
 | 
						 * Ensure the thing is persistent until balance_push_set(.on = false);
 | 
				
			||||||
 | 
						 */
 | 
				
			||||||
 | 
						rq->balance_callback = &balance_push_callback;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/*
 | 
						/*
 | 
				
			||||||
	 * Both the cpu-hotplug and stop task are in this case and are
 | 
						 * 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)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	rq_lock_irqsave(rq, &rf);
 | 
						rq_lock_irqsave(rq, &rf);
 | 
				
			||||||
	if (on)
 | 
						if (on)
 | 
				
			||||||
		rq->balance_flags |= BALANCE_PUSH;
 | 
							rq->balance_callback = &balance_push_callback;
 | 
				
			||||||
	else
 | 
						else
 | 
				
			||||||
		rq->balance_flags &= ~BALANCE_PUSH;
 | 
							rq->balance_callback = NULL;
 | 
				
			||||||
	rq_unlock_irqrestore(rq, &rf);
 | 
						rq_unlock_irqrestore(rq, &rf);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -975,7 +975,6 @@ struct rq {
 | 
				
			||||||
	unsigned long		cpu_capacity_orig;
 | 
						unsigned long		cpu_capacity_orig;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	struct callback_head	*balance_callback;
 | 
						struct callback_head	*balance_callback;
 | 
				
			||||||
	unsigned char		balance_flags;
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
	unsigned char		nohz_idle_balance;
 | 
						unsigned char		nohz_idle_balance;
 | 
				
			||||||
	unsigned char		idle_balance;
 | 
						unsigned char		idle_balance;
 | 
				
			||||||
| 
						 | 
					@ -1226,6 +1225,8 @@ struct rq_flags {
 | 
				
			||||||
#endif
 | 
					#endif
 | 
				
			||||||
};
 | 
					};
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					extern struct callback_head balance_push_callback;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
/*
 | 
					/*
 | 
				
			||||||
 * Lockdep annotation that avoids accidental unlocks; it's like a
 | 
					 * Lockdep annotation that avoids accidental unlocks; it's like a
 | 
				
			||||||
 * sticky/continuous lockdep_assert_held().
 | 
					 * sticky/continuous lockdep_assert_held().
 | 
				
			||||||
| 
						 | 
					@ -1243,9 +1244,9 @@ static inline void rq_pin_lock(struct rq *rq, struct rq_flags *rf)
 | 
				
			||||||
#ifdef CONFIG_SCHED_DEBUG
 | 
					#ifdef CONFIG_SCHED_DEBUG
 | 
				
			||||||
	rq->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
 | 
						rq->clock_update_flags &= (RQCF_REQ_SKIP|RQCF_ACT_SKIP);
 | 
				
			||||||
	rf->clock_update_flags = 0;
 | 
						rf->clock_update_flags = 0;
 | 
				
			||||||
#endif
 | 
					 | 
				
			||||||
#ifdef CONFIG_SMP
 | 
					#ifdef CONFIG_SMP
 | 
				
			||||||
	SCHED_WARN_ON(rq->balance_callback);
 | 
						SCHED_WARN_ON(rq->balance_callback && rq->balance_callback != &balance_push_callback);
 | 
				
			||||||
 | 
					#endif
 | 
				
			||||||
#endif
 | 
					#endif
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1408,9 +1409,6 @@ init_numa_balancing(unsigned long clone_flags, struct task_struct *p)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
#ifdef CONFIG_SMP
 | 
					#ifdef CONFIG_SMP
 | 
				
			||||||
 | 
					
 | 
				
			||||||
#define BALANCE_WORK	0x01
 | 
					 | 
				
			||||||
#define BALANCE_PUSH	0x02
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
static inline void
 | 
					static inline void
 | 
				
			||||||
queue_balance_callback(struct rq *rq,
 | 
					queue_balance_callback(struct rq *rq,
 | 
				
			||||||
		       struct callback_head *head,
 | 
							       struct callback_head *head,
 | 
				
			||||||
| 
						 | 
					@ -1418,13 +1416,12 @@ queue_balance_callback(struct rq *rq,
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	lockdep_assert_held(&rq->lock);
 | 
						lockdep_assert_held(&rq->lock);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (unlikely(head->next || (rq->balance_flags & BALANCE_PUSH)))
 | 
						if (unlikely(head->next || rq->balance_callback == &balance_push_callback))
 | 
				
			||||||
		return;
 | 
							return;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	head->func = (void (*)(struct callback_head *))func;
 | 
						head->func = (void (*)(struct callback_head *))func;
 | 
				
			||||||
	head->next = rq->balance_callback;
 | 
						head->next = rq->balance_callback;
 | 
				
			||||||
	rq->balance_callback = head;
 | 
						rq->balance_callback = head;
 | 
				
			||||||
	rq->balance_flags |= BALANCE_WORK;
 | 
					 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
#define rcu_dereference_check_sched_domain(p) \
 | 
					#define rcu_dereference_check_sched_domain(p) \
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue