Re: [PATCH 2/3] livepatch,sched: Add livepatch task switching to cond_resched()

[Petr Mladek]

On Wed 2023-02-15 18:26:30, Josh Poimboeuf wrote:
> On Wed, Feb 15, 2023 at 02:30:36PM +0100, Petr Mladek wrote:
> > >  static inline int _cond_resched(void)
> > >  {
> > > +	klp_sched_try_switch();
> > >  	return __cond_resched();
> > 
> > My only concern is if it might cause any performance problems.
> > 
> > On one hand, cond_resched() is used in code paths that are slow
> > on its own. Also it will do nothing most of the time.
> > 
> > On the other hand, cond_resched() is typically used in cycles.
> > One cycle might be fast. The code might be slow because there
> > are too many cycles. Repeating the same failing test might
> > prolong the time significantly.
> 
> Yes, but it should hopefully be very rare to patch a function in the
> call stack of a kthread loop.  In general it's a good idea for the patch
> author to avoid that.

I do not have any data about it.

> > An idea is to try the switch only when it was not done during
> > a real schedule. Something like:
> > 
> > static inline int _cond_resched(void)
> > {
> > 	int scheduled;
> > 
> > 	scheduled = __cond_resched();
> > 	if (scheduled)
> > 		klp_sched_try_switch();
> > 
> > 	return scheduled();
> > }
> > 
> > But it would make it less reliable/predictable. Also it won't work
> > in configurations when cond_resched() is always a nop.
> > 
> > I am probably too careful. We might keep it simple until any real
> > life problems are reported.
> 
> If we can get away with it, I much prefer the simple unconditional
> klp_sched_try_switch() because of the predictability and quickness with
> which the kthread gets patched.

I am fine with keeping it simple now. We could always change it when
it causes problems.

I primary wanted to point out the potential problem and check what
others think about it.

Best Regards,
Petr