[Intel-gfx] [PATCH v6 2/3] drm/i915: Prepare error capture to work with const modparams

[Jani Nikula]

On Tue, 19 Sep 2017, Michal Wajdeczko <michal.wajdeczko at intel.com> wrote:
> We are planning to enforce "read_mostly" access to modparams.
> Let start handle modparams as it was already defined as const.

FWIW, __read_mostly is a hint for caching, not so much for the
developer. We've added __read_mostly to *reflect* the usage pattern, not
to *enforce* a usage pattern.

I don't disagree modifying should be discouraged, but please don't
conflate the concepts.

>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index c7aaf62..15fe8ed 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -820,7 +820,7 @@ static void i915_error_object_free(struct drm_i915_error_object *obj)
>  	kfree(obj);
>  }
>  
> -static __always_inline void free_param(const char *type, void *x)
> +static __always_inline void free_param(const char *type, const void *x)
>  {
>  	if (!__builtin_strcmp(type, "char *"))
>  		kfree(*(void **)x);
> @@ -1680,7 +1680,7 @@ static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
>  	       sizeof(error->device_info));
>  }
>  
> -static __always_inline void dup_param(const char *type, void *x)
> +static __always_inline void dup_param(const char *type, const void *x)
>  {
>  	if (!__builtin_strcmp(type, "char *"))
>  		*(void **)x = kstrdup(*(void **)x, GFP_ATOMIC);
> @@ -1696,7 +1696,7 @@ static int capture(void *data)
>  		ktime_to_timeval(ktime_sub(ktime_get(),
>  					   error->i915->gt.last_init_time));
>  
> -	error->params = i915_modparams;
> +	memcpy(&error->params, &i915_modparams, sizeof(i915_modparams));

Nitpick, sizeof(error->params).

BR,
Jani.

>  #define DUP(T, x) dup_param(#T, &error->params.x);
>  	I915_PARAMS_FOR_EACH(DUP);
>  #undef DUP

-- 
Jani Nikula, Intel Open Source Technology Center