This hashtable is only used under a lock in vmw_execbuf_process and needs to be cleared before vmw_execbuf_process returns otherwise bad things happen because the nodes that are stored in the table come from an arena allocator that is cleared at the end of the function.
Rather than wasting time cleaning up the hashtable move it onto the stack so we don't have to do any cleanup. Signed-off-by: Ian Forbes <[email protected]> --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 9 ----- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 3 -- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 3 -- drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 43 +++------------------- drivers/gpu/drm/vmwgfx/vmwgfx_validation.h | 5 ++- 5 files changed, 9 insertions(+), 54 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 8ff958d119be..4b53d751b0e0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -814,13 +814,6 @@ static void vmw_write_driver_id(struct vmw_private *dev) } } -static void vmw_sw_context_init(struct vmw_private *dev_priv) -{ - struct vmw_sw_context *sw_context = &dev_priv->ctx; - - hash_init(sw_context->res_ht); -} - static void vmw_sw_context_fini(struct vmw_private *dev_priv) { struct vmw_sw_context *sw_context = &dev_priv->ctx; @@ -836,8 +829,6 @@ static int vmw_driver_load(struct vmw_private *dev_priv, u32 pci_id) enum vmw_res_type i; bool refuse_dma = false; - vmw_sw_context_init(dev_priv); - mutex_init(&dev_priv->cmdbuf_mutex); mutex_init(&dev_priv->binding_mutex); spin_lock_init(&dev_priv->resource_lock); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index eda5b6f8f4c4..e8004a0a68c9 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -78,7 +78,6 @@ #define VMW_RES_STREAM ttm_driver_type2 #define VMW_RES_FENCE ttm_driver_type3 #define VMW_RES_SHADER ttm_driver_type4 -#define VMW_RES_HT_ORDER 12 #define MKSSTAT_CAPACITY_LOG2 5U #define MKSSTAT_CAPACITY (1U << MKSSTAT_CAPACITY_LOG2) @@ -347,7 +346,6 @@ struct vmw_ctx_validation_info; /** * struct vmw_sw_context - Command submission context - * @res_ht: Pointer hash table used to find validation duplicates * @kernel: Whether the command buffer originates from kernel code rather * than from user-space * @fp: If @kernel is false, points to the file of the client. Otherwise @@ -377,7 +375,6 @@ struct vmw_ctx_validation_info; * @ctx: The validation context */ struct vmw_sw_context{ - DECLARE_HASHTABLE(res_ht, VMW_RES_HT_ORDER); bool kernel; struct vmw_fpriv *fp; struct drm_file *filp; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c index 819704ac675d..6a7a9fc13a7f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c @@ -4172,8 +4172,6 @@ int vmw_execbuf_process(struct drm_file *file_priv, if (unlikely(ret != 0)) goto out_err; - vmw_validation_drop_ht(&val_ctx); - ret = mutex_lock_interruptible(&dev_priv->binding_mutex); if (unlikely(ret != 0)) { ret = -ERESTARTSYS; @@ -4282,7 +4280,6 @@ int vmw_execbuf_process(struct drm_file *file_priv, __vmw_execbuf_release_pinned_bo(dev_priv, NULL); out_unlock: vmw_cmdbuf_res_revert(&sw_context->staged_cmd_res); - vmw_validation_drop_ht(&val_ctx); WARN_ON(!list_empty(&sw_context->ctx_list)); mutex_unlock(&dev_priv->cmdbuf_mutex); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c index 35dc94c3db39..1da7dbef905f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c @@ -166,7 +166,7 @@ vmw_validation_find_bo_dup(struct vmw_validation_context *ctx, struct vmwgfx_hash_item *hash; unsigned long key = (unsigned long) vbo; - hash_for_each_possible_rcu(ctx->sw_context->res_ht, hash, head, key) { + hash_for_each_possible(ctx->res_ht, hash, head, key) { if (hash->key == key) { bo_node = container_of(hash, typeof(*bo_node), hash); break; @@ -208,7 +208,7 @@ vmw_validation_find_res_dup(struct vmw_validation_context *ctx, struct vmwgfx_hash_item *hash; unsigned long key = (unsigned long) res; - hash_for_each_possible_rcu(ctx->sw_context->res_ht, hash, head, key) { + hash_for_each_possible(ctx->res_ht, hash, head, key) { if (hash->key == key) { res_node = container_of(hash, typeof(*res_node), hash); break; @@ -258,8 +258,7 @@ int vmw_validation_add_bo(struct vmw_validation_context *ctx, if (ctx->sw_context) { bo_node->hash.key = (unsigned long) vbo; - hash_add_rcu(ctx->sw_context->res_ht, &bo_node->hash.head, - bo_node->hash.key); + hash_add(ctx->res_ht, &bo_node->hash.head, bo_node->hash.key); } val_buf = &bo_node->base; vmw_bo_reference(vbo); @@ -305,11 +304,11 @@ int vmw_validation_add_resource(struct vmw_validation_context *ctx, if (ctx->sw_context) { node->hash.key = (unsigned long) res; - hash_add_rcu(ctx->sw_context->res_ht, &node->hash.head, node->hash.key); + hash_add(ctx->res_ht, &node->hash.head, node->hash.key); } node->res = vmw_resource_reference_unless_doomed(res); if (!node->res) { - hash_del_rcu(&node->hash.head); + hash_del(&node->hash.head); return -ESRCH; } @@ -611,38 +610,6 @@ int vmw_validation_res_validate(struct vmw_validation_context *ctx, bool intr) return 0; } -/** - * vmw_validation_drop_ht - Reset the hash table used for duplicate finding - * and unregister it from this validation context. - * @ctx: The validation context. - * - * The hash table used for duplicate finding is an expensive resource and - * may be protected by mutexes that may cause deadlocks during resource - * unreferencing if held. After resource- and buffer object registering, - * there is no longer any use for this hash table, so allow freeing it - * either to shorten any mutex locking time, or before resources- and - * buffer objects are freed during validation context cleanup. - */ -void vmw_validation_drop_ht(struct vmw_validation_context *ctx) -{ - struct vmw_validation_bo_node *entry; - struct vmw_validation_res_node *val; - - if (!ctx->sw_context) - return; - - list_for_each_entry(entry, &ctx->bo_list, base.head) - hash_del_rcu(&entry->hash.head); - - list_for_each_entry(val, &ctx->resource_list, head) - hash_del_rcu(&val->hash.head); - - list_for_each_entry(val, &ctx->resource_ctx_list, head) - hash_del_rcu(&val->hash.head); - - ctx->sw_context = NULL; -} - /** * vmw_validation_unref_lists - Unregister previously registered buffer * object and resources. diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h index 353d837907d8..2b82a1a3110d 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h @@ -37,10 +37,11 @@ #define VMW_RES_DIRTY_NONE 0 #define VMW_RES_DIRTY_SET BIT(0) #define VMW_RES_DIRTY_CLEAR BIT(1) +#define VMW_RES_HT_ORDER 7 /** * struct vmw_validation_context - Per command submission validation context - * @ht: Hash table used to find resource- or buffer object duplicates + * @res_ht: Hash table used to find resource- or buffer object duplicates * @resource_list: List head for resource validation metadata * @resource_ctx_list: List head for resource validation metadata for * resources that need to be validated before those in @resource_list @@ -55,6 +56,7 @@ */ struct vmw_validation_context { struct vmw_sw_context *sw_context; + DECLARE_HASHTABLE(res_ht, VMW_RES_HT_ORDER); struct list_head resource_list; struct list_head resource_ctx_list; struct list_head bo_list; @@ -84,6 +86,7 @@ struct vmw_fence_obj; #define DECLARE_VAL_CONTEXT(_name, _sw_context, _merge_dups) \ struct vmw_validation_context _name = \ { .sw_context = _sw_context, \ + .res_ht = {}, \ .resource_list = LIST_HEAD_INIT((_name).resource_list), \ .resource_ctx_list = LIST_HEAD_INIT((_name).resource_ctx_list), \ .bo_list = LIST_HEAD_INIT((_name).bo_list), \ -- 2.51.0
