Re: [PATCH v4 3/4] VT-d PI: restrict the vcpu number on a given pcpu

[Chao Gao <chao.gao@intel.com>]

On Mon, Jul 10, 2017 at 03:36:52AM -0600, Jan Beulich wrote:
>>>> On 10.07.17 at 03:17, <chao.gao@intel.com> wrote:
>> On Fri, Jul 07, 2017 at 09:57:47AM -0600, Jan Beulich wrote:
>>>>>> On 07.07.17 at 08:48, <chao.gao@intel.com> wrote:
>>>> +#define remote_pbl_operation_begin(flags)                   \
>>>> +({                                                          \
>>>> +    spin_lock_irqsave(&remote_pbl_operation, flags);        \
>>>> +})
>>>> +
>>>> +#define remote_pbl_operation_done(flags)                    \
>>>> +({                                                          \
>>>> +    spin_unlock_irqrestore(&remote_pbl_operation, flags);   \
>>>> +})
>>>
>>>No need for the ({ }) here.
>>>
>>>But then I don't understand what this is needed for in the first
>>>place. If this is once again about CPU offlining, then I can only
>>>repeat that such happens in stop_machine context. Otherwise
>> 
>> But I don't think vmx_pi_desc_fixup() happens in stop_machine context,
>> please refer to cpu_callback() function in hvm.c and the time
>> notifier_call_chain(CPU_DEAD) is called in cpu_down().
>
>While that's true, the CPU at that point is no longer marked
>online, so it shouldn't be a candidate anyway.
>
>> Our goal here is to avoid adding one entry to a destroyed list.
>> To avoid destruction happens during adding, we can put these two
>> process in critical sections, like
>> 
>> add:
>> 	remote_pbl_operation_begin()
>> 	add one entry to the list
>> 	remote_pbl_operation_end()
>> 
>> destroy:
>> 	remote_pbl_operation_begin()
>> 	destruction
>> 	remote_pbl_operation_end()
>> 
>> Destruction may happen before we enter the critical section.
>
>I don't think so, no: Xen is not preemptible, and stop-machine logic
>involves scheduling a tasklet on each pCPU and waiting for it to
>gain control. So as long as you don't "manually" force tasklets to
>be run, I still don't see the need for this extra locking.

Impressive! I understand why you repeatedly mentioned stop_machine()
context to me now.

Thanks
Chao

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