[Mesa-dev] [PATCH 2/5] clover: Replace open-coded event::signalled()

[Francisco Jerez]

Hey Tom,

Tom Stellard <thomas.stellard at amd.com> writes:

> This consolidates signalled checks into the same place.
> ---
>  src/gallium/state_trackers/clover/core/event.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/clover/core/event.cpp b/src/gallium/state_trackers/clover/core/event.cpp
> index 58de888..3c9336e 100644
> --- a/src/gallium/state_trackers/clover/core/event.cpp
> +++ b/src/gallium/state_trackers/clover/core/event.cpp
> @@ -66,7 +66,7 @@ event::signalled() const {
>  
>  void
>  event::chain(event &ev) {
> -   if (wait_count) {
> +   if (!signalled()) {

The problem with this is that signalled() is part of the public API of
this class, so it should implement locking to access the concurrently
modified method "wait_count" (you don't seem to have changed that in
this series).  chain() should also lock, as you do in PATCH 5, so this
will lead to a deadlock (unless the mutex is recursive but I doubt this
is enough reason to use a recursive mutex).

>        ev.wait_count++;
>        _chain.push_back(ev);
>     }
> -- 
> 2.0.4
-------------- 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/20150509/9cffdd9a/attachment-0001.sig>