Re: [RFC PATCH v4 3/4] livepatch: Implement replace set for scoped atomic replace
Song Liu <[email protected]>
| Newsgroups | org.kernel.vger.live-patching |
|---|---|
| Message-ID | <CAPhsuW4ge+nbXvqZO4FhQFhUbn7Lqt3Uvn0KTC-dYr=wL73FQA@mail.gmail.com> |
On Mon, Aug 3, 2026 at 11:50 PM Yafang Shao <[email protected]> wrote: [...] > /* > - * Check that the new livepatch will not break the existing system states. > - * Cumulative patches must handle all already modified states. > - * Non-cumulative patches can touch already modified states. > + * Refuse loading a livepatch which would want to modify a > + * function which is already livepatched with the livepatch > + * with another "provides". > */ The comment above is not really accurate. This function doesn't really check "provides" or "obsoletes". Also, as long as the old patch's "provides" is in the new patches "obsoletes" or "provides", this replace is allowed (the comment doesn't mention "obsoletes". ) > +static bool klp_has_function_conflict(struct klp_patch *patch, > + struct klp_patch *old_patch) > +{ > + struct klp_object *obj, *old_obj; > + struct klp_func *func; [...] > > + # Always add provides ID to obsoletes (even if provides=0) > + # This ensures the new patch replaces old patches with the same provides ID Do we really need th toolchain to enforce this? AFAICT, the kernel already guranteed "provides" means "obsoletes". Also, it will be great if we can add some basic tests for this. We are working on some new test framework for klp-build toolchain. But we don't have to wait for that. It is always good to add some tests for a major feature change. Thanks, Song > + if [[ -n "$OBSOLETES" && "$OBSOLETES" != "[]" ]]; then > + local obsoletes_clean="${OBSOLETES//[\[\] ]/}" > + local IFS=',' > + local -a obs_array=() > + local obs_id > + local provides_added=0 > + local already_exists [...]