Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Critical] The commit entirely removes the `stack_order` sysfs attribute 
while only marking it as `<deprecated>` in the ABI documentation, causing 
immediate backwards compatibility breakage and contradicting documentation 
guidelines.
--

commit 041b2dbba1a7397626104dd773fc1f37228d382c
Author: Yafang Shao <[email protected]>

livepatch: deprecate stack_order

This commit deprecates the stack_order sysfs attribute for atomic-replace
livepatches, since a single function can only be modified by a unique
replace_set.

> diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch 
> b/Documentation/ABI/testing/sysfs-kernel-livepatch
> index 6d75235a6a2e4..fddad93dba797 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-livepatch
> +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
> @@ -62,6 +62,7 @@ Description:
>               are applied to the system. If multiple live patches modify the 
> same
>               function, the implementation with the biggest 'stack_order' 
> number
>               is used, unless a transition is currently in progress.
> +             <deprecated>

[ ... ]

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 969fea2a92631..6d65f839f4420 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
[ ... ]
> @@ -457,38 +456,15 @@ static ssize_t replace_set_show(struct kobject *kobj,
>       return sysfs_emit(buf, "%u\n", patch->replace_set);
>  }
>  
> -static ssize_t stack_order_show(struct kobject *kobj,
> -                             struct kobj_attribute *attr, char *buf)
> -{
> -     struct klp_patch *patch, *this_patch;
> -     int stack_order = 0;
> -
> -     this_patch = container_of(kobj, struct klp_patch, kobj);
> -
> -     mutex_lock(&klp_mutex);
> -
> -     klp_for_each_patch(patch) {
> -             stack_order++;
> -             if (patch == this_patch)
> -                     break;
> -     }
> -
> -     mutex_unlock(&klp_mutex);
> -
> -     return sysfs_emit(buf, "%d\n", stack_order);
> -}
> -
>  static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
>  static struct kobj_attribute transition_kobj_attr = __ATTR_RO(transition);
>  static struct kobj_attribute force_kobj_attr = __ATTR_WO(force);
>  static struct kobj_attribute replace_set_kobj_attr = __ATTR_RO(replace_set);
> -static struct kobj_attribute stack_order_kobj_attr = __ATTR_RO(stack_order);
>  static struct attribute *klp_patch_attrs[] = {
>       &enabled_kobj_attr.attr,
>       &transition_kobj_attr.attr,
>       &force_kobj_attr.attr,
>       &replace_set_kobj_attr.attr,
> -     &stack_order_kobj_attr.attr,
>       NULL
>  };

[Severity: Critical]
The commit message and documentation state that the stack_order attribute
is being deprecated, but this code completely removes the attribute. Does
this abruptly break ABI compatibility for userspace tools that expect this
sysfs file to exist?

Normally, deprecating an interface requires keeping it around for a
transition period (perhaps returning a dummy value) rather than deleting
it immediately. Also, if it were actually removed, shouldn't the ABI
documentation be moved to the removed/ directory instead of just appending
<deprecated> to the existing entry?

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

Reply via email to