Live Patching
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Yafang Shao <laoar.shao@gmail.com>
Cc: jpoimboe@kernel.org, jikos@kernel.org, mbenes@suse.cz,
	joe.lawrence@redhat.com, song@kernel.org,
	live-patching@vger.kernel.org
Subject: Re: [PATCH v10 5/9] livepatch: Implement provides and obsoletes for scoped atomic replace
Date: Thu, 8 Oct 2026 12:30:31 +0200	[thread overview]
Message-ID: <asdwx-BENZy6dajG@pathway.suse.cz> (raw)
In-Reply-To: <CALOAHbCHwJ2htPOoako_EqNg-bQF=4NLNqVGAnzsAi2uitMdeQ@mail.gmail.com>

On Sun 2026-10-04 23:46:29, Yafang Shao wrote:
> On Sat, Oct 3, 2026 at 1:06 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> > On Wed 2026-09-30 11:03:19, Yafang Shao wrote:
> > > The current bool replace flag is too coarse: it is either all or
> > > nothing. A livepatch with .replace=true replaces ALL existing
> > > livepatches, which is safe but inflexible. There is no way to have
> > > multiple independent livepatch sets coexist on the same system.
> > >
> > > Replace it with a more flexible model using two new fields in
> > > struct klp_patch:
> > >
> > > - provides: an unsigned int id identifying the changes made by
> > >   the related livepatch. Where the changes are a set
> > >   of modified objects, functions, and used callbacks,
> > >   shadow variable ids, and state ids.
> > >
> > > - obsoletes: an optional array of unsigned int ids specifying
> > >   additional provides ids to be replaced. This allows a new patch
> > >   to explicitly obsolete patches from different scopes.
> > >
> > > A new livepatch atomically replaces any existing livepatch that
> > > meets any of the following conditions:
> > > 1. It has the same provides id as the new patch,
> > > 2. Its provides id is listed in the new patch's obsoletes list, or
> > > 3. The new patch's provides id is listed in its obsoletes list
> > >    (the obsoletes relationship is symmetric).
> > >
> > > The obsoletes relationship is symmetric: two livepatches that
> > > obsolete each other are mutually exclusive. Loading either one
> > > atomically removes the other, so they can never be enabled at the
> > > same time. This allows swapping between two independently prepared
> > > livepatches that cover the same scope, in either direction.
> >
> > Honestly, I am confused by the 3rd rule and by the last paragraph.
> > It is not compatible with my understanding of the provides/obsoletes
> > semantic.
> 
> Correct. In package management semantics, Obsoletes is not a symmetric
> relationship.
> 
> >
> > In my mental model, the provides and obsoletes define which
> > way is supported. Let me show an example, let's have:
> >
> >  Livepatch A:
> >    + provides: 1
> >    + obsoletes:
> >
> >  Livepatch B:
> >    + provides: 2
> >    + obsoletes: 1
> >
> >  Livepatch C:
> >    + provides: 3
> >    + obsoletes: 1,2
> >
> >  Livepatch D:
> >    + provides: 2
> >    + obsoletes: 1,3
> >
> > How is this supposed to work, please?
> >
> > In my mental model:
> >
> >   Livepatch A:
> >      + can be replaced by B, C, or D because they all obsolete 1
> >      + can _not_ replace B, C, D because A does not obsolete 2 and/or 3.
> >
> >   Livepatch B:
> >     + can be replaced by C because C obsoletes both 1 and 2
> >     + can be replaced by D beause D provides 2 and obsoletes 1
> >     + can't replace C or D because B neither provides not obsoletes 3.
> >
> >   Livepatch C:
> >     + can be replaced by D because D provides and obsoletes
> >        all ids which are proviced and obsoleted by D
> >
> >     + can replace D from the same reasons.
> >
> >
> > OK, I understand that it might be practical to allow replacing
> > the patches just because they are "conflicting". As a kind
> > of revert/downgrade.
> 
> Right. This will let us easily roll back the buggy livepatch and
> reinstall the old one as part of the rollout.
> 
> >
> > It might help to rename "obsoletes" to "conflicts". And just
> > say that the livepatch replaces livepatch with the same id
> > in provides and also all conflicting patches.
> >
> > But then I see another problem. We have three types of conflicts:
> >
> >   1. same id in provides/obsoletes/conflicts
> >   2. same livepatches function
> >   3. same livepatch state id
> >
> > It is weird that we would silently replace livepatches with 1st
> > type conflict. But we would refuse to load the livepatch with
> > the 2nd and/or 3rd type conflict.
> >
> > Note that the inconsistent handling of conflicts exists in
> > this patch even when the array is called "obsoletes". My proposal
> > to rename it to "conflicts" just helped me to see this
> > inconsistence.
> >
> > Any option/idea? Yet another naming?
> 
> Perhaps it could use a better name, but I'm not good at naming things ;)
> 
> >
> > One more thing, the commit message does not describe that
> > the livepatches might conflict when livepatching the same
> > function or using the same state id. It is important part
> > of the semantic and must be documented.
> 
> Sure. I will update it.
> 
> >
> > Best Regards,
> > Petr
> >
> > PS: I am sorry but I won't be able to finish the review before
> >     the Plumbers conference. But I am happy that I have found
> >     the above problem. We could discuss it at the conference
> >     and come up with some sane naming and semantic.
> 
> I can't be there in person, but I believe we'll come up with a
> sensible name or semantic. I'll hold off until we reach a final
> conclusion.

