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

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

On Fri, Sep 01, 2017 at 03:13:17AM -0600, Jan Beulich wrote:
>>>> On 01.09.17 at 09:55, <chao.gao@intel.com> wrote:
>> On Fri, Sep 01, 2017 at 02:24:08AM -0600, Jan Beulich wrote:
>>>>>> On 01.09.17 at 03:39, <chao.gao@intel.com> wrote:
>>>> After thinking it again, I want to define the counter as
>>>> a unsigned int variable for the following reasion:
>>>> 1. It is definite that the counter is closely related with
>>>> list_add() and list_del(). If the list is protected by the
>>>> lock, it is straightforward that the counter is also protected
>>>> by the lock.
>>>> 2. In patch 3, althought there are some lock-less readers, we
>>>> will check the counter still meets our requirement with the lock
>>>> held. Thus, I don't think there is a racing issue.
>>>
>>>I think that's fine, but then you still don't need LOCKed accesses
>>>to the counter for updating it; write_atomic() will suffice afaict.
>> 
>> A stupid question.
>> Is it contradictory that you think the counter can be protected by
>> the lock while suggesting using write_atomic() instead of LOCKed
>> accesses?
>> 
>> updating the counter is always accompanied by updating list and updating
>> list should in locked region. I meaned things like:
>> 
>> spin_lock()
>> list_add()
>> counter++
>> spin_unlock()
>> 
>> However, I am afraid that not using LOCKed accesses but using
>> write_atomic() means something like (separating updating the counter
>> from updating the list I think is not good):
>> 
>> spin_lock()
>> list_add()
>> spin_unlock()
>> write_atomic()
>
>No, I mean
>
> spin_lock()
> list_add()
> write_atomic()
> spin_unlock()
>
>whereas ...
>
>> And I think this version is:
>> 
>> spin_lock()
>> list_add()
>> add_sized()
>> spin_unlock()
>
>... this produces a needless LOCKed instruction redundant with being
>inside the locked region).

it seems add_sized() won't be a LOCKed instruction.
#define build_add_sized(name, size, type, reg) \
    static inline void name(volatile type *addr, type val)              \
    {                                                                   \
        asm volatile("add" size " %1,%0"                                \
                     : "=m" (*addr)                                     \
                     : reg (val));                                      \
    }

Thanks
Chao

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