Re: [PATCH 1/3] drm/virtio: set seqno for dma-fence

(Add missing CCs)

On Mon, Apr 29, 2019 at 3:08 PM Chia-I Wu olvaffe@gmail.com wrote:

This is motivated by having meaningful ftrace events, but it also fixes use cases where dma_fence_is_later is called, such as in sync_file_merge.

In other drivers, fence creation and cmdbuf submission normally happen atomically,

mutex_lock(); fence = dma_fence_create(..., ++timeline->seqno); submit_cmdbuf(); mutex_unlock();

and have no such issue. But in our driver, because most ioctls queue commands into ctrlq, we do not want to grab a lock. Instead, we set seqno to 0 when a fence is created, and update it when the command is finally queued and the seqno is known.

Signed-off-by: Chia-I Wu olvaffe@gmail.com

drivers/gpu/drm/virtio/virtgpu_drv.h | 1 - drivers/gpu/drm/virtio/virtgpu_fence.c | 17 ++++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 491dec0712b3..90461feeafdb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -102,7 +102,6 @@ struct virtio_gpu_fence { struct dma_fence f; struct virtio_gpu_fence_driver *drv; struct list_head node;

•   uint64_t seq;
  

}; #define to_virtio_fence(x) \ container_of(x, struct virtio_gpu_fence, f) diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index 87d1966192f4..72b4f7561432 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -40,16 +40,14 @@ bool virtio_fence_signaled(struct dma_fence *f) { struct virtio_gpu_fence *fence = to_virtio_fence(f);

•   if (atomic64_read(&fence->drv->last_seq) >= fence->seq)
  

•   if (atomic64_read(&fence->drv->last_seq) >= fence->f.seqno)
            return true;
    return false;
  

}

static void virtio_fence_value_str(struct dma_fence *f, char *str, int size) {

•   struct virtio_gpu_fence *fence = to_virtio_fence(f);
  

•   snprintf(str, size, "%llu", fence->seq);
  

•   snprintf(str, size, "%llu", f->seqno);
  

}

static void virtio_timeline_value_str(struct dma_fence *f, char *str, int size) @@ -76,6 +74,11 @@ struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev) return fence;

    fence->drv = drv;

•   /* This only partially initializes the fence because the seqno is
  

•    * unknown yet.  The fence must not be used outside of the driver
  

•    * until virtio_gpu_fence_emit is called.
  

•    */
    dma_fence_init(&fence->f, &virtio_fence_ops, &drv->lock, drv->context, 0);
  
    return fence;
  

@@ -89,13 +92,13 @@ int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, unsigned long irq_flags;

    spin_lock_irqsave(&drv->lock, irq_flags);

•   fence->seq = ++drv->sync_seq;
  

•   fence->f.seqno = ++drv->sync_seq;
    dma_fence_get(&fence->f);
    list_add_tail(&fence->node, &drv->fences);
    spin_unlock_irqrestore(&drv->lock, irq_flags);
  
    cmd_hdr->flags |= cpu_to_le32(VIRTIO_GPU_FLAG_FENCE);
  

•   cmd_hdr->fence_id = cpu_to_le64(fence->seq);
  

•   cmd_hdr->fence_id = cpu_to_le64(fence->f.seqno);
    return 0;
  

}

@@ -109,7 +112,7 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev, spin_lock_irqsave(&drv->lock, irq_flags); atomic64_set(&vgdev->fence_drv.last_seq, last_seq); list_for_each_entry_safe(fence, tmp, &drv->fences, node) {

•           if (last_seq < fence->seq)
  

•           if (last_seq < fence->f.seqno)
                    continue;
            dma_fence_signal_locked(&fence->f);
            list_del(&fence->node);
  

-- 2.21.0.593.g511ec345e18-goog