Re: [Xen-devel] [PATCH] xen: avoid updating node affinity twice when removing a CPU from a cpupool

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 03/12/2015 03:52 PM, Dario Faggioli wrote:
> On Thu, 2015-03-12 at 14:51 +0000, Jan Beulich wrote:
>>>>> On 12.03.15 at 14:45, <dario.faggioli@xxxxxxxxxx> wrote:
>>> Patch below, and attached. However, I think the correct thing to do
>>> would be to just revert 93be8285 "update domU's node-affinity on the
>>> cpupool_unassign_cpu() path", wouldn't it?
>>
>> Indeed - if the presented patch is what we want, it should be
>> carried out as a revert. But you'll then want to explain why you
>> did what you did there in the first place: 
>>
> Because I thought it was necessary. ISTR I spotted the lack of symmetry
> that George is also mentioning, by looking at its _assign_ counterpart,
> and did not notice, at that time, that it was actually ok, as the update
> happens already, although in schedule.c...
> 
>> It surely wasn't without
>> reason, 
>>
> It was for a wrong reason. :-)
> 
>> and hence I'd be afraid the revert would re-introduce
>> another problem. That explanation should then probably go in
>> as description for the revert.
>>
> I'm not sure I'm getting 100% of what you mean. Let me try:

I'd say something like:
----
Revert c/s 93be8285

At the point this patch calls domain_update_node_affinity(), the vcpu
hard affinities have not yet been updated; so calling it at this point
can in some circumstances trigger an ASSERT().

domain_update_node_affinity() is already called in
cpu_disable_scheduler(), so adding it to cpupool_unassign_cpu() is
redundant.  Simply reverting the patch is sufficient.
---

 -George


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel