Re: [PATCH tip/core/rcu 2/3] rcu: Provide polling interfaces for Tiny RCU grace periods
On Tue, Mar 23, 2021 at 03:02:07PM +0100, Frederic Weisbecker wrote: > On Mon, Mar 22, 2021 at 12:45:22PM -0700, Paul E. McKenney wrote: > > On Mon, Mar 22, 2021 at 08:00:35PM +0100, Frederic Weisbecker wrote: > > > But poll_state_synchronize_rcu() checks that the gp_num has changed, > > > which is not needed for cond_synchronize_rcu() since this it is > > > only allowed to be called from a QS. > > > > Good catch, and thank you! Back to a single might_sleep() it is! > > And then: Reviewed-by: Frederic Weisbecker Thank you! I will apply this on my next rebase. Thanx, Paul
Re: [PATCH tip/core/rcu 2/3] rcu: Provide polling interfaces for Tiny RCU grace periods
On Mon, Mar 22, 2021 at 12:45:22PM -0700, Paul E. McKenney wrote: > On Mon, Mar 22, 2021 at 08:00:35PM +0100, Frederic Weisbecker wrote: > > But poll_state_synchronize_rcu() checks that the gp_num has changed, > > which is not needed for cond_synchronize_rcu() since this it is > > only allowed to be called from a QS. > > Good catch, and thank you! Back to a single might_sleep() it is! And then: Reviewed-by: Frederic Weisbecker Thanks!
Re: [PATCH tip/core/rcu 2/3] rcu: Provide polling interfaces for Tiny RCU grace periods
On Mon, Mar 22, 2021 at 08:00:35PM +0100, Frederic Weisbecker wrote: > On Mon, Mar 22, 2021 at 08:47:44AM -0700, Paul E. McKenney wrote: > > On Sun, Mar 21, 2021 at 11:28:55PM +0100, Frederic Weisbecker wrote: > > > On Wed, Mar 03, 2021 at 04:26:31PM -0800, paul...@kernel.org wrote: > > > > From: "Paul E. McKenney" > > > > > > > > There is a need for a non-blocking polling interface for RCU grace > > > > periods, so this commit supplies start_poll_synchronize_rcu() and > > > > poll_state_synchronize_rcu() for this purpose. Note that the existing > > > > get_state_synchronize_rcu() may be used if future grace periods are > > > > inevitable (perhaps due to a later call_rcu() invocation). The new > > > > start_poll_synchronize_rcu() is to be used if future grace periods > > > > might not otherwise happen. Finally, poll_state_synchronize_rcu() > > > > provides a lockless check for a grace period having elapsed since > > > > the corresponding call to either of the get_state_synchronize_rcu() > > > > or start_poll_synchronize_rcu(). > > > > > > > > As with get_state_synchronize_rcu(), the return value from either > > > > get_state_synchronize_rcu() or start_poll_synchronize_rcu() is passed in > > > > to a later call to either poll_state_synchronize_rcu() or the existing > > > > (might_sleep) cond_synchronize_rcu(). > > > > > > > > Signed-off-by: Paul E. McKenney > > > > --- > > > > include/linux/rcutiny.h | 11 ++- > > > > kernel/rcu/tiny.c | 40 > > > > 2 files changed, 46 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h > > > > index 2a97334..69108cf4 100644 > > > > --- a/include/linux/rcutiny.h > > > > +++ b/include/linux/rcutiny.h > > > > @@ -17,14 +17,15 @@ > > > > /* Never flag non-existent other CPUs! */ > > > > static inline bool rcu_eqs_special_set(int cpu) { return false; } > > > > > > > > -static inline unsigned long get_state_synchronize_rcu(void) > > > > -{ > > > > - return 0; > > > > -} > > > > +unsigned long get_state_synchronize_rcu(void); > > > > +unsigned long start_poll_synchronize_rcu(void); > > > > +bool poll_state_synchronize_rcu(unsigned long oldstate); > > > > > > > > static inline void cond_synchronize_rcu(unsigned long oldstate) > > > > { > > > > - might_sleep(); > > > > + if (poll_state_synchronize_rcu(oldstate)) > > > > + return; > > > > + synchronize_rcu(); > > > > > > Perhaps cond_synchronize_rcu() could stay as it was. If it might > > > call synchronize_rcu() then it inherits its constraint to be > > > called from a quiescent state. > > > > As in leave the might_sleep()? How about something like this? > > > > static inline void cond_synchronize_rcu(unsigned long oldstate) > > { > > if (!poll_state_synchronize_rcu(oldstate)) > > synchronize_rcu(); > > else > > might_sleep(); > > } > > > > One advantage of this is that the Tiny and Tree implementations > > become identical and can then be consolidated. > > > > Or did I miss your point? > > But poll_state_synchronize_rcu() checks that the gp_num has changed, > which is not needed for cond_synchronize_rcu() since this it is > only allowed to be called from a QS. Good catch, and thank you! Back to a single might_sleep() it is! Thanx, Paul
Re: [PATCH tip/core/rcu 2/3] rcu: Provide polling interfaces for Tiny RCU grace periods
On Mon, Mar 22, 2021 at 08:47:44AM -0700, Paul E. McKenney wrote: > On Sun, Mar 21, 2021 at 11:28:55PM +0100, Frederic Weisbecker wrote: > > On Wed, Mar 03, 2021 at 04:26:31PM -0800, paul...@kernel.org wrote: > > > From: "Paul E. McKenney" > > > > > > There is a need for a non-blocking polling interface for RCU grace > > > periods, so this commit supplies start_poll_synchronize_rcu() and > > > poll_state_synchronize_rcu() for this purpose. Note that the existing > > > get_state_synchronize_rcu() may be used if future grace periods are > > > inevitable (perhaps due to a later call_rcu() invocation). The new > > > start_poll_synchronize_rcu() is to be used if future grace periods > > > might not otherwise happen. Finally, poll_state_synchronize_rcu() > > > provides a lockless check for a grace period having elapsed since > > > the corresponding call to either of the get_state_synchronize_rcu() > > > or start_poll_synchronize_rcu(). > > > > > > As with get_state_synchronize_rcu(), the return value from either > > > get_state_synchronize_rcu() or start_poll_synchronize_rcu() is passed in > > > to a later call to either poll_state_synchronize_rcu() or the existing > > > (might_sleep) cond_synchronize_rcu(). > > > > > > Signed-off-by: Paul E. McKenney > > > --- > > > include/linux/rcutiny.h | 11 ++- > > > kernel/rcu/tiny.c | 40 > > > 2 files changed, 46 insertions(+), 5 deletions(-) > > > > > > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h > > > index 2a97334..69108cf4 100644 > > > --- a/include/linux/rcutiny.h > > > +++ b/include/linux/rcutiny.h > > > @@ -17,14 +17,15 @@ > > > /* Never flag non-existent other CPUs! */ > > > static inline bool rcu_eqs_special_set(int cpu) { return false; } > > > > > > -static inline unsigned long get_state_synchronize_rcu(void) > > > -{ > > > - return 0; > > > -} > > > +unsigned long get_state_synchronize_rcu(void); > > > +unsigned long start_poll_synchronize_rcu(void); > > > +bool poll_state_synchronize_rcu(unsigned long oldstate); > > > > > > static inline void cond_synchronize_rcu(unsigned long oldstate) > > > { > > > - might_sleep(); > > > + if (poll_state_synchronize_rcu(oldstate)) > > > + return; > > > + synchronize_rcu(); > > > > Perhaps cond_synchronize_rcu() could stay as it was. If it might > > call synchronize_rcu() then it inherits its constraint to be > > called from a quiescent state. > > As in leave the might_sleep()? How about something like this? > > static inline void cond_synchronize_rcu(unsigned long oldstate) > { > if (!poll_state_synchronize_rcu(oldstate)) > synchronize_rcu(); > else > might_sleep(); > } > > One advantage of this is that the Tiny and Tree implementations > become identical and can then be consolidated. > > Or did I miss your point? But poll_state_synchronize_rcu() checks that the gp_num has changed, which is not needed for cond_synchronize_rcu() since this it is only allowed to be called from a QS. > > Thanx, Paul
Re: [PATCH tip/core/rcu 2/3] rcu: Provide polling interfaces for Tiny RCU grace periods
On Sun, Mar 21, 2021 at 11:28:55PM +0100, Frederic Weisbecker wrote: > On Wed, Mar 03, 2021 at 04:26:31PM -0800, paul...@kernel.org wrote: > > From: "Paul E. McKenney" > > > > There is a need for a non-blocking polling interface for RCU grace > > periods, so this commit supplies start_poll_synchronize_rcu() and > > poll_state_synchronize_rcu() for this purpose. Note that the existing > > get_state_synchronize_rcu() may be used if future grace periods are > > inevitable (perhaps due to a later call_rcu() invocation). The new > > start_poll_synchronize_rcu() is to be used if future grace periods > > might not otherwise happen. Finally, poll_state_synchronize_rcu() > > provides a lockless check for a grace period having elapsed since > > the corresponding call to either of the get_state_synchronize_rcu() > > or start_poll_synchronize_rcu(). > > > > As with get_state_synchronize_rcu(), the return value from either > > get_state_synchronize_rcu() or start_poll_synchronize_rcu() is passed in > > to a later call to either poll_state_synchronize_rcu() or the existing > > (might_sleep) cond_synchronize_rcu(). > > > > Signed-off-by: Paul E. McKenney > > --- > > include/linux/rcutiny.h | 11 ++- > > kernel/rcu/tiny.c | 40 > > 2 files changed, 46 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h > > index 2a97334..69108cf4 100644 > > --- a/include/linux/rcutiny.h > > +++ b/include/linux/rcutiny.h > > @@ -17,14 +17,15 @@ > > /* Never flag non-existent other CPUs! */ > > static inline bool rcu_eqs_special_set(int cpu) { return false; } > > > > -static inline unsigned long get_state_synchronize_rcu(void) > > -{ > > - return 0; > > -} > > +unsigned long get_state_synchronize_rcu(void); > > +unsigned long start_poll_synchronize_rcu(void); > > +bool poll_state_synchronize_rcu(unsigned long oldstate); > > > > static inline void cond_synchronize_rcu(unsigned long oldstate) > > { > > - might_sleep(); > > + if (poll_state_synchronize_rcu(oldstate)) > > + return; > > + synchronize_rcu(); > > Perhaps cond_synchronize_rcu() could stay as it was. If it might > call synchronize_rcu() then it inherits its constraint to be > called from a quiescent state. As in leave the might_sleep()? How about something like this? static inline void cond_synchronize_rcu(unsigned long oldstate) { if (!poll_state_synchronize_rcu(oldstate)) synchronize_rcu(); else might_sleep(); } One advantage of this is that the Tiny and Tree implementations become identical and can then be consolidated. Or did I miss your point? Thanx, Paul
Re: [PATCH tip/core/rcu 2/3] rcu: Provide polling interfaces for Tiny RCU grace periods
On Wed, Mar 03, 2021 at 04:26:31PM -0800, paul...@kernel.org wrote: > From: "Paul E. McKenney" > > There is a need for a non-blocking polling interface for RCU grace > periods, so this commit supplies start_poll_synchronize_rcu() and > poll_state_synchronize_rcu() for this purpose. Note that the existing > get_state_synchronize_rcu() may be used if future grace periods are > inevitable (perhaps due to a later call_rcu() invocation). The new > start_poll_synchronize_rcu() is to be used if future grace periods > might not otherwise happen. Finally, poll_state_synchronize_rcu() > provides a lockless check for a grace period having elapsed since > the corresponding call to either of the get_state_synchronize_rcu() > or start_poll_synchronize_rcu(). > > As with get_state_synchronize_rcu(), the return value from either > get_state_synchronize_rcu() or start_poll_synchronize_rcu() is passed in > to a later call to either poll_state_synchronize_rcu() or the existing > (might_sleep) cond_synchronize_rcu(). > > Signed-off-by: Paul E. McKenney > --- > include/linux/rcutiny.h | 11 ++- > kernel/rcu/tiny.c | 40 > 2 files changed, 46 insertions(+), 5 deletions(-) > > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h > index 2a97334..69108cf4 100644 > --- a/include/linux/rcutiny.h > +++ b/include/linux/rcutiny.h > @@ -17,14 +17,15 @@ > /* Never flag non-existent other CPUs! */ > static inline bool rcu_eqs_special_set(int cpu) { return false; } > > -static inline unsigned long get_state_synchronize_rcu(void) > -{ > - return 0; > -} > +unsigned long get_state_synchronize_rcu(void); > +unsigned long start_poll_synchronize_rcu(void); > +bool poll_state_synchronize_rcu(unsigned long oldstate); > > static inline void cond_synchronize_rcu(unsigned long oldstate) > { > - might_sleep(); > + if (poll_state_synchronize_rcu(oldstate)) > + return; > + synchronize_rcu(); Perhaps cond_synchronize_rcu() could stay as it was. If it might call synchronize_rcu() then it inherits its constraint to be called from a quiescent state. Thanks.
[PATCH tip/core/rcu 2/3] rcu: Provide polling interfaces for Tiny RCU grace periods
From: "Paul E. McKenney" There is a need for a non-blocking polling interface for RCU grace periods, so this commit supplies start_poll_synchronize_rcu() and poll_state_synchronize_rcu() for this purpose. Note that the existing get_state_synchronize_rcu() may be used if future grace periods are inevitable (perhaps due to a later call_rcu() invocation). The new start_poll_synchronize_rcu() is to be used if future grace periods might not otherwise happen. Finally, poll_state_synchronize_rcu() provides a lockless check for a grace period having elapsed since the corresponding call to either of the get_state_synchronize_rcu() or start_poll_synchronize_rcu(). As with get_state_synchronize_rcu(), the return value from either get_state_synchronize_rcu() or start_poll_synchronize_rcu() is passed in to a later call to either poll_state_synchronize_rcu() or the existing (might_sleep) cond_synchronize_rcu(). Signed-off-by: Paul E. McKenney --- include/linux/rcutiny.h | 11 ++- kernel/rcu/tiny.c | 40 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h index 2a97334..69108cf4 100644 --- a/include/linux/rcutiny.h +++ b/include/linux/rcutiny.h @@ -17,14 +17,15 @@ /* Never flag non-existent other CPUs! */ static inline bool rcu_eqs_special_set(int cpu) { return false; } -static inline unsigned long get_state_synchronize_rcu(void) -{ - return 0; -} +unsigned long get_state_synchronize_rcu(void); +unsigned long start_poll_synchronize_rcu(void); +bool poll_state_synchronize_rcu(unsigned long oldstate); static inline void cond_synchronize_rcu(unsigned long oldstate) { - might_sleep(); + if (poll_state_synchronize_rcu(oldstate)) + return; + synchronize_rcu(); } extern void rcu_barrier(void); diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c index aa897c3..c8a029f 100644 --- a/kernel/rcu/tiny.c +++ b/kernel/rcu/tiny.c @@ -32,12 +32,14 @@ struct rcu_ctrlblk { struct rcu_head *rcucblist; /* List of pending callbacks (CBs). */ struct rcu_head **donetail; /* ->next pointer of last "done" CB. */ struct rcu_head **curtail; /* ->next pointer of last CB. */ + unsigned long gp_seq; /* Grace-period counter. */ }; /* Definition for rcupdate control block. */ static struct rcu_ctrlblk rcu_ctrlblk = { .donetail = _ctrlblk.rcucblist, .curtail= _ctrlblk.rcucblist, + .gp_seq = 0 - 300UL, }; void rcu_barrier(void) @@ -56,6 +58,7 @@ void rcu_qs(void) rcu_ctrlblk.donetail = rcu_ctrlblk.curtail; raise_softirq_irqoff(RCU_SOFTIRQ); } + WRITE_ONCE(rcu_ctrlblk.gp_seq, rcu_ctrlblk.gp_seq + 1); local_irq_restore(flags); } @@ -177,6 +180,43 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func) } EXPORT_SYMBOL_GPL(call_rcu); +/* + * Return a grace-period-counter "cookie". For more information, + * see the Tree RCU header comment. + */ +unsigned long get_state_synchronize_rcu(void) +{ + return READ_ONCE(rcu_ctrlblk.gp_seq); +} +EXPORT_SYMBOL_GPL(get_state_synchronize_rcu); + +/* + * Return a grace-period-counter "cookie" and ensure that a future grace + * period completes. For more information, see the Tree RCU header comment. + */ +unsigned long start_poll_synchronize_rcu(void) +{ + unsigned long gp_seq = get_state_synchronize_rcu(); + + if (unlikely(is_idle_task(current))) { + /* force scheduling for rcu_qs() */ + resched_cpu(0); + } + return gp_seq; +} +EXPORT_SYMBOL_GPL(start_poll_synchronize_rcu); + +/* + * Return true if the grace period corresponding to oldstate has completed + * and false otherwise. For more information, see the Tree RCU header + * comment. + */ +bool poll_state_synchronize_rcu(unsigned long oldstate) +{ + return READ_ONCE(rcu_ctrlblk.gp_seq) != oldstate; +} +EXPORT_SYMBOL_GPL(poll_state_synchronize_rcu); + void __init rcu_init(void) { open_softirq(RCU_SOFTIRQ, rcu_process_callbacks); -- 2.9.5