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
