Re: [PATCH v2 3/4] livepatch/shadow: Introduce klp_shadow_type structure

[Josh Poimboeuf <jpoimboe@kernel.org>]

On Wed, Oct 26, 2022 at 04:41:21PM -0300, Marcos Paulo de Souza wrote:
> +++ b/include/linux/livepatch.h
> @@ -216,15 +216,26 @@ typedef int (*klp_shadow_ctor_t)(void *obj,
>  				 void *ctor_data);
>  typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data);
>  
> -void *klp_shadow_get(void *obj, unsigned long id);
> -void *klp_shadow_alloc(void *obj, unsigned long id,
> -		       size_t size, gfp_t gfp_flags,
> -		       klp_shadow_ctor_t ctor, void *ctor_data);
> -void *klp_shadow_get_or_alloc(void *obj, unsigned long id,
> -			      size_t size, gfp_t gfp_flags,
> -			      klp_shadow_ctor_t ctor, void *ctor_data);
> -void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor);
> -void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor);
> +/**
> + * struct klp_shadow_type - shadow variable type used by the klp_object
> + * @id:		shadow variable type indentifier

"identifier"

> diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c
> index aba44dcc0a88..64e83853891d 100644
> --- a/kernel/livepatch/shadow.c
> +++ b/kernel/livepatch/shadow.c
> @@ -63,24 +63,24 @@ struct klp_shadow {
>   * klp_shadow_match() - verify a shadow variable matches given <obj, id>

"matches given <obj, type>" ?

>   * @shadow:	shadow variable to match
>   * @obj:	pointer to parent object
> - * @id:		data identifier
> + * @shadow_type: type of the wanted shadow variable
>   *
>   * Return: true if the shadow variable matches.
>   */
>  static inline bool klp_shadow_match(struct klp_shadow *shadow, void *obj,
> -				unsigned long id)
> +				struct klp_shadow_type *shadow_type)
>  {
> -	return shadow->obj == obj && shadow->id == id;
> +	return shadow->obj == obj && shadow->id == shadow_type->id;

"shadow_type" is redundant, can we just call it "type"?

Same comment for all other instances of 'shadow_type' throughout the
patch.

> @@ -159,22 +157,25 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id,
>  	 * More complex setting can be done by @ctor function.  But it is
>  	 * called only when the buffer is really used (under klp_shadow_lock).
>  	 */
> -	new_shadow = kzalloc(size + sizeof(*new_shadow), gfp_flags);
> +	new_shadow = kzalloc(size + sizeof(struct klp_shadow), gfp_flags);

Unnecessary change?

-- 
Josh