search for: radeon_fence_check_signaled

...ence->rdev->fence_drv[fence->ring].last_seq) >= fence->seq) { > + 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); 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....

...;>> + 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); >> 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 tha...

...ing, > seq); > + (*fence)->rdev = rdev; > + (*fence)->seq = seq; > radeon_fence_ring_emit(rdev, ring, *fence); > trace_radeon_fence_emit(rdev->ddev, ring, (*fence)->seq); > return 0; > } > > /** > - * radeon_fence_process - process a fence > + * radeon_fence_check_signaled - callback from fence_queue > * > - * @rdev: radeon_device pointer > - * @ring: ring index the fence is associated with > - * > - * Checks the current fence value and wakes the fence queue > - * if the sequence number has increased (all asics). > + * this function is called w...

...;rdev->fence_queue.lock, rdev->fence_context + ring, seq); + (*fence)->rdev = rdev; + (*fence)->seq = seq; radeon_fence_ring_emit(rdev, ring, *fence); trace_radeon_fence_emit(rdev->ddev, ring, (*fence)->seq); return 0; } /** - * radeon_fence_process - process a fence + * radeon_fence_check_signaled - callback from fence_queue * - * @rdev: radeon_device pointer - * @ring: ring index the fence is associated with - * - * Checks the current fence value and wakes the fence queue - * if the sequence number has increased (all asics). + * this function is called with fence_queue lock held, which is...

...;rdev->fence_queue.lock, rdev->fence_context + ring, seq); + (*fence)->rdev = rdev; + (*fence)->seq = seq; radeon_fence_ring_emit(rdev, ring, *fence); trace_radeon_fence_emit(rdev->ddev, ring, (*fence)->seq); return 0; } /** - * radeon_fence_process - process a fence + * radeon_fence_check_signaled - callback from fence_queue * - * @rdev: radeon_device pointer - * @ring: ring index the fence is associated with - * - * Checks the current fence value and wakes the fence queue - * if the sequence number has increased (all asics). + * this function is called with fence_queue lock held, which is...

...;rdev->fence_queue.lock, rdev->fence_context + ring, seq); + (*fence)->rdev = rdev; + (*fence)->seq = seq; radeon_fence_ring_emit(rdev, ring, *fence); trace_radeon_fence_emit(rdev->ddev, ring, (*fence)->seq); return 0; } /** - * radeon_fence_process - process a fence + * radeon_fence_check_signaled - callback from fence_queue * - * @rdev: radeon_device pointer - * @ring: ring index the fence is associated with - * - * Checks the current fence value and wakes the fence queue - * if the sequence number has increased (all asics). + * this function is called with fence_queue lock held, which is...

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 >>>>

...t;>>>>>> + >>>>>>>>>>>>> + 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); >>>>>>>>&gt...

...t;>>>>>>> + } >>>>>>>>> + >>>>>>>>> + 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...

...(*fence)->seq = seq; radeon_fence_ring_emit(rdev, ring, *fence); trace_radeon_fence_emit(rdev->ddev, ring, (*fence)->seq); return 0; } /** - * radeon_fence_process - process a fence - * - * @rdev: radeon_device pointer - * @ring: ring index the fence is associated with + * radeon_fence_check_signaled - callback from fence_queue * - * Checks the current fence value and wakes the fence queue - * if the sequence number has increased (all asics). + * this function is called with fence_queue lock held, which is also used + * for the fence locking itself, so unlocked variants are used for + * fenc...

...t(rdev, ring, *fence); > trace_radeon_fence_emit(rdev->ddev, ring, (*fence)->seq); > return 0; > } > > /** > - * radeon_fence_process - process a fence > - * > - * @rdev: radeon_device pointer > - * @ring: ring index the fence is associated with > + * radeon_fence_check_signaled - callback from fence_queue > * > - * Checks the current fence value and wakes the fence queue > - * if the sequence number has increased (all asics). > + * this function is called with fence_queue lock held, which is also > used > + * for the fence locking itself, so unlocked...

...t;>>> + >>>>>>>>>>>>>> + 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 condi...

...t;>> + 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...

...ce->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); >>>> That looks like a race condition to me. The fence needs to be added >>>> to the wait queue before the check, not after....

...} >>>>>>>>>>> + >>>>>>>>>>> + 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...

...) >= fence->seq) { >> + 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); > 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 lik...

...n_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); >>> That looks like a race condition to me. The fence needs to be added to the wait queue before the check, not after. >>> >>> Apar...

...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 che...

...t(rdev, ring, *fence); > trace_radeon_fence_emit(rdev->ddev, ring, (*fence)->seq); > return 0; > } > > /** > - * radeon_fence_process - process a fence > - * > - * @rdev: radeon_device pointer > - * @ring: ring index the fence is associated with > + * radeon_fence_check_signaled - callback from fence_queue > * > - * Checks the current fence value and wakes the fence queue > - * if the sequence number has increased (all asics). > + * this function is called with fence_queue lock held, which is also > used > + * for the fence locking itself, so unlocked...

...t;>>> + } >>>>>>>>>> + >>>>>>>>>> + 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 w...