czw., 25 kwi 2019 o 20:11 Gustaw Smolarczyk <wielkie...@gmail.com> napisał(a):
>
> czw., 25 kwi 2019 o 19:42 Emil Velikov <emil.l.veli...@gmail.com> napisał(a):
> >
> > The function is analogous to lp_fence_wait() while taking at timeout
> > (ns) parameter, as needed for EGL fence/sync.
> >
> > v2:
> > - use absolute UTC time, as per spec (Gustaw)
> > - bail out on cnd_timedwait() failure (Gustaw)
> >
> > Cc: Gustaw Smolarczyk <wielkie...@gmail.com>
> > Cc: Roland Scheidegger <srol...@vmware.com>
> > Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
> > Reviewed-by: Roland Scheidegger <srol...@vmware.com> (v1)
> > ---
> > src/gallium/drivers/llvmpipe/lp_fence.c | 30 +++++++++++++++++++++++++
> > src/gallium/drivers/llvmpipe/lp_fence.h | 3 +++
> > 2 files changed, 33 insertions(+)
> >
> > diff --git a/src/gallium/drivers/llvmpipe/lp_fence.c
> > b/src/gallium/drivers/llvmpipe/lp_fence.c
> > index 20cd91cd63d..b79d773bf6c 100644
> > --- a/src/gallium/drivers/llvmpipe/lp_fence.c
> > +++ b/src/gallium/drivers/llvmpipe/lp_fence.c
> > @@ -125,3 +125,33 @@ lp_fence_wait(struct lp_fence *f)
> > }
> >
> >
> > +boolean
> > +lp_fence_timedwait(struct lp_fence *f, uint64_t timeout)
> > +{
> > + struct timespec ts;
> > + int ret;
> > +
> > + timespec_get(&ts, TIME_UTC);
> > +
> > + ts.tv_nsec += timeout % 1000000000L;
> > + ts.tv_sec += timeout / 1000000000L;
> > + if (ts.tv_nsec >= 1000000000L) {
> > + ts.tv_sec++;
> > + ts.tv_nsec -= 1000000000L;
> > + }
> > +
> > + if (LP_DEBUG & DEBUG_FENCE)
> > + debug_printf("%s %d\n", __FUNCTION__, f->id);
> > +
> > + mtx_lock(&f->mutex);
> > + assert(f->issued);
> > + while (f->count < f->rank) {
> > + ret = cnd_timedwait(&f->signalled, &f->mutex, &ts);
> > + if (ret != thrd_success)
> > + break;
> > + }
> > + mtx_unlock(&f->mutex);
> > + return (f->count >= f->rank && ret == thrd_success);
Hmm, you are reading from the fence object outside of the critical
section, which doesn't sound safe. Maybe compute the return value
before the mutex is unlocked?
const boolean result = (f->count >= f->rank);
mtx_unlock(&f->mutex);
return result;
Since f->rank is immutable and f->count never decreases, it might
still be ok without this change, though it is racy.
>
> Is checking for ret == thrd_success here really necessary? If the
> first part is true we already know that the fence has been signalled.
>
> With this changed or not:
>
> Reviewed-by: Gustaw Smolarczyk <wielkie...@gmail.com>
>
> > +}
> > +
> > +
> > diff --git a/src/gallium/drivers/llvmpipe/lp_fence.h
> > b/src/gallium/drivers/llvmpipe/lp_fence.h
> > index b72026492c6..5ba746d22d1 100644
> > --- a/src/gallium/drivers/llvmpipe/lp_fence.h
> > +++ b/src/gallium/drivers/llvmpipe/lp_fence.h
> > @@ -65,6 +65,9 @@ lp_fence_signalled(struct lp_fence *fence);
> > void
> > lp_fence_wait(struct lp_fence *fence);
> >
> > +boolean
> > +lp_fence_timedwait(struct lp_fence *fence, uint64_t timeout);
> > +
> > void
> > llvmpipe_init_screen_fence_funcs(struct pipe_screen *screen);
> >
> > --
> > 2.20.1
> >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
