Re: [PATCH v5 1/4] VT-d PI: track the number of vcpus on pi blocking list

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

On Wed, Aug 30, 2017 at 10:00:49AM -0600, Jan Beulich wrote:
>>>> On 16.08.17 at 07:14, <chao.gao@intel.com> wrote:
>> @@ -100,6 +101,24 @@ void vmx_pi_per_cpu_init(unsigned int cpu)
>>      spin_lock_init(&per_cpu(vmx_pi_blocking, cpu).lock);
>>  }
>>  
>> +static void vmx_pi_add_vcpu(struct pi_blocking_vcpu *pbv,
>> +                            struct vmx_pi_blocking_vcpu *vpbv)
>> +{
>> +    ASSERT(spin_is_locked(&vpbv->lock));
>
>You realize this is only a very weak check for a non-recursive lock?

I just thought the lock should be held when adding one entry to the
blocking list. Do you think we should remove this check or make it
stricter?

>
>> +    add_sized(&vpbv->counter, 1);
>> +    ASSERT(read_atomic(&vpbv->counter));
>
>Why add_sized() and read_atomic() when you hold the lock?
>

In patch 3, frequent reading the counter is used to find a suitable
vcpu and we can use add_sized() and read_atomic() to avoid acquiring the
lock. In one word, the lock doesn't protect the counter.

Thanks
Chao

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