diff mbox series

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

Message ID 20220512030442.2530552-3-joel@joelfernandes.org (mailing list archive)
State Superseded
Headers show
Series Implement call_rcu_lazy() and miscellaneous fixes | expand

Commit Message

Joel Fernandes May 12, 2022, 3:04 a.m. UTC
This will be used in kfree_rcu() later to make it do call_rcu() lazily.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 include/linux/workqueue.h |  1 +
 kernel/workqueue.c        | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

Comments

Paul E. McKenney May 12, 2022, 11:58 p.m. UTC | #1
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?

							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
>
Joel Fernandes May 14, 2022, 2:44 p.m. UTC | #2
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
> >
diff mbox series

Patch

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