sched: Fix smp_call_function_single_async() usage for ILB
The recent commit:90b5363acd("sched: Clean up scheduler_ipi()") got smp_call_function_single_async() subtly wrong. Even though it will return -EBUSY when trying to re-use a csd, that condition is not atomic and still requires external serialization. The change in kick_ilb() got this wrong. While on first reading kick_ilb() has an atomic test-and-set that appears to serialize the use, the matching 'release' is not in the right place to actually guarantee this serialization. Rework the nohz_idle_balance() trigger so that the release is in the IPI callback and thus guarantees the required serialization for the CSD. Fixes:90b5363acd("sched: Clean up scheduler_ipi()") Reported-by: Qian Cai <cai@lca.pw> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by: Ingo Molnar <mingo@kernel.org> Reviewed-by: Frederic Weisbecker <frederic@kernel.org> Cc: mgorman@techsingularity.net Link: https://lore.kernel.org/r/20200526161907.778543557@infradead.org
This commit is contained in:
		
							parent
							
								
									58ef57b16d
								
							
						
					
					
						commit
						19a1f5ec69
					
				| @ -637,41 +637,25 @@ void wake_up_nohz_cpu(int cpu) | ||||
| 		wake_up_idle_cpu(cpu); | ||||
| } | ||||
| 
 | ||||
| static inline bool got_nohz_idle_kick(void) | ||||
| { | ||||
| 	int cpu = smp_processor_id(); | ||||
| 
 | ||||
| 	if (!(atomic_read(nohz_flags(cpu)) & NOHZ_KICK_MASK)) | ||||
| 		return false; | ||||
| 
 | ||||
| 	if (idle_cpu(cpu) && !need_resched()) | ||||
| 		return true; | ||||
| 
 | ||||
| 	/*
 | ||||
| 	 * We can't run Idle Load Balance on this CPU for this time so we | ||||
| 	 * cancel it and clear NOHZ_BALANCE_KICK | ||||
| 	 */ | ||||
| 	atomic_andnot(NOHZ_KICK_MASK, nohz_flags(cpu)); | ||||
| 	return false; | ||||
| } | ||||
| 
 | ||||
| static void nohz_csd_func(void *info) | ||||
| { | ||||
| 	struct rq *rq = info; | ||||
| 	int cpu = cpu_of(rq); | ||||
| 	unsigned int flags; | ||||
| 
 | ||||
| 	if (got_nohz_idle_kick()) { | ||||
| 		rq->idle_balance = 1; | ||||
| 	/*
 | ||||
| 	 * Release the rq::nohz_csd. | ||||
| 	 */ | ||||
| 	flags = atomic_fetch_andnot(NOHZ_KICK_MASK, nohz_flags(cpu)); | ||||
| 	WARN_ON(!(flags & NOHZ_KICK_MASK)); | ||||
| 
 | ||||
| 	rq->idle_balance = idle_cpu(cpu); | ||||
| 	if (rq->idle_balance && !need_resched()) { | ||||
| 		rq->nohz_idle_balance = flags; | ||||
| 		raise_softirq_irqoff(SCHED_SOFTIRQ); | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| #else /* CONFIG_NO_HZ_COMMON */ | ||||
| 
 | ||||
| static inline bool got_nohz_idle_kick(void) | ||||
| { | ||||
| 	return false; | ||||
| } | ||||
| 
 | ||||
| #endif /* CONFIG_NO_HZ_COMMON */ | ||||
| 
 | ||||
| #ifdef CONFIG_NO_HZ_FULL | ||||
|  | ||||
| @ -10024,6 +10024,10 @@ static void kick_ilb(unsigned int flags) | ||||
| 	if (ilb_cpu >= nr_cpu_ids) | ||||
| 		return; | ||||
| 
 | ||||
| 	/*
 | ||||
| 	 * Access to rq::nohz_csd is serialized by NOHZ_KICK_MASK; he who sets | ||||
| 	 * the first flag owns it; cleared by nohz_csd_func(). | ||||
| 	 */ | ||||
| 	flags = atomic_fetch_or(flags, nohz_flags(ilb_cpu)); | ||||
| 	if (flags & NOHZ_KICK_MASK) | ||||
| 		return; | ||||
| @ -10371,20 +10375,14 @@ abort: | ||||
|  */ | ||||
| static bool nohz_idle_balance(struct rq *this_rq, enum cpu_idle_type idle) | ||||
| { | ||||
| 	int this_cpu = this_rq->cpu; | ||||
| 	unsigned int flags; | ||||
| 	unsigned int flags = this_rq->nohz_idle_balance; | ||||
| 
 | ||||
| 	if (!(atomic_read(nohz_flags(this_cpu)) & NOHZ_KICK_MASK)) | ||||
| 	if (!flags) | ||||
| 		return false; | ||||
| 
 | ||||
| 	if (idle != CPU_IDLE) { | ||||
| 		atomic_andnot(NOHZ_KICK_MASK, nohz_flags(this_cpu)); | ||||
| 		return false; | ||||
| 	} | ||||
| 	this_rq->nohz_idle_balance = 0; | ||||
| 
 | ||||
| 	/* could be _relaxed() */ | ||||
| 	flags = atomic_fetch_andnot(NOHZ_KICK_MASK, nohz_flags(this_cpu)); | ||||
| 	if (!(flags & NOHZ_KICK_MASK)) | ||||
| 	if (idle != CPU_IDLE) | ||||
| 		return false; | ||||
| 
 | ||||
| 	_nohz_idle_balance(this_rq, flags, idle); | ||||
|  | ||||
| @ -951,6 +951,7 @@ struct rq { | ||||
| 
 | ||||
| 	struct callback_head	*balance_callback; | ||||
| 
 | ||||
| 	unsigned char		nohz_idle_balance; | ||||
| 	unsigned char		idle_balance; | ||||
| 
 | ||||
| 	unsigned long		misfit_task_load; | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user