Re: [PATCH 1/2] livepatch: Send a fake signal periodically

[Petr Mladek]

On Wed 2018-06-06 09:49:50, Miroslav Benes wrote:
> On Tue, 5 Jun 2018, Josh Poimboeuf wrote:
> 
> > On Tue, Jun 05, 2018 at 09:17:52AM +0200, Miroslav Benes wrote:
> > > On Mon, 4 Jun 2018, Josh Poimboeuf wrote:
> > > 
> > > > On Mon, Jun 04, 2018 at 04:16:35PM +0200, Miroslav Benes wrote:
> > > > > An administrator may send a fake signal to all remaining blocking tasks
> > > > > of a running transition by writing to
> > > > > /sys/kernel/livepatch/<patch>/signal attribute. Let's do it
> > > > > automatically after 10 seconds. The timeout is chosen deliberately. It
> > > > > gives the tasks enough time to transition themselves.
> > > > > 
> > > > > Theoretically, sending it once should be more than enough. Better be safe
> > > > > than sorry, so send it periodically.
> > > > 
> > > > This is the part I don't understand.  Why do it periodically?
> > > 
> > > I met (rare!) cases when doing it once was not enough due to a race and 
> > > the signal was missed. However involved testcases were really artificial.
> > >  
> > > > Instead, might it make sense to just send the signals once, and if that
> > > > doesn't work, reverse the transition?  Then we could make patching a
> > > > synchronous operation.  But then, it might be remotely possible that the
> > > > reverse operation also stalls (e.g., on a kthread).  So, maybe it's best
> > > > to just leave all these controls in the hands of the user.
> > > 
> > > And there is 'force' option...
> > > 
> > > So given all this, I'd call klp_send_signals() once and then leave it up 
> > > to the user. Would that work for you?
> > 
> > Well, I don't know.  Since the patching process will already need to be
> > managed by user space, what's the benefit of having the kernel doing
> > only this part of it?
> 
> I'm not sure about 'will already need to be managed by user space' part. 
> Sending the fake signal would unblock transition in certain cases "for 
> free". No user interaction is really needed and we can do it 
> automatically. That's why I find it beneficial.

I wonder if several kicks might be necessary to get a kthread patched.

BTW: This approach has a bit different effect in compare with kGraft.
In kGraft, it was enough to force kthread going over an explicitly
defined safe point where it migrated itself. In upstream, the kthread
must get scheduled outside any patched function. The chance might
be a bit random.

To make it clear. I believe that sending signal once makes sense for
sure. And it would make sense to even repeat it.

BTW: If we allow to repeat it, we should reset the counter in
klp_reverse_transition().

Best Regards,
Petr