On Fri, Jun 7, 2024 at 8:33 PM Miroslav Benes <mbenes@xxxxxxx> wrote:
>
> Hi,
>
> I think the subject should be something like
>
> "livepatch: Add "replace" sysfs attribute"
agreed.
>
> 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.
agreed. I'm not a native speaker either :)
>
> > 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.
It seems I didn't describe it clearly. We are transitioning non atomic
replace livepatches to atomic replace livepatches, which will take
some time to complete in a large fleet of servers. In other words,
some servers are still running non atomic replace livepatches while
others are running atomic replace livepatches. As a result, the
sysadmins have complained that they can't find a way to identify
whether a livepatch is an atomic replace or not. It will be beneficial
to show it directly.
>
> > 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.
I will do it. Thanks for your suggestion.
--
Regards
Yafang
