On Fri, May 29, 2026 at 12:54 PM <[email protected]> wrote:
>
> 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?

I will wait for the feedback from the maintainers.

-- 
Regards
Yafang

Reply via email to