Re: [PATCH 09/10] iio:buffer: Add proper locking for iio_update_buffers()

[Jonathan Cameron <jic23@kernel.org>]

On 09/21/13 12:05, Lars-Peter Clausen wrote:
> On 09/21/2013 01:59 PM, Jonathan Cameron wrote:
>> On 09/18/13 21:27, Lars-Peter Clausen wrote:
>>> [...]
>>>> +int iio_update_buffers(struct iio_dev *indio_dev,
>>>> +               struct iio_buffer *insert_buffer,
>>>> +               struct iio_buffer *remove_buffer)
>>>> +{
>>>> +    int ret;
>>>> +
>>>> +    mutex_lock(&indio_dev->info_exist_lock);
>>>> +    mutex_lock(&indio_dev->mlock);
>>>> +
>>>> +    if (indio_dev->info == NULL)
>>>> +        return -ENODEV;
>>>
>>> Yea, ok that happens when send patches way to late in the evening. Of course we still need to unlock here...
>> Any reason not to do the indio_dev->info test before locking in the first place?
> 
> The check and the updating needs to happen in one atomic step, otherwise the updating might race against the
> iio_disable_all_buffers in the iio_device_unregister function.
Ah, gotcha.  Could you add a trivial comment for idiots like me pointing that out :)
> 
>>>
>>>> +    ret = __iio_update_buffers(indio_dev, insert_buffer, remove_buffer);
>>>> +
>>>> +    mutex_unlock(&indio_dev->mlock);
>>>> +    mutex_unlock(&indio_dev->info_exist_lock);
>>>> +
>>>> +    return ret;
>>>> +}
>>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>