Message ID | 20220512030442.2530552-3-joel@joelfernandes.org (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Implement call_rcu_lazy() and miscellaneous fixes | expand |
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 >
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 --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
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(+)