On Tue 2018-02-27 09:58:40, Joe Lawrence wrote: > On 02/27/2018 07:36 AM, Miroslav Benes wrote: > > On Fri, 23 Feb 2018, Joe Lawrence wrote: > > > >> [ ... snip ... ] > >> > >> +If a livepatch is replaced by a cumulative patch, then only the > >> +callbacks belonging to the cumulative patch will be executed. This > >> +simplifies the livepatching core for it is the responsibility of the > >> +cumulative patch to safely revert whatever needs to be reverted. See > >> +Documentation/livepatch/cumulative.txt for more information on such > >> +patches. > > > > s/cumulative/atomic replace/ almost everywhere? > > > > 'Documentation/livepatch/cumulative.txt' should be > > 'Documentation/livepatch/cumulative-patches.txt' and we may rename it > > atomic-replace-patches.txt. I don't know. Cumulative patches forms a > > subset of atomic replace patches in my understanding. The feature itself > > is more general. Even if practically used for cumulative patches only. But > > it is for you and Petr to decide. > > Hi Miroslav, > > Thanks for reviewing! > > I guess I'm a little confused about the distinction here. > > I understood a "cumulative-patch" to mean that it would contain the sum > of all changes. So instead of this: > > patch 1 = A > + patch 2 = B > + patch 3 = C > ----------------------- > net = A + B + C > > We can group all of the changes together into a single cumulative-patch > for the same net effect: > > patch 1 = A -replaced by- > patch 2 = A + B -replaced by- > patch 3 = A + B + C > > I assumed this would also mean to include any reverted changes as well. > So in the example above, if change C needed to be reverted, then: > > patch 4 = A + B > > and that would still be considered a "cumulative-patch". Yes, I would consider this a cumulative patch. > In my mind, atomic replace is the mechanism that forces patching to be > cumulative. Perhaps this is too strict? Are there other use-cases for > atomic-replace? Jason talked about using the atomic replace to get rid of any existing livepatches and adding another changes instead. The changes in the old and the new patch might be unrelated. They simply do not want to mind what was there before. The term "atomic replace" fits perfectly for this usecase. My understanding is that cumulative patches do similar thing. But the old and new patches should be related. In particular, any new patch should include most changes from the older one. The only exception is when an old change was wrong and we do not want it anymore. Now, your examples are close the the Jason's use case. They do: patch1 = A -replaced by- patch2 = B ------------------ result B I mean that one change is replaced by an "unrelated" one. It might confuse people. They might ask why the new patch is called cumulative when all the older changes are lost. I would suggest to rename the sample patch to livepatch-test-replace or so. Also I would try to avoid the world cumulative in the example to avoid confusion. I still would prefer to keep the documentation for the feature called cumulative-patches.txt. From my point of view, the atomic replace is rather a technical detail. It might be dangerous when used for non-related patches. On the other, cumulative patches seem to be a promising way how to keep livepatches maintainable and safe. Best Regards, Petr PS: I did not added these patches to v9 of the atomic replace patchset. It was already big enough. And I hope that v9 might be final. In addition, there are no conflicts on the touched files side. In each case, thanks a lot for these nice examples and for finding the bug. -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