I am not sure if you saw the video from the the conference.
It looked like we were going to nack this feature because
installing more livepatches on the system was considered
too dangerous.

In particular, it allows using livepatches provided by different
teams/vendors. It is almost impossible to prevent incompatible
changes and breaking the system. The check preventing livepatching
the same function or using the same state IDs is just the best
effort but it still allows many hidden incompatibilities.

We should rather discourage people installing more livepatches
in parallel.

Anyway, we had a follow-up discussion and agreed to accept this
feature in the end. We already allow to install more livepatches
in parallel. So this patchset is not adding any new danger.
Instead, it reduces the danger a bit without complicating the
code too much.

Also we understand that there might be some use cases for this
feature. And it might be safe when used carefully.


We also discussed the semantic of the obsolete field and we
would like to it simple. The livepatch with a provides ID should
replace a livepatch with another provides ID _only_ when the other
ID is mentioned in obsoletes.

By other words, please remove the symmetry and "support" for
a rollback. The symmetry causes some confusion as explained
before. I would summarize it by two reasons:

   + In the packaging world, the "obsoletes" behavior is not
     symmetric.

   + The symmetric behavior adds a "conflict" semantic. But
     this particular conflict is handled another way than
     the other conflicts (livepatches functions, state IDs).
     I mean replacing an already enabled livepatch vs.
     rejecting the new one.

In addition, we do not support the rollback in all situations,
for example, let's have:

Livepatch A:
  + provides: 1
  + obsoletes:

Livepatch B:
  + provides: 2
  + obsoletes:

Livepatch C:
  + provides: 3
  + obsoletes: 1,2

The meaning is:

  + A and B are independent livepatches.
  + C adds some dependency and has to replace both A and B.

Here a proper rollback from C to A+B would require loading both
A and B livepatches synchronously. It would require adding
special support into the module loader and the livepatch core code.
We do not want to add this complexity. Especially, because
the parallel installation of livepatches is considered a bad
practice.

A partial workaround might be to allow changing "obsoletes" via
a livepatch module parameter. It would allow to do the
rollback in 2 steps:

   modprobe obsoletes=3 livepatch_A
   modprobe livepatch_B

It is still not an atomic rollback. But it is better than
without the parameter which would require 3 steps:

   echo 0 > /sys/kernel/livepatch/livepatch_C/enabled
   modprobe livepatch_A
   modprobe livepatch_B



Summary:

We are open to accept this feature but please:

    + Remove the symmetric handling of obsolete flag.

    + Properly document, in both the commit message and
      documentation, when the livepatches will be replaced
      and when they will conflict.

    + Document the pitfalls with the rollback.

    + Add a warning that installing more livepatches in parallel
      is dangerous, especially when they are provided by different
      teams or vendors. The built-in checks of possible conflicts
      do not prevent potential incompatibilities and breakages.


Heh, I hope that I got it right.

Best Regards,
Petr

  reply	other threads:[~2026-10-08 10:30 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30  3:03 [PATCH v10 0/9] livepatch: Add support for scoped atomic replace Yafang Shao
2026-09-30  3:03 ` [PATCH v10 1/9] selftests/livepatch: Clarify test module file names Yafang Shao
2026-10-01 15:27   ` Petr Mladek
2026-09-30  3:03 ` [PATCH v10 2/9] selftests/livepatch: Adapt atomic replace tests to provides/obsoletes Yafang Shao
2026-10-01 15:57   ` Petr Mladek
2026-10-02 12:01   ` Miroslav Benes
2026-09-30  3:03 ` [PATCH v10 3/9] livepatch: Make klp_find_func() non static Yafang Shao
2026-10-02 14:19   ` Petr Mladek
2026-09-30  3:03 ` [PATCH v10 4/9] livepatch: Call klp_init_patch_early() earlier Yafang Shao
2026-10-02 15:58   ` Petr Mladek
2026-09-30  3:03 ` [PATCH v10 5/9] livepatch: Implement provides and obsoletes for scoped atomic replace Yafang Shao
2026-10-02 12:01   ` Miroslav Benes
2026-10-02 17:06   ` Petr Mladek
2026-10-04 15:46     ` Yafang Shao
2026-10-08 10:30       ` Petr Mladek [this message]
2026-10-08 11:04         ` Yafang Shao
2026-10-09  8:33           ` Yafang Shao
2026-09-30  3:03 ` [PATCH v10 6/9] livepatch: Deprecate stack_order Yafang Shao
2026-09-30  3:03 ` [PATCH v10 7/9] selftests/livepatch: Add provides/obsoletes test scenarios Yafang Shao
2026-09-30  3:13   ` sashiko-bot
2026-09-30  3:29     ` Yafang Shao
2026-10-02 12:27   ` Miroslav Benes
2026-09-30  3:03 ` [PATCH v10 8/9] selftests/livepatch: Add state test for provides/obsoletes Yafang Shao
2026-09-30  3:03 ` [PATCH v10 9/9] selftests/livepatch: Add function " Yafang Shao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=asdwx-BENZy6dajG@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@kernel.org \
    --cc=laoar.shao@gmail.com \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=song@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox