search for: fence_get

..., fence->ring); > + return false; > + } > + > + fence->fence_wake.flags = 0; > + fence->fence_wake.private = NULL; > + fence->fence_wake.func = radeon_fence_check_signaled; > + __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake); > + fence_get(f); That looks like a race condition to me. The fence needs to be added to the wait queue before the check, not after. Apart from that the whole approach looks like a really bad idea to me. How for example is lockup detection supposed to happen with this? Christian. Am 14.05.2014 16:58, schrie...

...c(sizeof(*shared) * fobj->shared_count, - GFP_KERNEL); - - if (!shared) { - ret = -ENOMEM; - __ttm_bo_unreserve(bo); - spin_unlock(&glob->lru_lock); - return ret; - } - - for (i = 0; i < fobj->shared_count; ++i) { - if (!fence_is_signaled(fobj->shared[i])) { - fence_get(fobj->shared[i]); - shared[shared_count++] = fobj->shared[i]; - } - } - if (!shared_count) { - kfree(shared); - shared = NULL; - } - } - - excl = reservation_object_get_excl(bo->resv); - if (excl && !fence_is_signaled(excl)) - fence_get(excl); - else - excl = NULL; -...

...+ >>> + fence->fence_wake.flags = 0; >>> + fence->fence_wake.private = NULL; >>> + fence->fence_wake.func = radeon_fence_check_signaled; >>> + __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake); >>> + fence_get(f); >> That looks like a race condition to me. The fence needs to be added >> to the wait queue before the check, not after. >> >> Apart from that the whole approach looks like a really bad idea to >> me. How for example is lockup detection supposed to happen with th...

