Date:   Mon, 30 Jan 2023 20:36:20 -0800
From:   Josh Poimboeuf <jpoimboe@kernel.org>
To:     Marcos Paulo de Souza <mpdesouza@suse.com>
Cc:     linux-kernel@vger.kernel.org, live-patching@vger.kernel.org,
        jpoimboe@redhat.com, joe.lawrence@redhat.com, pmladek@suse.com
Subject: Re: [PATCH v2 3/4] livepatch/shadow: Introduce klp_shadow_type
 structure
Message-ID: <20230131043620.6nnhqgqr4z55hxfr@treble>
References: <20221026194122.11761-1-mpdesouza@suse.com>
 <20221026194122.11761-4-mpdesouza@suse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20221026194122.11761-4-mpdesouza@suse.com>
Precedence: bulk

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