Re: [PATCH v3 1/2] livepatch: Add dynamic klp_object and klp_func iterators
From: Josh Poimboeuf
Date: Fri Oct 06 2017 - 17:24:05 EST
On Wed, Sep 27, 2017 at 11:41:29PM -0400, Jason Baron wrote:
> In preparation to introducing atomic replace, introduce iterators for
> klp_func and klp_object, such that objects and functions can be dynamically
> allocated (needed for atomic replace). This patch is intended to
> effectively be a no-op until atomic replace is introduced.
Sorry for coming in so late on this. This will be a nice feature. I
haven't gotten to the second patch yet, but I have a few comments
1) Having both static *and* dynamic lists seems to add a lot of
complexity and brittleness.
The livepatch API is easier to use with static data structures, so I
don't think we should change the API. But I also don't think we
should allow the API to overly constrain our internal representation
of the data.
What I'd *really* like to do is completely separate the user-supplied
data structures from our internal data . At registration time, we
can convert all the arrays to dynamic lists, and then never touch the
arrays again. Then managing the lists is completely straightforward.
That would also allow us to have an external public struct and an
internal private struct, so the interface is more clear and we don't
have to worry about any patch modules accidentally messing with our
2) There seems to be a general consensus that the cumulative "replace"
model is the best one. In light of that, I wonder if we can simplify
our code a bit. Specifically, can we get rid of the func stack?
If the user always uses replace, then the func stack will never have
more than one func. And we can reject a patch if the user doesn't
use replace when they're trying to patch a function which has already
Then the func stack doesn't have to be a stack, it can just be a
That would allow us to simplify the code in several places for the
common case, yet still allow those who want to apply non-cumulative
patches to do so most of the time.
It's not a *huge* simplification, but we've already made livepatch so
flexible that it's too complex to fit everything in your head at the
same time, and any simplification we can get away with is well worth
it IMO. And once we have replace, I wonder if anybody would really
miss the func stack.