...ct sync_fence, cbs[1]), name); - if (fence == NULL) + sync_fence = sync_fence_alloc(size, name); + if (sync_fence == NULL) return NULL; - fence->num_fences = 1; - atomic_set(&fence->status, 1); + sync_fence->num_fences = num_fences; + atomic_set(&sync_fence->status, 0); - fence_get(&pt->base); - fence->cbs[0].sync_pt = &pt->base; - fence->cbs[0].fence = fence; - if (fence_add_callback(&pt->base, &fence->cbs[0].cb, - fence_check_cb_func)) - atomic_dec(&fence->status); + for (i = 0; i < num_fences; i++) { + struct fence *f...

...>>>>>>>>> + fence->fence_wake.func = radeon_fence_check_signaled; >>>>>>>>> + __add_wait_queue(&fence->rdev->fence_queue, >>>>>>>>> &fence->fence_wake); >>>>>>>>> + fence_get(f); >>>>>>>> That looks like a race condition to me. The fence needs to be >>>>>>>> added to the wait queue before the check, not after. >>>>>>>> >>>>>>>> Apart from that the whole approach looks like a...

...q_put(rdev, fence->ring); > + return false; > + } > + > + fence->fence_wake.flags = 0; > + fence->fence_wake.private = NULL; > + fence->fence_wake.func = radeon_fence_check_signaled; > + __add_wait_queue(&rdev->fence_queue, &fence->fence_wake); > + fence_get(f); > + > + return true; > +} > + > /** > * radeon_fence_signaled - check if a fence has signaled > * > @@ -250,11 +346,13 @@ bool radeon_fence_signaled(struct radeon_fence > *fence) > if (!fence) { > return true; > } > - if (fence->seq == RADEO...

...gt;>>>> + fence->fence_wake.private = NULL; >>>>>>> + fence->fence_wake.func = radeon_fence_check_signaled; >>>>>>> + __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake); >>>>>>> + fence_get(f); >>>>>> That looks like a race condition to me. The fence needs to be >>>>>> added to the wait queue before the check, not after. >>>>>> >>>>>> Apart from that the whole approach looks like a really bad idea >>>&g...

...radeon_irq_kms_sw_irq_put(fence->rdev, fence->ring); + return false; + } + + fence->fence_wake.flags = 0; + fence->fence_wake.private = NULL; + fence->fence_wake.func = radeon_fence_check_signaled; + __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake); + fence_get(f); + + return true; +} + /** * radeon_fence_signaled - check if a fence has signaled * @@ -250,11 +326,13 @@ bool radeon_fence_signaled(struct radeon_fence *fence) if (!fence) { return true; } - if (fence->seq == RADEON_FENCE_SIGNALED_SEQ) { - return true; - } + if (radeon_fence_...

...nce_wake.flags = 0; >>>>> + fence->fence_wake.private = NULL; >>>>> + fence->fence_wake.func = radeon_fence_check_signaled; >>>>> + __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake); >>>>> + fence_get(f); >>>> That looks like a race condition to me. The fence needs to be added >>>> to the wait queue before the check, not after. >>>> >>>> Apart from that the whole approach looks like a really bad idea to >>>> me. How for example is loc...

...>>> + fence->fence_wake.func = radeon_fence_check_signaled; >>>>>>>>>>> + __add_wait_queue(&fence->rdev->fence_queue, >>>>>>>>>>> &fence->fence_wake); >>>>>>>>>>> + fence_get(f); >>>>>>>>>> That looks like a race condition to me. The fence needs to be >>>>>>>>>> added to the wait queue before the check, not after. >>>>>>>>>> >>>>>>>>>> Apart from that...

...fence->seq) { + radeon_irq_kms_sw_irq_put(rdev, fence->ring); + return false; + } + + fence->fence_wake.flags = 0; + fence->fence_wake.private = NULL; + fence->fence_wake.func = radeon_fence_check_signaled; + __add_wait_queue(&rdev->fence_queue, &fence->fence_wake); + fence_get(f); + + return true; +} + /** * radeon_fence_signaled - check if a fence has signaled * @@ -250,11 +346,13 @@ bool radeon_fence_signaled(struct radeon_fence *fence) if (!fence) { return true; } - if (fence->seq == RADEON_FENCE_SIGNALED_SEQ) { - return true; - } + if (radeon_fence_...

...gt; + } >> + >> + fence->fence_wake.flags = 0; >> + fence->fence_wake.private = NULL; >> + fence->fence_wake.func = radeon_fence_check_signaled; >> + __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake); >> + fence_get(f); > That looks like a race condition to me. The fence needs to be added to the wait queue before the check, not after. > > Apart from that the whole approach looks like a really bad idea to me. How for example is lockup detection supposed to happen with this? It's not a race conditi...

...fence->fence_wake.flags = 0; >>>> + fence->fence_wake.private = NULL; >>>> + fence->fence_wake.func = radeon_fence_check_signaled; >>>> + __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake); >>>> + fence_get(f); >>> That looks like a race condition to me. The fence needs to be added to the wait queue before the check, not after. >>> >>> Apart from that the whole approach looks like a really bad idea to me. How for example is lockup detection supposed to happen with this? &gt...

...>fence_wake.func = radeon_fence_check_signaled; >>>>>>>>>>>>> + __add_wait_queue(&fence->rdev->fence_queue, >>>>>>>>>>>>> &fence->fence_wake); >>>>>>>>>>>>> + fence_get(f); >>>>>>>>>>>> That looks like a race condition to me. The fence needs to >>>>>>>>>>>> be added to the wait queue before the check, not after. >>>>>>>>>>>> >>>>>>>>...

op 09-07-14 14:57, Deucher, Alexander schreef: >> <snip> >> +static const char *radeon_fence_get_timeline_name(struct fence *f) >> +{ >> + struct radeon_fence *fence = to_radeon_fence(f); >> + switch (fence->ring) { >> + case RADEON_RING_TYPE_GFX_INDEX: return "radeon.gfx"; >> + case CAYMAN_RING_TYPE_CP1_INDEX: return "radeon.cp1"; >>...

From: Gustavo Padovan <gustavo.padovan at collabora.co.uk> Instead of wrapping virtio_gpu_execbuffer() to execute the ioctl just execute it directly. Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk> --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git

From: Gustavo Padovan <gustavo.padovan at collabora.co.uk> Instead of wrapping virtio_gpu_execbuffer() to execute the ioctl just execute it directly. Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk> --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git

...t;>>>>>>>> + fence->fence_wake.func = radeon_fence_check_signaled; >>>>>>>>>>>>>> + __add_wait_queue(&fence->rdev->fence_queue, &fence->fence_wake); >>>>>>>>>>>>>> + fence_get(f); >>>>>>>>>>>>> That looks like a race condition to me. The fence needs to be added to the wait queue before the check, not after. >>>>>>>>>>>>> >>>>>>>>>>>>> Apart from that the w...

...et = 0; struct dma_buf *dmabuf; + struct fence *fence = NULL; + + if (flags & DRM_SYNC_FD) { +#ifdef CONFIG_SYNC + struct sync_fence *sf = sync_fence_fdget(*prime_fd); + if (!sf) + return -ENOENT; + if (sf->num_fences != 1) { + sync_fence_put(sf); + return -EINVAL; + } + fence = fence_get(sf->cbs[0].sync_pt); + sync_fence_put(sf); + flags &= ~DRM_SYNC_FD; +#else + return -ENODEV; +#endif + } mutex_lock(&file_priv->prime.lock); obj = drm_gem_object_lookup(dev, file_priv, handle); @@ -453,6 +473,14 @@ out_have_obj: goto fail_put_dmabuf; out_have_handle: +...

Am 19.05.2014 15:35, schrieb Maarten Lankhorst: > op 19-05-14 14:30, Christian K?nig schreef: >> Am 19.05.2014 12:10, schrieb Maarten Lankhorst: >>> op 19-05-14 10:27, Christian K?nig schreef: >>>> Am 19.05.2014 10:00, schrieb Maarten Lankhorst: >>>> [SNIP] >>>> The problem here is that the whole approach collides with the way >>>>