[Intel-gfx] [PATCH 3/3] drm/i915: Use rcu_dereference for rcu protected pointer

[Niranjan Vishwanathapura]

On Wed, Nov 06, 2019 at 08:52:58AM +0000, Chris Wilson wrote:
>Quoting Niranjan Vishwanathapura (2019-11-06 02:16:42)
>> On Wed, Nov 06, 2019 at 12:36:47AM +0000, Chris Wilson wrote:
>> >Quoting Niranjana Vishwanathapura (2019-11-06 00:02:05)
>> >> 'ctx\->vm' is rcu protected, so use rcu_dereference inside
>> >> read side critical section. It fixes a sparse warning.
>> >>
>> >> Cc: Chris P Wilson <chris.p.wilson at intel.com>
>> >> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 +++++----
>> >>  1 file changed, 5 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> >> index de6e55af82cf..d3a622f60fd9 100644
>> >> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> >> @@ -1002,12 +1002,13 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv,
>> >>         struct i915_address_space *vm;
>> >>         int ret;
>> >>
>> >> -       if (!rcu_access_pointer(ctx->vm))
>> >> -               return -ENODEV;
>> >
>> >This is correctly serialised by the caller.
>>
>> Ok, is this worth fixing the sparse warning?
>
>  CHECK   drivers/gpu/drm/i915/gem/i915_gem_context.c
>drivers/gpu/drm/i915/gem/i915_gem_context.c:2087:17: error: bad integer constant expression
>drivers/gpu/drm/i915/gem/i915_gem_context.c:2088:17: error: bad integer constant expression
>drivers/gpu/drm/i915/gem/i915_gem_context.c:2089:17: error: bad integer constant expression
>drivers/gpu/drm/i915/gem/i915_gem_context.c:2090:17: error: bad integer constant expression
>drivers/gpu/drm/i915/gem/i915_gem_context.c:2091:17: error: bad integer constant expression
>drivers/gpu/drm/i915/gem/i915_gem_context.c:2092:17: error: bad integer constant expression
>
>Odd.

I am seeing,

  CHECK   drivers/gpu/drm/i915/gem/i915_gem_context.c
drivers/gpu/drm/i915/gem/i915_gem_context.c:1009:29: warning: incorrect type in argument 1 (different address spaces)
drivers/gpu/drm/i915/gem/i915_gem_context.c:1009:29:    expected struct i915_address_space *vm
drivers/gpu/drm/i915/gem/i915_gem_context.c:1009:29:    got struct i915_address_space [noderef] <asn:4> *vm
drivers/gpu/drm/i915/gem/i915_gem_context.c:1958:25: warning: incorrect type in argument 1 (different address spaces)
drivers/gpu/drm/i915/gem/i915_gem_context.c:1958:25:    expected struct i915_gem_engines *e
drivers/gpu/drm/i915/gem/i915_gem_context.c:1958:25:    got struct i915_gem_engines [noderef] <asn:4> *engines
drivers/gpu/drm/i915/gem/i915_gem_context.c:1993:15: warning: incorrect type in assignment (different address spaces)
drivers/gpu/drm/i915/gem/i915_gem_context.c:1993:15:    expected struct i915_gem_engines *clone
drivers/gpu/drm/i915/gem/i915_gem_context.c:1993:15:    got struct i915_gem_engines [noderef] <asn:4> *engines

>
>Sure, for correctness see clone_vm().

Ok, I can put a common helper function like vm_get_rcu_safe(ctx).

Niranjana

>-Chris