Re: [Xen-devel] [PATCH v3 3/3] VT-d PI: restrict the vcpu number on a given pcpu

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.06.17 at 06:22, <chao.gao@xxxxxxxxx> wrote:
> On Fri, Jun 16, 2017 at 09:09:13AM -0600, Jan Beulich wrote:
>>>>> On 24.05.17 at 08:56, <chao.gao@xxxxxxxxx> wrote:
>>> +    {
>>> +        pi_cpu = cpumask_cycle(pi_cpu, &cpu_online_map);
>>
>>With this, how could the CPU be offline by the time you make it
>>back to the check above.
> 
> Thanks to point it out. It would incur a bug.

I don't understand what you're trying to tell me here.

> I think we should do things like this:
> 
> IF pi_blocking_list of current pcpu doesn't over the limit:
>       add the vcpu to current pcpu.
> ELSE
>       add the vcpu to another pcpu.

But that's what supposedly the patch already tries to do?

> To add the vcpu to another pcpu, we should avoid concurrency with
> vmx_pi_desc_fixup(). Thus, a lock (e.g. remote_pi_list_lock)

Please use names which actually exist in source code, or make
clear what exactly you're referring to. Talking of
remote_pi_list_lock, which I can't find any instance of, does not
help the discussion, as you leave me guessing whose lock you
mean to acquire.

> can solve this potential concurrency. Using this lock like below:
> 
> in vmx_vcpu_block():
> 
> IF pi_blocking_list of current pcpu doesn't over the limit:
>       add the vcpu to current pcpu
> ELSE
>       acquire remote_pi_list_lock
>       choose another online pcpu      (don't worry this pcpu would goes
>                                        offline for we hold the
>                                        remote_pi_list_lock, which blocks
>                                        calling vmx_pi_desc_fixup(),
>                                        thus at least we can add this
>                                        vcpu to the pi_blocking_list
>                                        before cleanup)

I can't see why you need to hold a lock to make sure a pCPU doesn't
go offline - pCPU offlining happens in stop_machine context anyway.

Jan

>       add the vcpu to the chosen pcpu
>       release remote_pi_list_lock
> 
> in vmx_pi_desc_fixup():
> acquire remote_pi_list_lock
> ...
> release remote_pi_list_lock
> 
> Thanks
> Chao




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