[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Y+9Fea1svbbbkZ3k@alley>
Date: Fri, 17 Feb 2023 10:14:33 +0100
From: Petr Mladek <pmladek@...e.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
Seth Forshee <sforshee@...italocean.com>,
Peter Zijlstra <peterz@...radead.org>,
Song Liu <song@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Joe Lawrence <joe.lawrence@...hat.com>,
Miroslav Benes <mbenes@...e.cz>,
Jiri Kosina <jikos@...nel.org>, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH 2/3] livepatch,sched: Add livepatch task switching to
cond_resched()
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
Powered by blists - more mailing lists