Re: [PATCH v5] videobuf2: Add missing lock held on vb2_fop_relase

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

Hello,

(dropping some unrelated e-mail addresses from Cc)

On 06/11/13 10:07, Hans Verkuil wrote:
> On 11/06/13 09:24, Ricardo Ribalda Delgado wrote:
>> From: Ricardo Ribalda <ricardo.ribalda@gmail.com>
>>
>> vb2_fop_relase does not held the lock although it is modifying the
> 
> Small typo: _relase -> _release
> 
>> queue->owner field.
>>
[...]
>>
>> Signed-off-by: Ricardo Ribalda <ricardo.ribalda@gmail.com>
>> ---
>>
>> v2: Comments by Sylvester Nawrocki
>>
>> fimc-capture and fimc-lite where calling vb2_fop_release with the lock held.
>> Therefore a new __vb2_fop_release function has been created to be used by
>> drivers that overload the release function.
>>
>> v3: Comments by Sylvester Nawrocki and Mauro Carvalho Chehab
>>
>> Use vb2_fop_release_locked instead of __vb2_fop_release
>>
>> v4: Comments by Sylvester Nawrocki
>>
>> Rename vb2_fop_release_locked to __vb2_fop_release and fix patch format
>>
>> v5: Comments by Sylvester Nawrocki and Hans Verkuil
>>
>> Rename __vb2_fop_release to vb2_fop_release_unlock and rearrange
>> arguments
> 
> I know I suggested the vb2_fop_release_unlock name, but on second thoughts
> that's not a good name. I suggest vb2_fop_release_no_lock instead.
> '_unlock' suggests that there is a _lock version as well, which isn't the
> case here.

Yes. Sorry, but to me vb2_fop_release_no_lock() looks s bit ugly.
Couldn't we just use double underscore prefix instead of _no_lock postfix,
as is commonly done in the kernel ?
Grep reveals almost no use of "_no_lock" in function names.

Thanks,
Sylwester