| Message ID | 20240801175548.17185-1-zack.rusin@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/vmwgfx: Prevent unmapping active read buffers | expand |
Remove `busy_places` now that it's unused. There's also probably a better place to put `map_count` in the struct layout to avoid false sharing with `cpu_writers`. I'd repack the whole struct if we're going to be adding and removing fields.
On Tue, Aug 13, 2024 at 1:29 PM Ian Forbes <ian.forbes@broadcom.com> wrote: > > Remove `busy_places` now that it's unused. There's also probably a > better place to put `map_count` in the struct layout to avoid false > sharing with `cpu_writers`. I'd repack the whole struct if we're going > to be adding and removing fields. Those are not related to this change. They'd be two seperate changes. One to remove other unused members and third to relayout the struct. z
In that case move `map_count` above `map` which should move it to a separate cache line and update the doc strings as needed. Reviewed-by: Ian Forbes <ian.forbes@broadcom.com> On Tue, Aug 13, 2024 at 12:40 PM Zack Rusin <zack.rusin@broadcom.com> wrote: > > On Tue, Aug 13, 2024 at 1:29 PM Ian Forbes <ian.forbes@broadcom.com> wrote: > > > > Remove `busy_places` now that it's unused. There's also probably a > > better place to put `map_count` in the struct layout to avoid false > > sharing with `cpu_writers`. I'd repack the whole struct if we're going > > to be adding and removing fields. > > Those are not related to this change. They'd be two seperate changes. > One to remove other unused members and third to relayout the struct. > > z
On Tue, Aug 13, 2024 at 1:56 PM Ian Forbes <ian.forbes@broadcom.com> wrote: > > In that case move `map_count` above `map` which should move it to a > separate cache line and update the doc strings as needed. Sorry, I'm not sure I understand. What are you trying to fix? z
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index f42ebc4a7c22..a0e433fbcba6 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -360,6 +360,8 @@ void *vmw_bo_map_and_cache_size(struct vmw_bo *vbo, size_t size) void *virtual; int ret; + atomic_inc(&vbo->map_count); + virtual = ttm_kmap_obj_virtual(&vbo->map, ¬_used); if (virtual) return virtual; @@ -383,11 +385,17 @@ void *vmw_bo_map_and_cache_size(struct vmw_bo *vbo, size_t size) */ void vmw_bo_unmap(struct vmw_bo *vbo) { + int map_count; + if (vbo->map.bo == NULL) return; - ttm_bo_kunmap(&vbo->map); - vbo->map.bo = NULL; + map_count = atomic_dec_return(&vbo->map_count); + + if (!map_count) { + ttm_bo_kunmap(&vbo->map); + vbo->map.bo = NULL; + } } @@ -421,6 +429,7 @@ static int vmw_bo_init(struct vmw_private *dev_priv, vmw_bo->tbo.priority = 3; vmw_bo->res_tree = RB_ROOT; xa_init(&vmw_bo->detached_resources); + atomic_set(&vmw_bo->map_count, 0); params->size = ALIGN(params->size, PAGE_SIZE); drm_gem_private_object_init(vdev, &vmw_bo->tbo.base, params->size); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h index 62b4342d5f7c..dc13f1e996c1 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h @@ -90,6 +90,7 @@ struct vmw_bo { u32 res_prios[TTM_MAX_BO_PRIORITY]; struct xarray detached_resources; + atomic_t map_count; atomic_t cpu_writers; /* Not ref-counted. Protected by binding_mutex */ struct vmw_resource *dx_query_ctx;
The kms paths keep a persistent map active to read and compare the cursor buffer. These maps can race with each other in simple scenario where: a) buffer "a" mapped for update b) buffer "a" mapped for compare c) do the compare d) unmap "a" for compare e) update the cursor f) unmap "a" for update At step "e" the buffer has been unmapped and the read contents is bogus. Prevent unmapping of active read buffers by simply keeping a count of how many paths have currently active maps and unmap only when the count reaches 0. Fixes: 485d98d472d5 ("drm/vmwgfx: Add support for CursorMob and CursorBypass 4") Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com> Cc: dri-devel@lists.freedesktop.org Cc: <stable@vger.kernel.org> # v5.19+ Signed-off-by: Zack Rusin <zack.rusin@broadcom.com> --- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 13 +++++++++++-- drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 1 + 2 files changed, 12 insertions(+), 2 deletions(-)