Skip to content

kpatch-build: set replace mode off on default#1310

Closed
robinYin996 wants to merge 1 commit intodynup:masterfrom
robinYin996:master
Closed

kpatch-build: set replace mode off on default#1310
robinYin996 wants to merge 1 commit intodynup:masterfrom
robinYin996:master

Conversation

@robinYin996
Copy link
Copy Markdown

As the kernel commit e1452b607c, replace mode is to revert particular fix, But now all the previous patches are replaced by new livepatch. one patch fix one problem is common scenario,so set replace off on default.

Signed-off-by: yinbinbin yinbinbin001@linux.alibaba.com

As the kernel commit e1452b607c, replace mode is  to revert particular fix,
But now all the previous patches are replaced by new livepatch. one patch
fix one problem is common scenario,so set replace off on default.

Signed-off-by: yinbinbin <yinbinbin001@linux.alibaba.com>
@joe-lawrence
Copy link
Copy Markdown
Contributor

Hi @robinYin996 , thanks for the PR, however, when atomic replace support was added to kpatch-build, it was purposely made the default option. Since the livepatch community generally recommends accumulative patches over many independent ones (simplifies dependencies, smaller potential test matrix, etc.), it naturally follows to use replace mode to best support updates.

I'd be curious to hear about your experiences with "one patch, one problem" livepatching strategy -- how many patches do you normally have in flight at any given time?

@robinYin996
Copy link
Copy Markdown
Author

Hi @robinYin996 , thanks for the PR, however, when atomic replace support was added to kpatch-build, it was purposely made the default option. Since the livepatch community generally recommends accumulative patches over many independent ones (simplifies dependencies, smaller potential test matrix, etc.), it naturally follows to use replace mode to best support updates.

I'd be curious to hear about your experiences with "one patch, one problem" livepatching strategy -- how many patches do you normally have in flight at any given time?

Each independent patch fix one issue, some patches solve memory issue, some solve file systems, etc., so we build livepatch separately.
For example, a release kernel version may have 10 ~ 50 independent livepatch, may more than five livepatch running at the same time on one machine,but not installed at the same time。

I think independent livepatch advantages:
1 . independent livepatch have less stop machine time (klp_enable_patch->ftrace_set_filter_ip->stop_machine), But accumulative patches may needs to replace or ftrace all of functions at the same time。
2. If we want to revert particular fix, we just need unload one livepatch(and load new one again)。But accumulative patches needs to replace/ftrace all of functions again

@jpoimboe
Copy link
Copy Markdown
Member

I think independent livepatch advantages: 1 . independent livepatch have less stop machine time (klp_enable_patch->ftrace_set_filter_ip->stop_machine), But accumulative patches may needs to replace or ftrace all of functions at the same time

But ftrace doesn't use stop_machine() anymore, for the major arches.

  1. If we want to revert particular fix, we just need unload one livepatch(and load new one again)。But accumulative patches needs to replace/ftrace all of functions again

Right, this is the main advantage of independent patches: it gives you more flexibility.

The disadvantage is that it adds risk:

  1. If two patches inadvertently patch the same function, bad things can happen, like accidentally disabling a fix or corrupting the system (especially if one of the patches makes a function API or semantic change). And note that even if the source patches don't overlap, the compiled patches still might overlap unexpectedly, due to compiler decisions.

  2. The test matrix is combinatorial and, given enough patches, can be difficult-to-impossible to verify.

That's why we recommend (and default to) the less risky option: replace.

@jpoimboe jpoimboe closed this Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants