Re: [PATCH v14 05/11] livepatch: Simplify API by removing registration step

[Petr Mladek]

On Thu 2018-12-06 10:23:40, Miroslav Benes wrote:
> On Thu, 6 Dec 2018, Petr Mladek wrote:
> 
> > On Wed 2018-12-05 14:32:53, Joe Lawrence wrote:
> > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > > > index 972520144713..e01dfa3b58d2 100644
> > > > --- a/kernel/livepatch/core.c
> > > > +++ b/kernel/livepatch/core.c
> > > > @@ -45,7 +45,7 @@
> > > >   */
> > > >  DEFINE_MUTEX(klp_mutex);
> > > >  
> > > > -/* Registered patches */
> > > > +/* Actively used patches. */
> > > >  LIST_HEAD(klp_patches);
> > > 
> > > By itself, this comment makes me wonder if there are un-active and/or
> > > un-used patches that I need to worry about.  After this patchset,
> > > klp_patches will include patches that have been enabled and those that
> > > have been replaced, but the replacement transition is still in progress.  
> > > 
> > > If that sounds accurate, how about adding to the comment:
> > > 
> > > /* Actively used patches: enabled or replaced and awaiting transition */
> > 
> > The replaced patches are not in the list. This is why I used the word
> > "actively".
> 
> The replaced patches are removed in klp_discard_replaced_patches(), which 
> is called from klp_complete_transition(). Joe is right. The patches are in 
> the list if a transition is still in progress.

These are patches that are being replaced. The replaced (after the
 transition finishes) are not in the list.

By other word, Joe's text could be understand that replaced patches
will never get removed from the list.

So, is the text below acceptable?

/*
 * Actively used patches: enabled or in transition. Note that replaced
 * or disabled patches are not listed even though the related kernel
 * module still can be loaded.
 */

Or anyone has a better one?

Best Regards,
Petr