[PATCH] dma-buf/sync-file: Avoid enable fence signaling if poll(.timeout=0)

[Rafael Antognolli]

Hi Chris and Gustavo,

On Mon, Aug 29, 2016 at 07:16:13PM +0100, Chris Wilson wrote:
> If we being polled with a timeout of zero, a nonblocking busy query,
> we don't need to install any fence callbacks as we will not be waiting.
> As we only install the callback once, the overhead comes from the atomic
> bit test that also causes serialisation between threads.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Sumit Semwal <sumit.semwal at linaro.org>
> Cc: Gustavo Padovan <gustavo at padovan.org>
> Cc: linux-media at vger.kernel.org
> Cc: dri-devel at lists.freedesktop.org
> Cc: linaro-mm-sig at lists.linaro.org
> ---
>  drivers/dma-buf/sync_file.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 486d29c1a830..abb5fdab75fd 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -306,7 +306,8 @@ static unsigned int sync_file_poll(struct file *file, poll_table *wait)
>  
>  	poll_wait(file, &sync_file->wq, wait);
>  
> -	if (!test_and_set_bit(POLL_ENABLED, &sync_file->fence->flags)) {
> +	if (!poll_does_not_wait(wait) &&
> +	    !test_and_set_bit(POLL_ENABLED, &sync_file->fence->flags)) {
>  		if (fence_add_callback(sync_file->fence, &sync_file->cb,
>  				       fence_check_cb_func) < 0)
>  			wake_up_all(&sync_file->wq);

This commit is causing an error on one of the tests that Robert Foss
submitted for i-g-t. The one that does random merge of fences from
different timelines. A simple version of the test that still triggers
this is:

static void test_sync_simple_merge(void)
{
        int fence1, fence2, fence_merge, timeline1, timeline2;
        int ret;

        timeline1 = sw_sync_timeline_create();
        timeline2 = sw_sync_timeline_create();
        fence1 = sw_sync_fence_create(timeline1, 1);
        fence2 = sw_sync_fence_create(timeline2, 2);
        fence_merge = sw_sync_merge(fence1, fence2);
        sw_sync_timeline_inc(timeline1, 5);
        sw_sync_timeline_inc(timeline2, 5);

        ret = sw_sync_wait(fence_merge, 0);
        igt_assert_f(ret > 0, "Failure triggering fence\n");

        sw_sync_fence_destroy(fence_merge);
        sw_sync_fence_destroy(fence1);
        sw_sync_fence_destroy(fence2);
        sw_sync_timeline_destroy(timeline1);
        sw_sync_timeline_destroy(timeline2);
}

It looks like you cannot trust fence_is_signaled() without a
fence_add_callback(). I think the fence_array->num_pending won't get
updated. Although I couldn't figure out why it only happens if you merge
fences from different timelines.

Regards,
Rafael