Re: [PATCH 2/2] kernel: add support for live patching

[Josh Poimboeuf]

On Thu, Nov 06, 2014 at 10:57:48AM -0600, Seth Jennings wrote:
> On Thu, Nov 06, 2014 at 04:51:02PM +0100, Jiri Slaby wrote:
> > On 11/06/2014, 03:39 PM, Seth Jennings wrote:
> > > +/* must be called with lpc_mutex held */
> > > +static int lpc_enable_patch(struct lpc_patch *patch)
> > 
> > The question I want to raise here is whether we need two-state
> > registration: register+enable. We don't in kGraft. Why do you?
> 
> We actually don't in kpatch either and this was a late change for this
> patchset.  The thinking was that, while the patch modules would normally
> call lpc_register_patch() and lpc_enable_patch() in the same way all the
> time, breaking them up created more symmetric code and gives more flexibility
> to the API.
> 
> Josh might like to elaborate here.

Yes, this was my brilliant idea :-)  I like it because it makes the
register/unregister interfaces more symmetrical.

We already have to separate disable and unregister so that a patch can
be disabled from sysfs, so it makes sense IMO to likewise separate
register and enable.

The downside is an extra function call.  The upside is it makes the code
cleaner, and the API easier to understand and more flexible.

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/