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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Jul 2017 11:07:34 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Peter Zijlstra <peterz@...radead.org>
cc:     Vikram Mulukutla <markivx@...eaurora.org>,
        Rusty Russell <rusty@...tcorp.com.au>,
        Tejun Heo <tj@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Sebastian Sewior <bigeasy@...utronix.de>
Subject: Re: [PATCH] smp/hotplug: Move unparking of percpu threads to the
 control CPU

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