Re: [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier

From: Juergen Gross <jgross@suse.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Dario Faggioli <dfaggioli@suse.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
Date: Wed, 27 Mar 2019 17:45:00 +0100	[thread overview]
Message-ID: <0b849a5c-9af8-2b47-ced3-376af22c6141@suse.com> (raw)
In-Reply-To: <5C9BA336020000780022235B@suse.com>

On 27/03/2019 17:22, Jan Beulich wrote:
>>>> On 18.03.19 at 14:11, <jgross@suse.com> wrote:
>> @@ -1738,6 +1733,9 @@ static int cpu_schedule_callback(
>>          rc = cpu_schedule_up(cpu);
>>          break;
>>      case CPU_DEAD:
>> +        rcu_read_lock(&domlist_read_lock);
>> +        cpu_disable_scheduler(cpu);
>> +        rcu_read_unlock(&domlist_read_lock);
>>          SCHED_OP(sched, deinit_pdata, sd->sched_priv, cpu);
>>          /* Fallthrough */
>>      case CPU_UP_CANCELED:
> 
> cpu_disable_scheduler() has a return value (and hence means to
> fail) - is ignoring this here really appropriate?

You are right, I should handle those cases in cpu_disable_scheduler().
Without the complete series committed this will result in a case not
handled correctly: dom0 trying to pin a vcpu to a physical cpu other
than cpu 0 via SCHEDOP_pin_override and suspending in that state. I'm
not aware of any dom0 trying to do that.

> Also while indeed (as the description says) there's no need to
> run the function on the CPU itself, it's not obvious to me that
> it's safe to run it outside of stop_machine() context. Or to be
> more precise, it's not clear to me that leaving stop_machine()
> context with the adjustments not done yet is not going to
> lead to problems (due to the gap between leaving that context
> and acquiring the RCU lock). Could you clarify this in the
> description, please (if it indeed is fine this way)?

It is fine, as the chances are zero that any code will run on the cpu
just taken down and that cpu is not holding any locks we might need.

I'll add that to the commit message.

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel