Re: [PATCH] smp/hotplug: Move unparking of percpu threads to the control CPU

[Thomas Gleixner <tglx@linutronix.de>]

On Wed, 5 Jul 2017, Peter Zijlstra wrote:
> On Tue, Jul 04, 2017 at 10:20:23PM +0200, Thomas Gleixner wrote:
> > @@ -775,23 +787,13 @@ void notify_cpu_starting(unsigned int cp
> 
> The comment right above this function now seems stale..

Will fix.

> >  void cpuhp_online_idle(enum cpuhp_state state)
> >  {
> >  	struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state);
> > -	unsigned int cpu = smp_processor_id();
> >  
> >  	/* Happens for the boot cpu */
> >  	if (state != CPUHP_AP_ONLINE_IDLE)
> >  		return;
> >  
> >  	st->state = CPUHP_AP_ONLINE_IDLE;
> > -
> > -	/* Unpark the stopper thread and the hotplug thread of this cpu */
> > -	stop_machine_unpark(cpu);
> > -	kthread_unpark(st->thread);
> > -
> > -	/* Should we go further up ? */
> > -	if (st->target > CPUHP_AP_ONLINE_IDLE)
> > -		__cpuhp_kick_ap_work(st);
> > -	else
> > -		complete(&st->done);
> > +	complete(&st->done);
> >  }
> 
> 
> OK, so if I get this right we do something like:
> 
> 
> BP				AP
> 
> bringup_cpu();
>   __cpu_up()  ------------>     /* stuff */
>   bringup_wait_for_ap()
>     wait_for_completion();
> 				cpuhp_online_idle();
> 		<------------    complete(&st->done);
>     unpark()
> 				while(1)
> 				  do_idle();

actually I added after unpark():

     kick_ap()
     wait_for_completion()

So the AP will execute the online callbacks in its own hotplug thread.

> Where you moved the unpark() from the AP's idle thread to the BP's
> context and thus allow scheduling etc..
> 
> Yes that should work fine I think.

Thanks,

	tglx