Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-20 Thread Peter Zijlstra
On Tue, Apr 20, 2021 at 05:53:40PM +0100, Vincent Donnefort wrote: > All good with that snippet on my end. > > I wonder if balance_push() shouldn't use the cpu_of() accessor > instead of rq->cpu. That might be a personal quirk of mine, but for code that is under CONFIG_SMP (as all balancing code

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-20 Thread Vincent Donnefort
On Tue, Apr 20, 2021 at 04:58:00PM +0200, Peter Zijlstra wrote: > On Tue, Apr 20, 2021 at 04:39:04PM +0200, Peter Zijlstra wrote: > > On Tue, Apr 20, 2021 at 04:20:56PM +0200, Peter Zijlstra wrote: > > > On Tue, Apr 20, 2021 at 10:46:33AM +0100, Vincent Donnefort wrote: > > > > > > > Found the

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-20 Thread Peter Zijlstra
On Tue, Apr 20, 2021 at 04:39:04PM +0200, Peter Zijlstra wrote: > On Tue, Apr 20, 2021 at 04:20:56PM +0200, Peter Zijlstra wrote: > > On Tue, Apr 20, 2021 at 10:46:33AM +0100, Vincent Donnefort wrote: > > > > > Found the issue: > > > > > > $ cat hotplug/states: > > > 219: sched:active > > > 220:

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-20 Thread Peter Zijlstra
On Tue, Apr 20, 2021 at 04:20:56PM +0200, Peter Zijlstra wrote: > On Tue, Apr 20, 2021 at 10:46:33AM +0100, Vincent Donnefort wrote: > > > Found the issue: > > > > $ cat hotplug/states: > > 219: sched:active > > 220: online > > > > CPU0: > > > > $ echo 219 > hotplug/fail > > $ echo 0 > online

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-20 Thread Peter Zijlstra
On Tue, Apr 20, 2021 at 10:46:33AM +0100, Vincent Donnefort wrote: > Found the issue: > > $ cat hotplug/states: > 219: sched:active > 220: online > > CPU0: > > $ echo 219 > hotplug/fail > $ echo 0 > online > > => cpu_active = 1 cpu_dying = 1 > > which means that later on, for another CPU

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-20 Thread Vincent Donnefort
On Mon, Apr 19, 2021 at 11:56:30AM +0100, Vincent Donnefort wrote: > On Thu, Apr 15, 2021 at 03:32:11PM +0100, Valentin Schneider wrote: > > On 15/04/21 10:59, Peter Zijlstra wrote: > > > Can't make sense of what I did.. I've removed that hunk. Patch now looks > > > like this. > > > > > > > Small

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-19 Thread Vincent Donnefort
On Thu, Apr 15, 2021 at 03:32:11PM +0100, Valentin Schneider wrote: > On 15/04/21 10:59, Peter Zijlstra wrote: > > Can't make sense of what I did.. I've removed that hunk. Patch now looks > > like this. > > > > Small nit below, but regardless feel free to apply to the whole lot: > Reviewed-by:

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-15 Thread Valentin Schneider
On 15/04/21 17:29, Peter Zijlstra wrote: > On Thu, Apr 15, 2021 at 03:32:11PM +0100, Valentin Schneider wrote: >> I'd word that "is enabled below sched_cpu_activate()", since >> sched_cpu_deactivate() is now out of the picture. > > I are confused (again!). Did you mean to say something like: 'is

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-15 Thread Peter Zijlstra
On Thu, Apr 15, 2021 at 03:32:11PM +0100, Valentin Schneider wrote: > > So instead, make sure balance_push is enabled between > > sched_cpu_deactivate() and sched_cpu_activate() (eg. when > > !cpu_active()), and gate it's utility with cpu_dying(). > > I'd word that "is enabled below

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-15 Thread Valentin Schneider
On 15/04/21 10:59, Peter Zijlstra wrote: > Can't make sense of what I did.. I've removed that hunk. Patch now looks > like this. > Small nit below, but regardless feel free to apply to the whole lot: Reviewed-by: Valentin Schneider @VincentD, ISTR you had tested the initial version of this with

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-15 Thread Peter Zijlstra
On Tue, Apr 13, 2021 at 08:51:23AM +0200, Peter Zijlstra wrote: > > I'm afraid I don't follow; we're replacing a read of rq->balance_push with > > cpu_dying(), and those are still written on the same side of the > > synchronize_rcu(). What am I missing? > > Yeah, I'm not sure anymnore either; I

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-13 Thread Peter Zijlstra
On Mon, Apr 12, 2021 at 06:22:42PM +0100, Valentin Schneider wrote: > On 12/04/21 14:03, Peter Zijlstra wrote: > > On Thu, Mar 11, 2021 at 03:13:04PM +, Valentin Schneider wrote: > >> Peter Zijlstra writes: > >> > @@ -7910,6 +7908,14 @@ int sched_cpu_deactivate(unsigned int cp > >> >} >

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-12 Thread Valentin Schneider
On 12/04/21 14:03, Peter Zijlstra wrote: > On Thu, Mar 11, 2021 at 03:13:04PM +, Valentin Schneider wrote: >> Peter Zijlstra writes: >> > @@ -7910,6 +7908,14 @@ int sched_cpu_deactivate(unsigned int cp >> >} >> >rq_unlock_irqrestore(rq, ); >> > >> > + /* >> > + * From this point

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-04-12 Thread Peter Zijlstra
On Thu, Mar 11, 2021 at 03:13:04PM +, Valentin Schneider wrote: > Peter Zijlstra writes: > > @@ -7883,14 +7889,6 @@ int sched_cpu_deactivate(unsigned int cp > > set_cpu_active(cpu, false); > > > > /* > > -* From this point forward, this CPU will refuse to run any task that > > -

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-03-11 Thread Peter Zijlstra
On Thu, Mar 11, 2021 at 03:13:04PM +, Valentin Schneider wrote: > > #ifdef CONFIG_SCHED_SMT > > /* > > * When going down, decrement the number of cores with SMT present. > > > @@ -8206,7 +8212,7 @@ void __init sched_init(void) > > rq->sd = NULL; > > rq->rd =

Re: [PATCH 3/3] sched: Use cpu_dying() to fix balance_push vs hotplug-rollback

2021-03-11 Thread Valentin Schneider
Peter Zijlstra writes: > @@ -7883,14 +7889,6 @@ int sched_cpu_deactivate(unsigned int cp > set_cpu_active(cpu, false); > > /* > - * From this point forward, this CPU will refuse to run any task that > - * is not: migrate_disable() or KTHREAD_IS_PER_CPU, and will actively >