From 69e032653df5aae335764f6346703a1e55c96a2d Mon Sep 17 00:00:00 2001 From: Sami Kiminki Date: Tue, 14 Nov 2017 15:56:09 +0200 Subject: gpu: nvgpu: Add synchronization to comptag alloc and clearing Comptags allocation and clearing was not synchronized for a buffer. Fix this race by serializing the operations with the gk20a_dmabuf_priv lock. While doing that, add an error check in the cbc_ctrl call. Bug 1902982 Change-Id: Icd96f1855eb5e5340651bcc85849b5ccc199b821 Signed-off-by: Sami Kiminki Reviewed-on: https://git-master.nvidia.com/r/1597904 Reviewed-by: Alex Waterman Reviewed-by: Terje Bergstrom Reviewed-by: mobile promotions Tested-by: mobile promotions --- drivers/gpu/nvgpu/common/linux/comptags.c | 50 +++++++++++++++++++++++++----- drivers/gpu/nvgpu/common/mm/vm.c | 27 ++++++++++------ drivers/gpu/nvgpu/include/nvgpu/comptags.h | 20 +++++++++++- 3 files changed, 80 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/nvgpu/common/linux/comptags.c b/drivers/gpu/nvgpu/common/linux/comptags.c index 7d095793..353f6363 100644 --- a/drivers/gpu/nvgpu/common/linux/comptags.c +++ b/drivers/gpu/nvgpu/common/linux/comptags.c @@ -37,7 +37,9 @@ void gk20a_get_comptags(struct nvgpu_os_buffer *buf, return; } + nvgpu_mutex_acquire(&priv->lock); *comptags = priv->comptags; + nvgpu_mutex_release(&priv->lock); } int gk20a_alloc_or_get_comptags(struct gk20a *g, @@ -55,20 +57,26 @@ int gk20a_alloc_or_get_comptags(struct gk20a *g, if (!priv) return -ENOSYS; + nvgpu_mutex_acquire(&priv->lock); + if (priv->comptags.allocated) { /* * already allocated */ *comptags = priv->comptags; - return 0; + + err = 0; + goto exit_locked; } ctag_granularity = g->ops.fb.compression_page_size(g); lines = DIV_ROUND_UP_ULL(buf->dmabuf->size, ctag_granularity); /* 0-sized buffer? Shouldn't occur, but let's check anyways. */ - if (lines < 1) - return -EINVAL; + if (lines < 1) { + err = -EINVAL; + goto exit_locked; + } /* store the allocator so we can use it when we free the ctags */ priv->comptag_allocator = allocator; @@ -89,16 +97,44 @@ int gk20a_alloc_or_get_comptags(struct gk20a *g, * would not be safe to re-allocate comptags anyways on * successive calls, as that would break map aliasing. */ + err = 0; priv->comptags.allocated = true; *comptags = priv->comptags; - return 0; + +exit_locked: + nvgpu_mutex_release(&priv->lock); + + return err; } -void gk20a_mark_comptags_cleared(struct nvgpu_os_buffer *buf) +bool gk20a_comptags_start_clear(struct nvgpu_os_buffer *buf) { struct gk20a_dmabuf_priv *priv = dma_buf_get_drvdata(buf->dmabuf, buf->dev); - if (priv) - priv->comptags.needs_clear = false; + bool clear_started = false; + + if (priv) { + nvgpu_mutex_acquire(&priv->lock); + + clear_started = priv->comptags.needs_clear; + + if (!clear_started) + nvgpu_mutex_release(&priv->lock); + } + + return clear_started; +} + +void gk20a_comptags_finish_clear(struct nvgpu_os_buffer *buf, + bool clear_successful) +{ + struct gk20a_dmabuf_priv *priv = dma_buf_get_drvdata(buf->dmabuf, + buf->dev); + if (priv) { + if (clear_successful) + priv->comptags.needs_clear = false; + + nvgpu_mutex_release(&priv->lock); + } } diff --git a/drivers/gpu/nvgpu/common/mm/vm.c b/drivers/gpu/nvgpu/common/mm/vm.c index c3f6b79d..ebe8e381 100644 --- a/drivers/gpu/nvgpu/common/mm/vm.c +++ b/drivers/gpu/nvgpu/common/mm/vm.c @@ -874,12 +874,17 @@ struct nvgpu_mapped_buf *nvgpu_vm_map(struct vm_gk20a *vm, */ if (comptags.needs_clear) { if (g->ops.ltc.cbc_ctrl) { - g->ops.ltc.cbc_ctrl( - g, gk20a_cbc_op_clear, - comptags.offset, - (comptags.offset + - comptags.lines - 1)); - gk20a_mark_comptags_cleared(os_buf); + if (gk20a_comptags_start_clear(os_buf)) { + err = g->ops.ltc.cbc_ctrl( + g, gk20a_cbc_op_clear, + comptags.offset, + (comptags.offset + + comptags.lines - 1)); + gk20a_comptags_finish_clear( + os_buf, err == 0); + if (err) + goto clean_up; + } } else { /* * Cleared as part of gmmu map @@ -920,6 +925,9 @@ struct nvgpu_mapped_buf *nvgpu_vm_map(struct vm_gk20a *vm, goto clean_up; } + if (clear_ctags) + clear_ctags = gk20a_comptags_start_clear(os_buf); + map_addr = g->ops.mm.gmmu_map(vm, map_addr, sgt, @@ -935,14 +943,15 @@ struct nvgpu_mapped_buf *nvgpu_vm_map(struct vm_gk20a *vm, false, batch, aperture); + + if (clear_ctags) + gk20a_comptags_finish_clear(os_buf, map_addr != 0); + if (!map_addr) { err = -ENOMEM; goto clean_up; } - if (clear_ctags) - gk20a_mark_comptags_cleared(os_buf); - nvgpu_init_list_node(&mapped_buffer->buffer_list); nvgpu_ref_init(&mapped_buffer->ref); mapped_buffer->addr = map_addr; diff --git a/drivers/gpu/nvgpu/include/nvgpu/comptags.h b/drivers/gpu/nvgpu/include/nvgpu/comptags.h index 99e2d657..679a0f9e 100644 --- a/drivers/gpu/nvgpu/include/nvgpu/comptags.h +++ b/drivers/gpu/nvgpu/include/nvgpu/comptags.h @@ -74,6 +74,24 @@ int gk20a_alloc_or_get_comptags(struct gk20a *g, struct gk20a_comptags *comptags); void gk20a_get_comptags(struct nvgpu_os_buffer *buf, struct gk20a_comptags *comptags); -void gk20a_mark_comptags_cleared(struct nvgpu_os_buffer *buf); + +/* + * These functions must be used to synchronize comptags clear. The usage: + * + * if (gk20a_comptags_start_clear(os_buf)) { + * // we now hold the buffer lock for clearing + * + * bool successful = hw_clear_comptags(); + * + * // mark the buf cleared (or not) and release the buffer lock + * gk20a_comptags_finish_clear(os_buf, successful); + * } + * + * If gk20a_start_comptags_clear() returns false, another caller has + * already cleared the comptags. + */ +bool gk20a_comptags_start_clear(struct nvgpu_os_buffer *buf); +void gk20a_comptags_finish_clear(struct nvgpu_os_buffer *buf, + bool clear_successful); #endif -- cgit v1.2.2