On Fri, 2024-06-07 at 14:33 +0200, Miroslav Benes wrote:
> Hi,
>
> I think the subject should be something like
>
> "livepatch: Add "replace" sysfs attribute"
>
> or use a different way to stress "replace" is the name.
>
> On Fri, 7 Jun 2024, Yafang Shao wrote:
>
> > When building a livepatch, a user can set it to be either an
> > atomic-replace
> > livepatch or a non-atomic-replace livepatch.
>
> I am not a native speaker but I would drop '-' everywhere in this
> context.
>
> > However, it is not easy to
> > identify whether a livepatch is atomic-replace or not until it
> > actually
> > replaces some old livepatches.
>
> Ok.
>
> > This can lead to mistakes in a mixed
> > atomic-replace and non-atomic-replace environment, especially when
> > transitioning all livepatches from non-atomic-replace to atomic-
> > replace in
> > a large fleet of servers.
>
> Out of curiosity, could you describe your setup in more detail here?
> It is
> interesting to mix different type of livepatches so I would like to
> learn
> more.
>
> > To address this issue, a new sysfs interface called 'replace' is
> > introduced
> > in this patch. The result after this change is as follows:
> >
> > $ cat /sys/kernel/livepatch/livepatch-non_replace/replace
> > 0
> >
> > $ cat /sys/kernel/livepatch/livepatch-replace/replace
> > 1
> >
> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > ---
> > Documentation/ABI/testing/sysfs-kernel-livepatch | 8 ++++++++
> > kernel/livepatch/core.c | 12 ++++++++++++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch
> > b/Documentation/ABI/testing/sysfs-kernel-livepatch
> > index a5df9b4910dc..3735d868013d 100644
> > --- a/Documentation/ABI/testing/sysfs-kernel-livepatch
> > +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
> > @@ -47,6 +47,14 @@ Description:
> > disabled when the feature is used. See
> > Documentation/livepatch/livepatch.rst for more
> > information.
> >
> > +What: /sys/kernel/livepatch/<patch>/replace
> > +Date: Jun 2024
> > +KernelVersion: 6.11.0
> > +Contact: live-patching@xxxxxxxxxxxxxxx
> > +Description:
> > + An attribute which indicates whether the patch
> > supports
> > + atomic-replace.
> > +
> > What: /sys/kernel/livepatch/<patch>/<object>
> > Date: Nov 2014
> > KernelVersion: 3.19.0
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 52426665eecc..0e9832f146f1 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -346,6 +346,7 @@ int klp_apply_section_relocs(struct module
> > *pmod, Elf_Shdr *sechdrs,
> > * /sys/kernel/livepatch/<patch>/enabled
> > * /sys/kernel/livepatch/<patch>/transition
> > * /sys/kernel/livepatch/<patch>/force
> > + * /sys/kernel/livepatch/<patch>/replace
> > * /sys/kernel/livepatch/<patch>/<object>
> > * /sys/kernel/livepatch/<patch>/<object>/patched
> > * /sys/kernel/livepatch/<patch>/<object>/<function,sympos>
> > @@ -443,13 +444,24 @@ static ssize_t force_store(struct kobject
> > *kobj, struct kobj_attribute *attr,
> > return count;
> > }
> >
> > +static ssize_t replace_show(struct kobject *kobj,
> > + struct kobj_attribute *attr, char
> > *buf)
> > +{
> > + struct klp_patch *patch;
> > +
> > + patch = container_of(kobj, struct klp_patch, kobj);
> > + return snprintf(buf, PAGE_SIZE-1, "%d\n", patch->replace);
>
> It might be better to use sysfs_emit() here. See patched_show() in
> the
> same file. There are still snprintf() occurrences and it might be a
> separate cleanup patch if you are interested.
Hi Yafang, thanks for the patch!
Besides Miroslav's comments, I think that a new kselftest exercising
this attribute would be good, since we have tests for other attributes
as well.
Thanks,
Marcos
>
> Regards,
> Miroslav
>
