drm/i915/selftests: Mark up sole accessor to ctx->vm as being protected
diff mbox series

Message ID 20191107221201.30497-1-chris@chris-wilson.co.uk
State New
Headers show
Series
  • drm/i915/selftests: Mark up sole accessor to ctx->vm as being protected
Related show

Commit Message

Chris Wilson Nov. 7, 2019, 10:12 p.m. UTC
In the selftests, where we are accessing a private ctx from within the
confines of a single test, we know that the ctx->vm pointer is static
and bounded by the lifetime of the test. We can use a simple helper to
provide the RCU annotations to keep sparse happy.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 .../drm/i915/gem/selftests/i915_gem_context.c | 24 +++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

Comments

Tvrtko Ursulin Nov. 8, 2019, 10:40 a.m. UTC | #1
On 07/11/2019 22:12, Chris Wilson wrote:
> In the selftests, where we are accessing a private ctx from within the
> confines of a single test, we know that the ctx->vm pointer is static
> and bounded by the lifetime of the test. We can use a simple helper to
> provide the RCU annotations to keep sparse happy.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   .../drm/i915/gem/selftests/i915_gem_context.c | 24 +++++++++++--------
>   1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index 0d4cdf38d5f9..13ae8f66bed6 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -26,6 +26,12 @@
>   
>   #define DW_PER_PAGE (PAGE_SIZE / sizeof(u32))
>   
> +static inline struct i915_address_space *ctx_vm(struct i915_gem_context *ctx)
> +{
> +	/* single threaded, private ctx */
> +	return rcu_dereference_protected(ctx->vm, true);
> +}
> +
>   static int live_nop_switch(void *arg)
>   {
>   	const unsigned int nctx = 1024;
> @@ -787,14 +793,15 @@ static int igt_shared_ctx_exec(void *arg)
>   			}
>   
>   			mutex_lock(&ctx->mutex);
> -			__assign_ppgtt(ctx, parent->vm);
> +			__assign_ppgtt(ctx, ctx_vm(parent));
>   			mutex_unlock(&ctx->mutex);
>   
>   			ce = i915_gem_context_get_engine(ctx, engine->legacy_idx);
>   			GEM_BUG_ON(IS_ERR(ce));
>   
>   			if (!obj) {
> -				obj = create_test_object(parent->vm, file, &objects);
> +				obj = create_test_object(ctx_vm(parent),
> +							 file, &objects);
>   				if (IS_ERR(obj)) {
>   					err = PTR_ERR(obj);
>   					intel_context_put(ce);
> @@ -1344,14 +1351,11 @@ static int igt_ctx_readonly(void *arg)
>   		goto out_file;
>   	}
>   
> -	rcu_read_lock();
> -	vm = rcu_dereference(ctx->vm) ?: &i915->ggtt.alias->vm;
> +	vm = ctx_vm(ctx) ?: &i915->ggtt.alias->vm;
>   	if (!vm || !vm->has_read_only) {
> -		rcu_read_unlock();
>   		err = 0;
>   		goto out_file;
>   	}
> -	rcu_read_unlock();
>   
>   	ndwords = 0;
>   	dw = 0;
> @@ -1381,7 +1385,7 @@ static int igt_ctx_readonly(void *arg)
>   				pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) [full-ppgtt? %s], err=%d\n",
>   				       ndwords, dw, max_dwords(obj),
>   				       ce->engine->name,
> -				       yesno(!!rcu_access_pointer(ctx->vm)),
> +				       yesno(!!ctx_vm(ctx)),
>   				       err);
>   				i915_gem_context_unlock_engines(ctx);
>   				goto out_file;
> @@ -1699,11 +1703,11 @@ static int igt_vm_isolation(void *arg)
>   	}
>   
>   	/* We can only test vm isolation, if the vm are distinct */
> -	if (ctx_a->vm == ctx_b->vm)
> +	if (ctx_vm(ctx_a) == ctx_vm(ctx_b))
>   		goto out_file;
>   
> -	vm_total = ctx_a->vm->total;
> -	GEM_BUG_ON(ctx_b->vm->total != vm_total);
> +	vm_total = ctx_vm(ctx_a)->total;
> +	GEM_BUG_ON(ctx_vm(ctx_b)->total != vm_total);
>   	vm_total -= I915_GTT_PAGE_SIZE;
>   
>   	count = 0;
>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 0d4cdf38d5f9..13ae8f66bed6 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -26,6 +26,12 @@ 
 
 #define DW_PER_PAGE (PAGE_SIZE / sizeof(u32))
 
+static inline struct i915_address_space *ctx_vm(struct i915_gem_context *ctx)
+{
+	/* single threaded, private ctx */
+	return rcu_dereference_protected(ctx->vm, true);
+}
+
 static int live_nop_switch(void *arg)
 {
 	const unsigned int nctx = 1024;
@@ -787,14 +793,15 @@  static int igt_shared_ctx_exec(void *arg)
 			}
 
 			mutex_lock(&ctx->mutex);
-			__assign_ppgtt(ctx, parent->vm);
+			__assign_ppgtt(ctx, ctx_vm(parent));
 			mutex_unlock(&ctx->mutex);
 
 			ce = i915_gem_context_get_engine(ctx, engine->legacy_idx);
 			GEM_BUG_ON(IS_ERR(ce));
 
 			if (!obj) {
-				obj = create_test_object(parent->vm, file, &objects);
+				obj = create_test_object(ctx_vm(parent),
+							 file, &objects);
 				if (IS_ERR(obj)) {
 					err = PTR_ERR(obj);
 					intel_context_put(ce);
@@ -1344,14 +1351,11 @@  static int igt_ctx_readonly(void *arg)
 		goto out_file;
 	}
 
-	rcu_read_lock();
-	vm = rcu_dereference(ctx->vm) ?: &i915->ggtt.alias->vm;
+	vm = ctx_vm(ctx) ?: &i915->ggtt.alias->vm;
 	if (!vm || !vm->has_read_only) {
-		rcu_read_unlock();
 		err = 0;
 		goto out_file;
 	}
-	rcu_read_unlock();
 
 	ndwords = 0;
 	dw = 0;
@@ -1381,7 +1385,7 @@  static int igt_ctx_readonly(void *arg)
 				pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) [full-ppgtt? %s], err=%d\n",
 				       ndwords, dw, max_dwords(obj),
 				       ce->engine->name,
-				       yesno(!!rcu_access_pointer(ctx->vm)),
+				       yesno(!!ctx_vm(ctx)),
 				       err);
 				i915_gem_context_unlock_engines(ctx);
 				goto out_file;
@@ -1699,11 +1703,11 @@  static int igt_vm_isolation(void *arg)
 	}
 
 	/* We can only test vm isolation, if the vm are distinct */
-	if (ctx_a->vm == ctx_b->vm)
+	if (ctx_vm(ctx_a) == ctx_vm(ctx_b))
 		goto out_file;
 
-	vm_total = ctx_a->vm->total;
-	GEM_BUG_ON(ctx_b->vm->total != vm_total);
+	vm_total = ctx_vm(ctx_a)->total;
+	GEM_BUG_ON(ctx_vm(ctx_b)->total != vm_total);
 	vm_total -= I915_GTT_PAGE_SIZE;
 
 	count = 0;