Re: [RFC PATCH 2/2 with seqcount v3] reservation: add suppport for read-only access using rcu

[Maarten Lankhorst <maarten.lankhorst@canonical.com>]

op 20-05-14 17:13, Thomas Hellstrom schreef:
> On 05/19/2014 03:13 PM, Maarten Lankhorst wrote:
>> op 19-05-14 15:42, Thomas Hellstrom schreef:
>>> Hi, Maarten!
>>>
>>> Some nitpicks, and that krealloc within rcu lock still worries me.
>>> Otherwise looks good.
>>>
>>> /Thomas
>>>
>>>
>>>
>>> On 04/23/2014 12:15 PM, Maarten Lankhorst wrote:
>>>> @@ -55,8 +60,8 @@ int reservation_object_reserve_shared(struct
>>>> reservation_object *obj)
>>>>                kfree(obj->staged);
>>>>                obj->staged = NULL;
>>>>                return 0;
>>>> -        }
>>>> -        max = old->shared_max * 2;
>>>> +        } else
>>>> +            max = old->shared_max * 2;
>>> Perhaps as a separate reformatting patch?
>> I'll fold it in to the patch that added
>> reservation_object_reserve_shared.
>>>> +
>>>> +int reservation_object_get_fences_rcu(struct reservation_object *obj,
>>>> +                      struct fence **pfence_excl,
>>>> +                      unsigned *pshared_count,
>>>> +                      struct fence ***pshared)
>>>> +{
>>>> +    unsigned shared_count = 0;
>>>> +    unsigned retry = 1;
>>>> +    struct fence **shared = NULL, *fence_excl = NULL;
>>>> +    int ret = 0;
>>>> +
>>>> +    while (retry) {
>>>> +        struct reservation_object_list *fobj;
>>>> +        unsigned seq;
>>>> +
>>>> +        seq = read_seqcount_begin(&obj->seq);
>>>> +
>>>> +        rcu_read_lock();
>>>> +
>>>> +        fobj = rcu_dereference(obj->fence);
>>>> +        if (fobj) {
>>>> +            struct fence **nshared;
>>>> +
>>>> +            shared_count = ACCESS_ONCE(fobj->shared_count);
>>> ACCESS_ONCE() shouldn't be needed inside the seqlock?
>> Yes it is, shared_count may be increased, leading to potential
>> different sizes for krealloc and memcpy
>> if the ACCESS_ONCE is removed. I could use shared_max here instead,
>> which stays the same,
>> but it would waste more memory.
> Maarten, Another perhaps ignorant question WRT this,
> Does ACCESS_ONCE() guarantee that the value accessed is read atomically?
Well I've reworked the code to use shared_max, so this point is moot. :-)

On any archs I'm aware of it would work, either the old or new value would be visible, as long as natural alignment is used.
rcu uses the same trick in the rcu_dereference macro, so if this didn't work rcu wouldn't work either.

~Maarten