On Fri, Apr 1, 2016 at 4:48 AM, Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
> Am Donnerstag, den 31.03.2016, 16:26 -0400 schrieb Rob Clark:
>> A bit overkill since, for example, the rcu_dereference_protected() in
>> reservation_object_get_list() will WARN. But this is much less subtle
>> for folks reading the code.
>>
>> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>
>> ---
>> drivers/dma-buf/reservation.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
>> index c0bd572..0de3ea6 100644
>> --- a/drivers/dma-buf/reservation.c
>> +++ b/drivers/dma-buf/reservation.c
>> @@ -52,6 +52,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj)
>> struct reservation_object_list *fobj, *old;
>> u32 max;
>>
>> + WARN_ON(!ww_mutex_is_locked(&obj->lock));
>> +
> I don't really like those WARN_ONs to enforce the expected locking.
>
> IMHO lockdep_assert_held is the better way, as it's as obvious as the
> WARN_ONs to someone reading the code, blows up in the same way if you
> are using a lockdep enabled build (which should be default when touching
> locking code) and avoids the runtime overhead for production kernels.
>
> It also checks that it's really the expected execution strand holding
> the lock and not some other thread on another CPU.
>
> I don't know if lockdep_assert_held works with ww_mutex currently, if
> not we should definitely extend it to do so.
I guess it should, since there is reservation_object_held().. I
suppose I should use this.
I suppose all the WARN_ON(!drm_modeset_is_locked()) should someday get
the same treatment..
BR,
-R
> Regards,
> Lucas
>
>> old = reservation_object_get_list(obj);
>>
>> if (old && old->shared_max) {
>> @@ -189,6 +191,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
>> {
>> struct reservation_object_list *old, *fobj = obj->staged;
>>
>> + WARN_ON(!ww_mutex_is_locked(&obj->lock));
>> +
>> old = reservation_object_get_list(obj);
>> obj->staged = NULL;
>>
>> @@ -207,6 +211,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
>> struct reservation_object_list *old;
>> u32 i = 0;
>>
>> + WARN_ON(!ww_mutex_is_locked(&obj->lock));
>> +
>> old = reservation_object_get_list(obj);
>> if (old)
>> i = old->shared_count;
>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
