Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] Removing `data` from `struct klp_state` breaks compilation of the 
livepatch selftests and leaves kernel documentation explicitly referencing a 
non-existent API.
--

commit 792ddcf269c47d4de395b3873cfd5f02cbf282ef
Author: Petr Mladek <[email protected]>

livepatch: Remove "data" from struct klp_state

The commit removes the data pointer from struct klp_state since its lifetime
is tied to the module rather than the state. This pointer is lost during a
livepatch replacement, so shadow variables are recommended instead.

> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index fef0c2f970838..56b7e4fe01bc8 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -152,14 +152,12 @@ struct klp_state_callbacks {
>   * @callbacks:       optional callbacks used when enabling or disabling the 
> state
>   * @is_shadow:       the state handles lifetime of a shadow variable with
>   *           the same @id
> - * @data:    custom data
>   */
>  struct klp_state {
>       unsigned long id;
>       unsigned int version;
>       struct klp_state_callbacks callbacks;
>       bool is_shadow;
> -     void *data;

Does this code removal cause a regression by breaking the compilation of
livepatch selftests?

For example, test_klp_state.c and test_klp_state2.c actively use this field
to store and transfer state.

In tools/testing/selftests/livepatch/test_modules/test_klp_state.c:

allocate_loglevel_state() {
        ...
        loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
        if (!loglevel_state->data)
                return -ENOMEM;
        ...
}

Additionally, does Documentation/livepatch/system-state.rst still reference
this non-existent API? It continues to provide instructions like:

In Documentation/livepatch/system-state.rst:
  - Allocate *state->data* when necessary. The allocation might fail
  ...
  - Make sure that *state->data* has all necessary information.

Should the selftests and documentation be updated as part of this change to
prevent build failures and confusion?

>  };
>  
>  /**

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=4

Reply via email to