On Thu, Feb 13, 2025 at 9:58 AM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
>
> On Wed, Feb 12, 2025 at 10:42:33AM +0800, Yafang Shao wrote:
> > On Wed, Feb 12, 2025 at 8:52 AM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> > >
> > > On Tue, Feb 11, 2025 at 02:24:37PM +0800, Yafang Shao wrote:
> > > > +++ b/kernel/livepatch/transition.c
> > > > @@ -491,9 +491,18 @@ void klp_try_complete_transition(void)
> > > > complete = false;
> > > > break;
> > > > }
> > > > +
> > > > + /* Avoid potential RCU stalls */
> > > > + if (need_resched()) {
> > > > + complete = false;
> > > > + break;
> > > > + }
> > > > }
> > > > read_unlock(&tasklist_lock);
> > > >
> > > > + /* The above operation might be expensive. */
> > > > + cond_resched();
> > > > +
> > >
> > > This is also nasty, yet another reason to use rcu_read_lock() if we can.
> >
> > The RCU stalls still happen even if we use rcu_read_lock() as it is
> > still in the RCU read-side critical section.
> >
> > >
> > > Also, with the new lazy preemption model, I believe cond_resched() is
> > > pretty much deprecated.
> >
> > I'm not familiar with the newly introduced PREEMPT_LAZY, but it
> > appears to be a configuration option. Therefore, we still need this
> > cond_resched() for users who don't have PREEMPT_LAZY set as the
> > default.
>
> IIRC, the goal is to get rid of PREEMPT_NONE and PREEMPT_VOLUNTARY (and
> PREEMPT_DYNAMIC) and to remove almost all the cond_resched() calls. So
> we should really avoid adding them at this point.
>
> The patch already breaks out of the loop for need_resched(), is the
> cond_resched() really needed there? For the PREEMPT_VOLUNTARY case I
> think it should already get preempted anyway when it releases the lock.
>
> And regardless, by that point it's fairly close to scheduling the
> delayed work and returning back to the user. That could happen even
> sooner by skipping the "swapper" task loop.
You're correct. I’ve verified that it can avoid the RCU stalls even
without the cond_resched().
--
Regards
Yafang
