Re: [RFC v1 02/14] workqueue: Add a lazy version of queue_rcu_work()

From: Joel Fernandes <joel@joelfernandes.org>
To: "Paul E. McKenney" <paulmck@kernel.org>
Cc: rcu@vger.kernel.org, rushikesh.s.kadam@intel.com,
	urezki@gmail.com, neeraj.iitr10@gmail.com, frederic@kernel.org,
	rostedt@goodmis.org
Subject: Re: [RFC v1 02/14] workqueue: Add a lazy version of queue_rcu_work()
Date: Sat, 14 May 2022 14:44:27 +0000	[thread overview]
Message-ID: <Yn/ASx54ooFbhb0R@google.com> (raw)
In-Reply-To: <20220512235818.GV1790663@paulmck-ThinkPad-P17-Gen-1>

On Thu, May 12, 2022 at 04:58:18PM -0700, Paul E. McKenney wrote:
> On Thu, May 12, 2022 at 03:04:30AM +0000, Joel Fernandes (Google) wrote:
> > This will be used in kfree_rcu() later to make it do call_rcu() lazily.
> 
> Given that kfree_rcu() does its own laziness in filling in the page
> of pointers, do we really need to add additional laziness?
> 
> Or are you measuring a significant benefit from doing this?

The benefit I was measuring was, prior to this, I was seeing frequent calls
to queue_delay_rcu(). Based on the latest test results, though, I am not
seeing a signficant benefit and Vlad also confirms it. So I might drop this
patch completely. For easier testing though, it would be nice to make
KFREE_DRAIN_JIFFIES and mainline that change as well, since I believe we can
achieve similar effect by just increasing that timeout.

thanks!

 - Joel

> 
> 							Thanx, Paul
> 
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> >  include/linux/workqueue.h |  1 +
> >  kernel/workqueue.c        | 25 +++++++++++++++++++++++++
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> > index 7fee9b6cfede..2678a6b5b3f3 100644
> > --- a/include/linux/workqueue.h
> > +++ b/include/linux/workqueue.h
> > @@ -444,6 +444,7 @@ extern bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
> >  extern bool mod_delayed_work_on(int cpu, struct workqueue_struct *wq,
> >  			struct delayed_work *dwork, unsigned long delay);
> >  extern bool queue_rcu_work(struct workqueue_struct *wq, struct rcu_work *rwork);
> > +extern bool queue_rcu_work_lazy(struct workqueue_struct *wq, struct rcu_work *rwork);
> >  
> >  extern void flush_workqueue(struct workqueue_struct *wq);
> >  extern void drain_workqueue(struct workqueue_struct *wq);
> > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > index 33f1106b4f99..9444949cc148 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
> > @@ -1796,6 +1796,31 @@ bool queue_rcu_work(struct workqueue_struct *wq, struct rcu_work *rwork)
> >  }
> >  EXPORT_SYMBOL(queue_rcu_work);
> >  
> > +/**
> > + * queue_rcu_work_lazy - queue work after a RCU grace period
> > + * @wq: workqueue to use
> > + * @rwork: work to queue
> > + *
> > + * Return: %false if @rwork was already pending, %true otherwise.  Note
> > + * that a full RCU grace period is guaranteed only after a %true return.
> > + * While @rwork is guaranteed to be executed after a %false return, the
> > + * execution may happen before a full RCU grace period has passed.
> > + */
> > +bool queue_rcu_work_lazy(struct workqueue_struct *wq, struct rcu_work *rwork)
> > +{
> > +	struct work_struct *work = &rwork->work;
> > +
> > +	if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))) {
> > +		rwork->wq = wq;
> > +		call_rcu_lazy(&rwork->rcu, rcu_work_rcufn);
> > +		return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +EXPORT_SYMBOL(queue_rcu_work_lazy);
> > +
> > +
> >  /**
> >   * worker_enter_idle - enter idle state
> >   * @worker: worker which is entering idle state
> > -- 
> > 2.36.0.550.gb090851708-goog
> > 