[PATCH] reservation: sprinkle some WARN_ON()s

[Rob Clark]

On Sat, Jun 4, 2016 at 8:52 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Thu, Apr 07, 2016 at 08:52:56PM -0400, Rob Clark wrote:
>> 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.
>>
>> v2: use reservation_object_held() instead of ww_mutex_is_locked()
>>
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>
> Doesn't seem to build here:
>
>
> In file included from ./arch/arm/include/asm/bug.h:59:0,
>                  from include/linux/bug.h:4,
>                  from include/linux/thread_info.h:11,
>                  from include/asm-generic/current.h:4,
>                  from arch/arm/include/generated/asm/current.h:1,
>                  from include/linux/mutex.h:13,
>                  from include/linux/ww_mutex.h:17,
>                  from include/linux/reservation.h:42,
>                  from drivers/dma-buf/reservation.c:35:
> drivers/dma-buf/reservation.c: In function ‘reservation_object_reserve_shared’:
> include/linux/reservation.h:82:38: error: implicit declaration of function ‘lockdep_is_held’ [-Werror=implicit-function-declaration]
>  #define reservation_object_held(obj) lockdep_is_held(&(obj)->lock.base)
>                                       ^
> include/asm-generic/bug.h:92:25: note: in definition of macro ‘WARN_ON’
>   int __ret_warn_on = !!(condition);    \
>                          ^
> drivers/dma-buf/reservation.c:74:11: note: in expansion of macro ‘reservation_object_held’
>   WARN_ON(!reservation_object_held(obj));
>
> Sounds like we should include some headers from reservation.h.


Sumit hit something similar.. not sure if he figured out the problem.
But seemed to only work if lockdep was enabled, iirc?  It is probably
true that I didn't try building without lockdep..  although I don't
remember kbuild robot complaining.  I'm a bit confused since
reservation_object_held() was used before.

Hmm, on second thought, without lockdep, rcu_lockdep_assert() macro is
a no-op, so the use of reservation_object_held() goes away..  :-/

Maybe we should just go back to earlier version of the patch which
used mutex_is_locked() instead?

BR,
-R

> -Daniel
>
>> ---
>>  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..439af82 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(!reservation_object_held(obj));
>> +
>>       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(!reservation_object_held(obj));
>> +
>>       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(!reservation_object_held(obj));
>> +
>>       old = reservation_object_get_list(obj);
>>       if (old)
>>               i = old->shared_count;
>> --
>> 2.5.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch