Messages in this thread | | | Date | Wed, 26 Nov 2014 09:27:59 -0600 | From | Josh Poimboeuf <> | Subject | Re: [PATCHv4 0/3] Kernel Live Patching |
| |
On Wed, Nov 26, 2014 at 10:18:24AM +0100, Jiri Kosina wrote: > On Wed, 26 Nov 2014, Masami Hiramatsu wrote: > > > > Note to Steve: > > > Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, > > > I'll rebase and make the change to set IPMODIFY. Do not pull this for -next > > > yet. This version (v4) is for review and gathering acks. > > > > BTW, as we discussed IPMODIFY is an exclusive flag. So if we allocate > > ftrace_ops for each function in each patch, it could be conflict each > > other. > > Yup, this corresponds to what Petr brought up yesterday. There are cases > where all solutions (kpatch, kgraft, klp) would allocate multiple > ftrace_ops for a single function entry (think of patching one function > multiple times in a row). > > So it's not as easy as just setting the flag. > > > Maybe we need to have another ops hashtable to find such conflict and > > new handler to handle it. > > If I understand your proposal correctly, that would sound like a hackish > workaround, trying to basically trick the IPMODIFY flag semantics you just > implemented :)
I think Masami may be proposing something similar to what we do in kpatch today. We have a single ftrace_ops and handler which is used for all functions. The handler accesses a global hash of kpatch_func structs which is indexed by the original function's IP address.
It actually works out pretty well because it nicely encapsulates the knowledge about which functions are patched in a single place. And it makes it easy to track function versions (for incremental patching and rollback).
> What I'd propose instead is to make sure that we always have > just a ftrace_ops per function entry, and only update the pointers there > as necessary. Fortunately we can do the switch atomically, by making use > of ->private.
But how would you update multiple functions atomically, to enforce per-thread consistency?
-- Josh
| |