[Mesa-dev] [PATCH 1/3] clover: Refactor event::trigger and ::abort to prevent deadlock and reentrancy issues.

[Francisco Jerez]

Tom Stellard <tom at stellard.net> writes:

> On Sat, May 09, 2015 at 04:42:29PM +0300, Francisco Jerez wrote:
>> Refactor ::trigger and ::abort to split out the operations that access
>> concurrently modified data members and require locking from the
>> recursive and possibly re-entrant part of these methods.  This will
>> avoid some deadlock situations when locking is implemented.
>
> I've pushed the two bug-fixes that you reviewed, so this series should apply
> cleanly to master now.
>
> For the series:
> Tested-by: Tom Stellard <thomas.stellard at amd.com>
>
Thanks.

> CC: stable should also be added to these.
>

Okay.  Done.

> Thanks,
> Tom
>
>> ---
>>  src/gallium/state_trackers/clover/core/event.cpp | 43 +++++++++++++++++-------
>>  src/gallium/state_trackers/clover/core/event.hpp |  3 ++
>>  2 files changed, 34 insertions(+), 12 deletions(-)
>> 
>> diff --git a/src/gallium/state_trackers/clover/core/event.cpp b/src/gallium/state_trackers/clover/core/event.cpp
>> index 5579303..d03e0b4 100644
>> --- a/src/gallium/state_trackers/clover/core/event.cpp
>> +++ b/src/gallium/state_trackers/clover/core/event.cpp
>> @@ -36,28 +36,47 @@ event::event(clover::context &ctx, const ref_vector<event> &deps,
>>  event::~event() {
>>  }
>>  
>> +std::vector<intrusive_ref<event>>
>> +event::trigger_self() {
>> +   std::vector<intrusive_ref<event>> evs;
>> +
>> +   if (!--wait_count)
>> +      std::swap(_chain, evs);
>> +
>> +   return evs;
>> +}
>> +
>>  void
>>  event::trigger() {
>> -   if (!--wait_count) {
>> -      cv.notify_all();
>> -      action_ok(*this);
>> +   auto evs = trigger_self();
>>  
>> -      while (!_chain.empty()) {
>> -         _chain.back()().trigger();
>> -         _chain.pop_back();
>> -      }
>> +   if (signalled()) {
>> +      action_ok(*this);
>> +      cv.notify_all();
>>     }
>> +
>> +   for (event &ev : evs)
>> +      ev.trigger();
>> +}
>> +
>> +std::vector<intrusive_ref<event>>
>> +event::abort_self(cl_int status) {
>> +   std::vector<intrusive_ref<event>> evs;
>> +
>> +   _status = status;
>> +   std::swap(_chain, evs);
>> +
>> +   return evs;
>>  }
>>  
>>  void
>>  event::abort(cl_int status) {
>> -   _status = status;
>> +   auto evs = abort_self(status);
>> +
>>     action_fail(*this);
>>  
>> -   while (!_chain.empty()) {
>> -      _chain.back()().abort(status);
>> -      _chain.pop_back();
>> -   }
>> +   for (event &ev : evs)
>> +      ev.abort(status);
>>  }
>>  
>>  bool
>> diff --git a/src/gallium/state_trackers/clover/core/event.hpp b/src/gallium/state_trackers/clover/core/event.hpp
>> index 0914842..f638c5b 100644
>> --- a/src/gallium/state_trackers/clover/core/event.hpp
>> +++ b/src/gallium/state_trackers/clover/core/event.hpp
>> @@ -84,6 +84,9 @@ namespace clover {
>>        std::vector<intrusive_ref<event>> deps;
>>  
>>     private:
>> +      std::vector<intrusive_ref<event>> trigger_self();
>> +      std::vector<intrusive_ref<event>> abort_self(cl_int status);
>> +
>>        unsigned wait_count;
>>        action action_ok;
>>        action action_fail;
>> -- 
>> 2.3.5
>> 
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150512/58b61caa/attachment.sig>