On Wed, Apr 11, 2018 at 10:07:31AM +0200, Miroslav Benes wrote: > > > I was confused by wording "in the middle". It suggested that there > > > might had been enabled patches on the top and the bottom of the stack > > > and some disabled patches in between at the same time (or vice versa). > > > This was not true. > > > > That *was* what I meant. Consider the following sequence of events: > > > > - Register patch 1 > > - Enable patch 1 > > - Register patch 2 > > - Enable patch 2 > > - Disable patch 2 > > - Register patch 3 > > - Enable patch 3 > > > > Notice that patch 2 (in the middle) is disabled, whereas patch 1 (on the > > bottom) and patch 3 (on the top) are enabled. > > This should not be possible at all. > > __klp_enable_patch: > > if (patch->list.prev != &klp_patches && > !list_prev_entry(patch, list)->enabled) > return -EBUSY; > > When patch 3 is enabled, list_prev_entry() returns patch 2 and its > ->enabled is false. Hm, you're right. I'm not sure how I got that idea... I still agree with my original conclusion that enforcing stack order no longer makes sense though. > > > Another possibility would be to get rid of the enable/disable states. > > > I mean that the patch will be automatically enabled during > > > registration and removed during unregistration. > > > > I don't see how disabling during unregistration would be possible, since > > the unregister is called from the patch module exit function, which > > can only be called *after* the patch is disabled. > > > > However, we could unregister immediately after disabling (i.e., in > > enabled_store context). > > I think this is what Petr meant. So there would be nothing in the patch > module exit function. Well, not exactly. We'd need to remove sysfs dir and > maybe something more. Sounds good to me, though aren't the livepatch sysfs entries removed by klp during unregister? > > > The question is what is acceptable to others > > > > If there are any objections, this is their chance to speak up :-) > > > > > and if it needs to be done as part of this patch set. > > > > Maybe so, for at least a few reasons: > > > > - This patch set makes the 'stack' obsolete, so it makes sense to remove > > the 'stack' with it. > > Not necessarily. I like Petr's rebase explanation here. I'm not sure what you mean. IIRC, his rebase explanation referred to how we handle 'replace' patches, for which there is no stacking (as I meant the term: enforcement of stack order for registration and enablement). -- Josh -- 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
