Re: [PATCH 1/3] livepatch: Add comment to clarify klp_add_nops()

[Petr Mladek <pmladek@suse.com>]

On Tue 2025-02-11 14:24:35, Yafang Shao wrote:
> Add detailed comments to clarify the purpose of klp_add_nops() function.
> These comments are based on Petr's explanation[0].
> 
> Link: https://lore.kernel.org/all/Z6XUA7D0eU_YDMVp@pathway.suse.cz/ [0]
> Suggested-by: Josh Poimboeuf <jpoimboe@kernel.org>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Cc: Petr Mladek <pmladek@suse.com>
> ---
>  kernel/livepatch/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 0cd39954d5a1..5b2a52e7c2f6 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -604,6 +604,9 @@ static int klp_add_object_nops(struct klp_patch *patch,
>   * Add 'nop' functions which simply return to the caller to run
>   * the original function. The 'nop' functions are added to a
>   * patch to facilitate a 'replace' mode.
> + *
> + * The 'nop' entries are added only for functions which are currently
> + * livepatched but are no longer included in the new livepatch.
>   */

The new comment makes perfect sense. But I would re-shuffle the text a bit
to to make it more clear that it is used only in the 'replace' mode.
Something like:

/*
 * Add 'nop' functions which simply return to the caller to run the original
 * function.
 *
 * They are added only when the atomic replace mode is used and only for
 * functions which are currently livepatched but are no longer included
 * in the new livepatch.
 */


>  static int klp_add_nops(struct klp_patch *patch)
>  {

Best Regards,
Petr