Re: [Xen-devel] [PATCHv8 2/3] gnttab: per-active entry locking

[David Vrabel <david.vrabel@xxxxxxxxxx>]

On 19/05/15 09:27, Jan Beulich wrote:
>>>> On 12.05.15 at 16:15, <david.vrabel@xxxxxxxxxx> wrote:
>> @@ -546,15 +554,28 @@ static void mapcount(
>>  
>>      *wrc = *rdc = 0;
>>  
>> +    /* 
>> +     * N.B.: while taking the local maptrack spinlock prevents any
>> +     * mapping changes, the remote active entries could be changing
>> +     * while we are counting. The caller has to hold the grant table
>> +     * write lock, or some other mechanism should be used to prevent
>> +     * concurrent changes during this operation.  This is tricky
>> +     * because we can't promote a read lock into a write lock.
>> +     */
>> +    ASSERT(rw_is_locked(&rd->grant_table->lock));
> 
> So both callers only hold the read lock - what is this "other mechanism"
> that they employ?

This comment is just nonsense.  We must hold the grant table write lock
for mapcount() to be safe -- the maptrack lock doesn't help and as you
say, this "other mechanism" just doesn't exist.

I think the callers of mapcount() need to be refactored so iommu updates
and the mapcount() call are done with the write lock held only (the
maptrack nor active entry locks are relevant here).

David

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