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

[Tom Stellard]

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>

CC: stable should also be added to these.

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