On Fri, Jun 7, 2024 at 8:59 PM Marcos Paulo de Souza <mpdesouza@xxxxxxxx> wrote:
>
> 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.
I will add it. Thanks for your suggestion.
--
Regards
Yafang
