Hello!
This series provides a polling interface for SRCU grace periods. The
API is as follows:
unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
Returns a "cookie" that can be thought of as a snapshot of
the specified SRCU instance's grace-period sequence number.
Also ensures that enough future grace periods happen to eventually
make the grace-period sequence number reach the cookie.
bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
Given a cookie from start_poll_synchronize_srcu(), returns true if
at least one full SRCU grace period has elapsed in the meantime.
Given finite SRCU readers in a well-behaved kernel, the following
code will complete in finite time:
cookie = start_poll_synchronize_srcu(&my_srcu);
while (!poll_state_synchronize_srcu(&my_srcu, cookie))
schedule_timeout_uninterruptible(1);
unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
Like start_poll_synchronize_srcu(), except that it does not start
any grace periods. This means that the following code is -not-
guaranteed to complete:
cookie = get_state_synchronize_srcu(&my_srcu);
while (!poll_state_synchronize_srcu(&my_srcu, cookie))
schedule_timeout_uninterruptible(1);
Use this if you know that something else will be starting the
needed SRCU grace periods. This might also be useful if you
had items that were likely to be reused before the SRCU grace
period elapsed, so that you avoid burning CPU on SRCU grace
periods that prove to be unnecessary. Or if you don't want
to have more than (say) 100 SRCU grace periods per seconds,
in which case you might use a timer to start the grace periods.
Or maybe you don't bother starting the SRCU grace period until
some sort of emergency situation has arisen. Or...
OK, maybe no one needs it, but rcutorture does need it, so here
it is anyway.
The patches in this series are as follows:
1. Make Tiny SRCU use multi-bit grace-period counter.
2. Provide internal interface to start a Tiny SRCU grace period.
3. Provide internal interface to start a Tree SRCU grace period.
4. Provide polling interfaces for Tiny SRCU grace periods.
5. Provide polling interfaces for Tree SRCU grace periods.
Thanx, Paul
------------------------------------------------------------------------
include/linux/rcupdate.h | 2
include/linux/srcu.h | 3 +
include/linux/srcutiny.h | 5 +
kernel/rcu/srcutiny.c | 74 +++++++++++++++++++++++---
kernel/rcu/srcutree.c | 129 +++++++++++++++++++++++++++++++++++------------
5 files changed, 169 insertions(+), 44 deletions(-)
From: "Paul E. McKenney" <[email protected]>
There is a need for a polling interface for SRCU grace periods. This
polling needs to distinguish between an SRCU instance being idle on the
one hand or in the middle of a grace period on the other. This commit
therefore converts the Tiny SRCU srcu_struct structure's srcu_idx from
a defacto boolean to a free-running counter, using the bottom bit to
indicate that a grace period is in progress. The second-from-bottom
bit is thus used as the index returned by srcu_read_lock().
Link: https://lore.kernel.org/rcu/[email protected]/
Reported-by: Kent Overstreet <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
---
include/linux/srcutiny.h | 4 ++--
kernel/rcu/srcutiny.c | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
index 5a5a194..fed4a2d 100644
--- a/include/linux/srcutiny.h
+++ b/include/linux/srcutiny.h
@@ -15,7 +15,7 @@
struct srcu_struct {
short srcu_lock_nesting[2]; /* srcu_read_lock() nesting depth. */
- short srcu_idx; /* Current reader array element. */
+ unsigned short srcu_idx; /* Current reader array element in bit 0x2. */
u8 srcu_gp_running; /* GP workqueue running? */
u8 srcu_gp_waiting; /* GP waiting for readers? */
struct swait_queue_head srcu_wq;
@@ -59,7 +59,7 @@ static inline int __srcu_read_lock(struct srcu_struct *ssp)
{
int idx;
- idx = READ_ONCE(ssp->srcu_idx);
+ idx = (READ_ONCE(ssp->srcu_idx) & 0x2) / 2;
WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
return idx;
}
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index 6208c1d..5598cf6 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -124,11 +124,12 @@ void srcu_drive_gp(struct work_struct *wp)
ssp->srcu_cb_head = NULL;
ssp->srcu_cb_tail = &ssp->srcu_cb_head;
local_irq_enable();
- idx = ssp->srcu_idx;
- WRITE_ONCE(ssp->srcu_idx, !ssp->srcu_idx);
+ idx = (ssp->srcu_idx & 0x2) / 2;
+ WRITE_ONCE(ssp->srcu_idx, ssp->srcu_idx + 1);
WRITE_ONCE(ssp->srcu_gp_waiting, true); /* srcu_read_unlock() wakes! */
swait_event_exclusive(ssp->srcu_wq, !READ_ONCE(ssp->srcu_lock_nesting[idx]));
WRITE_ONCE(ssp->srcu_gp_waiting, false); /* srcu_read_unlock() cheap. */
+ WRITE_ONCE(ssp->srcu_idx, ssp->srcu_idx + 1);
/* Invoke the callbacks we removed above. */
while (lh) {
--
2.9.5
From: "Paul E. McKenney" <[email protected]>
There is a need for a polling interface for SRCU grace periods.
This polling needs to initiate an SRCU grace period without having
to queue (and manage) a callback. This commit therefore splits the
Tree SRCU __call_srcu() function into callback-initialization and
queuing/start-grace-period portions, with the latter in a new function
named srcu_gp_start_if_needed(). This function may be passed a NULL
callback pointer, in which case it will refrain from queuing anything.
Why have the new function mess with queuing? Locking considerations,
of course!
Link: https://lore.kernel.org/rcu/[email protected]/
Reported-by: Kent Overstreet <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
---
kernel/rcu/srcutree.c | 66 +++++++++++++++++++++++++++++----------------------
1 file changed, 37 insertions(+), 29 deletions(-)
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 79b7081..d930ece 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -808,6 +808,42 @@ static void srcu_leak_callback(struct rcu_head *rhp)
}
/*
+ * Start an SRCU grace period, and also queue the callback if non-NULL.
+ */
+static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rhp, bool do_norm)
+{
+ unsigned long flags;
+ int idx;
+ bool needexp = false;
+ bool needgp = false;
+ unsigned long s;
+ struct srcu_data *sdp;
+
+ idx = srcu_read_lock(ssp);
+ sdp = raw_cpu_ptr(ssp->sda);
+ spin_lock_irqsave_rcu_node(sdp, flags);
+ rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
+ rcu_segcblist_advance(&sdp->srcu_cblist,
+ rcu_seq_current(&ssp->srcu_gp_seq));
+ s = rcu_seq_snap(&ssp->srcu_gp_seq);
+ (void)rcu_segcblist_accelerate(&sdp->srcu_cblist, s);
+ if (ULONG_CMP_LT(sdp->srcu_gp_seq_needed, s)) {
+ sdp->srcu_gp_seq_needed = s;
+ needgp = true;
+ }
+ if (!do_norm && ULONG_CMP_LT(sdp->srcu_gp_seq_needed_exp, s)) {
+ sdp->srcu_gp_seq_needed_exp = s;
+ needexp = true;
+ }
+ spin_unlock_irqrestore_rcu_node(sdp, flags);
+ if (needgp)
+ srcu_funnel_gp_start(ssp, sdp, s, do_norm);
+ else if (needexp)
+ srcu_funnel_exp_start(ssp, sdp->mynode, s);
+ srcu_read_unlock(ssp, idx);
+}
+
+/*
* Enqueue an SRCU callback on the srcu_data structure associated with
* the current CPU and the specified srcu_struct structure, initiating
* grace-period processing if it is not already running.
@@ -838,13 +874,6 @@ static void srcu_leak_callback(struct rcu_head *rhp)
static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
rcu_callback_t func, bool do_norm)
{
- unsigned long flags;
- int idx;
- bool needexp = false;
- bool needgp = false;
- unsigned long s;
- struct srcu_data *sdp;
-
check_init_srcu_struct(ssp);
if (debug_rcu_head_queue(rhp)) {
/* Probable double call_srcu(), so leak the callback. */
@@ -853,28 +882,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
return;
}
rhp->func = func;
- idx = srcu_read_lock(ssp);
- sdp = raw_cpu_ptr(ssp->sda);
- spin_lock_irqsave_rcu_node(sdp, flags);
- rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
- rcu_segcblist_advance(&sdp->srcu_cblist,
- rcu_seq_current(&ssp->srcu_gp_seq));
- s = rcu_seq_snap(&ssp->srcu_gp_seq);
- (void)rcu_segcblist_accelerate(&sdp->srcu_cblist, s);
- if (ULONG_CMP_LT(sdp->srcu_gp_seq_needed, s)) {
- sdp->srcu_gp_seq_needed = s;
- needgp = true;
- }
- if (!do_norm && ULONG_CMP_LT(sdp->srcu_gp_seq_needed_exp, s)) {
- sdp->srcu_gp_seq_needed_exp = s;
- needexp = true;
- }
- spin_unlock_irqrestore_rcu_node(sdp, flags);
- if (needgp)
- srcu_funnel_gp_start(ssp, sdp, s, do_norm);
- else if (needexp)
- srcu_funnel_exp_start(ssp, sdp->mynode, s);
- srcu_read_unlock(ssp, idx);
+ srcu_gp_start_if_needed(ssp, rhp, do_norm);
}
/**
--
2.9.5
From: "Paul E. McKenney" <[email protected]>
There is a need for a polling interface for SRCU grace periods.
This polling needs to initiate an SRCU grace period without
having to queue (and manage) a callback. This commit therefore
splits the Tiny SRCU call_srcu() function into callback-queuing and
start-grace-period portions, with the latter in a new function named
srcu_gp_start_if_needed().
Link: https://lore.kernel.org/rcu/[email protected]/
Reported-by: Kent Overstreet <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
---
kernel/rcu/srcutiny.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index 5598cf6..3bac1db 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -152,6 +152,16 @@ void srcu_drive_gp(struct work_struct *wp)
}
EXPORT_SYMBOL_GPL(srcu_drive_gp);
+static void srcu_gp_start_if_needed(struct srcu_struct *ssp)
+{
+ if (!READ_ONCE(ssp->srcu_gp_running)) {
+ if (likely(srcu_init_done))
+ schedule_work(&ssp->srcu_work);
+ else if (list_empty(&ssp->srcu_work.entry))
+ list_add(&ssp->srcu_work.entry, &srcu_boot_list);
+ }
+}
+
/*
* Enqueue an SRCU callback on the specified srcu_struct structure,
* initiating grace-period processing if it is not already running.
@@ -167,12 +177,7 @@ void call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
*ssp->srcu_cb_tail = rhp;
ssp->srcu_cb_tail = &rhp->next;
local_irq_restore(flags);
- if (!READ_ONCE(ssp->srcu_gp_running)) {
- if (likely(srcu_init_done))
- schedule_work(&ssp->srcu_work);
- else if (list_empty(&ssp->srcu_work.entry))
- list_add(&ssp->srcu_work.entry, &srcu_boot_list);
- }
+ srcu_gp_start_if_needed(ssp);
}
EXPORT_SYMBOL_GPL(call_srcu);
--
2.9.5
From: "Paul E. McKenney" <[email protected]>
There is a need for a polling interface for SRCU grace
periods, so this commit supplies get_state_synchronize_srcu(),
start_poll_synchronize_srcu(), and poll_state_synchronize_srcu() for this
purpose. The first can be used if future grace periods are inevitable
(perhaps due to a later call_srcu() invocation), the second if future
grace periods might not otherwise happen, and the third to check if a
grace period has elapsed since the corresponding call to either of the
first two.
As with get_state_synchronize_rcu() and cond_synchronize_rcu(),
the return value from either get_state_synchronize_srcu() or
start_poll_synchronize_srcu() must be passed in to a later call to
poll_state_synchronize_srcu().
Link: https://lore.kernel.org/rcu/[email protected]/
Reported-by: Kent Overstreet <[email protected]>
[ paulmck: Add EXPORT_SYMBOL_GPL() per kernel test robot feedback. ]
Signed-off-by: Paul E. McKenney <[email protected]>
---
kernel/rcu/srcutree.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 60 insertions(+), 3 deletions(-)
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index d930ece..015d80e 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -810,7 +810,8 @@ static void srcu_leak_callback(struct rcu_head *rhp)
/*
* Start an SRCU grace period, and also queue the callback if non-NULL.
*/
-static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rhp, bool do_norm)
+static unsigned long srcu_gp_start_if_needed(struct srcu_struct *ssp,
+ struct rcu_head *rhp, bool do_norm)
{
unsigned long flags;
int idx;
@@ -822,7 +823,8 @@ static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rh
idx = srcu_read_lock(ssp);
sdp = raw_cpu_ptr(ssp->sda);
spin_lock_irqsave_rcu_node(sdp, flags);
- rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
+ if (rhp)
+ rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
rcu_segcblist_advance(&sdp->srcu_cblist,
rcu_seq_current(&ssp->srcu_gp_seq));
s = rcu_seq_snap(&ssp->srcu_gp_seq);
@@ -841,6 +843,7 @@ static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rh
else if (needexp)
srcu_funnel_exp_start(ssp, sdp->mynode, s);
srcu_read_unlock(ssp, idx);
+ return s;
}
/*
@@ -882,7 +885,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
return;
}
rhp->func = func;
- srcu_gp_start_if_needed(ssp, rhp, do_norm);
+ (void)srcu_gp_start_if_needed(ssp, rhp, do_norm);
}
/**
@@ -1011,6 +1014,60 @@ void synchronize_srcu(struct srcu_struct *ssp)
}
EXPORT_SYMBOL_GPL(synchronize_srcu);
+/**
+ * get_state_synchronize_srcu - Provide an end-of-grace-period cookie
+ * @ssp: srcu_struct to provide cookie for.
+ *
+ * This function returns a cookie that can be passed to
+ * poll_state_synchronize_srcu(), which will return true if a full grace
+ * period has elapsed in the meantime. It is the caller's responsibility
+ * to make sure that grace period happens, for example, by invoking
+ * call_srcu() after return from get_state_synchronize_srcu().
+ */
+unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
+{
+ // Any prior manipulation of SRCU-protected data must happen
+ // before the load from ->srcu_gp_seq.
+ smp_mb();
+ return rcu_seq_snap(&ssp->srcu_gp_seq);
+}
+EXPORT_SYMBOL_GPL(get_state_synchronize_srcu);
+
+/**
+ * start_poll_synchronize_srcu - Provide cookie and start grace period
+ * @ssp: srcu_struct to provide cookie for.
+ *
+ * This function returns a cookie that can be passed to
+ * poll_state_synchronize_srcu(), which will return true if a full grace
+ * period has elapsed in the meantime. Unlike get_state_synchronize_srcu(),
+ * this function also ensures that any needed SRCU grace period will be
+ * started. This convenience does come at a cost in terms of CPU overhead.
+ */
+unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
+{
+ return srcu_gp_start_if_needed(ssp, NULL, true);
+}
+EXPORT_SYMBOL_GPL(start_poll_synchronize_srcu);
+
+/**
+ * poll_state_synchronize_srcu - Has cookie's grace period ended?
+ * @ssp: srcu_struct to provide cookie for.
+ * @cookie: Return value from get_state_synchronize_srcu() or start_poll_synchronize_srcu().
+ *
+ * This function takes the cookie that was returned from either
+ * get_state_synchronize_srcu() or start_poll_synchronize_srcu(), and
+ * returns @true if an SRCU grace period elapsed since the time that the
+ * cookie was created.
+ */
+bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
+{
+ if (!rcu_seq_done(&ssp->srcu_gp_seq, cookie))
+ return false;
+ smp_mb(); // ^^^
+ return true;
+}
+EXPORT_SYMBOL_GPL(poll_state_synchronize_srcu);
+
/*
* Callback function for srcu_barrier() use.
*/
--
2.9.5
Hi Paul,
On 11/17/2020 6:10 AM, [email protected] wrote:
> From: "Paul E. McKenney" <[email protected]>
>
> There is a need for a polling interface for SRCU grace periods. This
> polling needs to distinguish between an SRCU instance being idle on the
> one hand or in the middle of a grace period on the other. This commit
> therefore converts the Tiny SRCU srcu_struct structure's srcu_idx from
> a defacto boolean to a free-running counter, using the bottom bit to
> indicate that a grace period is in progress. The second-from-bottom
> bit is thus used as the index returned by srcu_read_lock().
>
> Link: https://lore.kernel.org/rcu/[email protected]/
> Reported-by: Kent Overstreet <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> ---
> include/linux/srcutiny.h | 4 ++--
> kernel/rcu/srcutiny.c | 5 +++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> index 5a5a194..fed4a2d 100644
> --- a/include/linux/srcutiny.h
> +++ b/include/linux/srcutiny.h
> @@ -15,7 +15,7 @@
>
> struct srcu_struct {
> short srcu_lock_nesting[2]; /* srcu_read_lock() nesting depth. */
> - short srcu_idx; /* Current reader array element. */
> + unsigned short srcu_idx; /* Current reader array element in bit 0x2. */
> u8 srcu_gp_running; /* GP workqueue running? */
> u8 srcu_gp_waiting; /* GP waiting for readers? */
> struct swait_queue_head srcu_wq;
> @@ -59,7 +59,7 @@ static inline int __srcu_read_lock(struct srcu_struct *ssp)
> {
> int idx;
>
> - idx = READ_ONCE(ssp->srcu_idx);
> + idx = (READ_ONCE(ssp->srcu_idx) & 0x2) / 2;
Should we use bit 0x2 of (READ_ONCE(ssp->srcu_idx) + 1) , if GP
(srcu_drive_gp()) is in progress? Or am I missing something here?
idx = ((READ_ONCE(ssp->srcu_idx) +1) & 0x2) / 2;
Also, any reason for using divison instead of shift; something to
do with 16-bit srcu_idx which I am missing here?
Thanks
Neeraj
> WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> return idx;
> }
> diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> index 6208c1d..5598cf6 100644
> --- a/kernel/rcu/srcutiny.c
> +++ b/kernel/rcu/srcutiny.c
> @@ -124,11 +124,12 @@ void srcu_drive_gp(struct work_struct *wp)
> ssp->srcu_cb_head = NULL;
> ssp->srcu_cb_tail = &ssp->srcu_cb_head;
> local_irq_enable();
> - idx = ssp->srcu_idx;
> - WRITE_ONCE(ssp->srcu_idx, !ssp->srcu_idx);
> + idx = (ssp->srcu_idx & 0x2) / 2;
> + WRITE_ONCE(ssp->srcu_idx, ssp->srcu_idx + 1);
> WRITE_ONCE(ssp->srcu_gp_waiting, true); /* srcu_read_unlock() wakes! */
> swait_event_exclusive(ssp->srcu_wq, !READ_ONCE(ssp->srcu_lock_nesting[idx]));
> WRITE_ONCE(ssp->srcu_gp_waiting, false); /* srcu_read_unlock() cheap. */
> + WRITE_ONCE(ssp->srcu_idx, ssp->srcu_idx + 1);
>
> /* Invoke the callbacks we removed above. */
> while (lh) {
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
On Thu, Nov 19, 2020 at 01:44:49PM +0530, Neeraj Upadhyay wrote:
> Hi Paul,
>
> On 11/17/2020 6:10 AM, [email protected] wrote:
> > From: "Paul E. McKenney" <[email protected]>
> >
> > There is a need for a polling interface for SRCU grace periods. This
> > polling needs to distinguish between an SRCU instance being idle on the
> > one hand or in the middle of a grace period on the other. This commit
> > therefore converts the Tiny SRCU srcu_struct structure's srcu_idx from
> > a defacto boolean to a free-running counter, using the bottom bit to
> > indicate that a grace period is in progress. The second-from-bottom
> > bit is thus used as the index returned by srcu_read_lock().
> >
> > Link: https://lore.kernel.org/rcu/[email protected]/
> > Reported-by: Kent Overstreet <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > ---
> > include/linux/srcutiny.h | 4 ++--
> > kernel/rcu/srcutiny.c | 5 +++--
> > 2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> > index 5a5a194..fed4a2d 100644
> > --- a/include/linux/srcutiny.h
> > +++ b/include/linux/srcutiny.h
> > @@ -15,7 +15,7 @@
> > struct srcu_struct {
> > short srcu_lock_nesting[2]; /* srcu_read_lock() nesting depth. */
> > - short srcu_idx; /* Current reader array element. */
> > + unsigned short srcu_idx; /* Current reader array element in bit 0x2. */
> > u8 srcu_gp_running; /* GP workqueue running? */
> > u8 srcu_gp_waiting; /* GP waiting for readers? */
> > struct swait_queue_head srcu_wq;
> > @@ -59,7 +59,7 @@ static inline int __srcu_read_lock(struct srcu_struct *ssp)
> > {
> > int idx;
> > - idx = READ_ONCE(ssp->srcu_idx);
> > + idx = (READ_ONCE(ssp->srcu_idx) & 0x2) / 2;
>
> Should we use bit 0x2 of (READ_ONCE(ssp->srcu_idx) + 1) , if GP
> (srcu_drive_gp()) is in progress? Or am I missing something here?
>
> idx = ((READ_ONCE(ssp->srcu_idx) +1) & 0x2) / 2;
You miss nothing! I am running about 200 hours of concurrent rcutorture
of the SRCU-t and SRCU-u scenarios, but I must admit that this race could
be hard to hit. But it could of course result in too-short grace periods.
I will fold this into the original with attribution.
> Also, any reason for using divison instead of shift; something to
> do with 16-bit srcu_idx which I am missing here?
I just figure that the compiler is better at selecting instructions
than I am. Either would work.
Thanx, Paul
> Thanks
> Neeraj
>
> > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > return idx;
> > }
> > diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> > index 6208c1d..5598cf6 100644
> > --- a/kernel/rcu/srcutiny.c
> > +++ b/kernel/rcu/srcutiny.c
> > @@ -124,11 +124,12 @@ void srcu_drive_gp(struct work_struct *wp)
> > ssp->srcu_cb_head = NULL;
> > ssp->srcu_cb_tail = &ssp->srcu_cb_head;
> > local_irq_enable();
> > - idx = ssp->srcu_idx;
> > - WRITE_ONCE(ssp->srcu_idx, !ssp->srcu_idx);
> > + idx = (ssp->srcu_idx & 0x2) / 2;
> > + WRITE_ONCE(ssp->srcu_idx, ssp->srcu_idx + 1);
> > WRITE_ONCE(ssp->srcu_gp_waiting, true); /* srcu_read_unlock() wakes! */
> > swait_event_exclusive(ssp->srcu_wq, !READ_ONCE(ssp->srcu_lock_nesting[idx]));
> > WRITE_ONCE(ssp->srcu_gp_waiting, false); /* srcu_read_unlock() cheap. */
> > + WRITE_ONCE(ssp->srcu_idx, ssp->srcu_idx + 1);
> > /* Invoke the callbacks we removed above. */
> > while (lh) {
> >
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of
> the Code Aurora Forum, hosted by The Linux Foundation
On 11/17/2020 6:10 AM, [email protected] wrote:
> From: "Paul E. McKenney" <[email protected]>
>
> There is a need for a polling interface for SRCU grace periods.
> This polling needs to initiate an SRCU grace period without
> having to queue (and manage) a callback. This commit therefore
> splits the Tiny SRCU call_srcu() function into callback-queuing and
> start-grace-period portions, with the latter in a new function named
> srcu_gp_start_if_needed().
>
> Link: https://lore.kernel.org/rcu/[email protected]/
> Reported-by: Kent Overstreet <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> ---
Reviewed-by: Neeraj Upadhyay <[email protected]>
Thanks
Neeraj
> kernel/rcu/srcutiny.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> index 5598cf6..3bac1db 100644
> --- a/kernel/rcu/srcutiny.c
> +++ b/kernel/rcu/srcutiny.c
> @@ -152,6 +152,16 @@ void srcu_drive_gp(struct work_struct *wp)
> }
> EXPORT_SYMBOL_GPL(srcu_drive_gp);
>
> +static void srcu_gp_start_if_needed(struct srcu_struct *ssp)
> +{
> + if (!READ_ONCE(ssp->srcu_gp_running)) {
> + if (likely(srcu_init_done))
> + schedule_work(&ssp->srcu_work);
> + else if (list_empty(&ssp->srcu_work.entry))
> + list_add(&ssp->srcu_work.entry, &srcu_boot_list);
> + }
> +}
> +
> /*
> * Enqueue an SRCU callback on the specified srcu_struct structure,
> * initiating grace-period processing if it is not already running.
> @@ -167,12 +177,7 @@ void call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> *ssp->srcu_cb_tail = rhp;
> ssp->srcu_cb_tail = &rhp->next;
> local_irq_restore(flags);
> - if (!READ_ONCE(ssp->srcu_gp_running)) {
> - if (likely(srcu_init_done))
> - schedule_work(&ssp->srcu_work);
> - else if (list_empty(&ssp->srcu_work.entry))
> - list_add(&ssp->srcu_work.entry, &srcu_boot_list);
> - }
> + srcu_gp_start_if_needed(ssp);
> }
> EXPORT_SYMBOL_GPL(call_srcu);
>
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
On 11/17/2020 6:10 AM, [email protected] wrote:
> From: "Paul E. McKenney" <[email protected]>
>
> There is a need for a polling interface for SRCU grace periods.
> This polling needs to initiate an SRCU grace period without having
> to queue (and manage) a callback. This commit therefore splits the
> Tree SRCU __call_srcu() function into callback-initialization and
> queuing/start-grace-period portions, with the latter in a new function
> named srcu_gp_start_if_needed(). This function may be passed a NULL
> callback pointer, in which case it will refrain from queuing anything.
>
> Why have the new function mess with queuing? Locking considerations,
> of course!
>
> Link: https://lore.kernel.org/rcu/[email protected]/
> Reported-by: Kent Overstreet <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> ---
Reviewed-by: Neeraj Upadhyay <[email protected]>
Thanks
Neeraj
> kernel/rcu/srcutree.c | 66 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 79b7081..d930ece 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -808,6 +808,42 @@ static void srcu_leak_callback(struct rcu_head *rhp)
> }
>
> /*
> + * Start an SRCU grace period, and also queue the callback if non-NULL.
> + */
> +static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rhp, bool do_norm)
> +{
> + unsigned long flags;
> + int idx;
> + bool needexp = false;
> + bool needgp = false;
> + unsigned long s;
> + struct srcu_data *sdp;
> +
> + idx = srcu_read_lock(ssp);
> + sdp = raw_cpu_ptr(ssp->sda);
> + spin_lock_irqsave_rcu_node(sdp, flags);
> + rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> + rcu_segcblist_advance(&sdp->srcu_cblist,
> + rcu_seq_current(&ssp->srcu_gp_seq));
> + s = rcu_seq_snap(&ssp->srcu_gp_seq);
> + (void)rcu_segcblist_accelerate(&sdp->srcu_cblist, s);
> + if (ULONG_CMP_LT(sdp->srcu_gp_seq_needed, s)) {
> + sdp->srcu_gp_seq_needed = s;
> + needgp = true;
> + }
> + if (!do_norm && ULONG_CMP_LT(sdp->srcu_gp_seq_needed_exp, s)) {
> + sdp->srcu_gp_seq_needed_exp = s;
> + needexp = true;
> + }
> + spin_unlock_irqrestore_rcu_node(sdp, flags);
> + if (needgp)
> + srcu_funnel_gp_start(ssp, sdp, s, do_norm);
> + else if (needexp)
> + srcu_funnel_exp_start(ssp, sdp->mynode, s);
> + srcu_read_unlock(ssp, idx);
> +}
> +
> +/*
> * Enqueue an SRCU callback on the srcu_data structure associated with
> * the current CPU and the specified srcu_struct structure, initiating
> * grace-period processing if it is not already running.
> @@ -838,13 +874,6 @@ static void srcu_leak_callback(struct rcu_head *rhp)
> static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> rcu_callback_t func, bool do_norm)
> {
> - unsigned long flags;
> - int idx;
> - bool needexp = false;
> - bool needgp = false;
> - unsigned long s;
> - struct srcu_data *sdp;
> -
> check_init_srcu_struct(ssp);
> if (debug_rcu_head_queue(rhp)) {
> /* Probable double call_srcu(), so leak the callback. */
> @@ -853,28 +882,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> return;
> }
> rhp->func = func;
> - idx = srcu_read_lock(ssp);
> - sdp = raw_cpu_ptr(ssp->sda);
> - spin_lock_irqsave_rcu_node(sdp, flags);
> - rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> - rcu_segcblist_advance(&sdp->srcu_cblist,
> - rcu_seq_current(&ssp->srcu_gp_seq));
> - s = rcu_seq_snap(&ssp->srcu_gp_seq);
> - (void)rcu_segcblist_accelerate(&sdp->srcu_cblist, s);
> - if (ULONG_CMP_LT(sdp->srcu_gp_seq_needed, s)) {
> - sdp->srcu_gp_seq_needed = s;
> - needgp = true;
> - }
> - if (!do_norm && ULONG_CMP_LT(sdp->srcu_gp_seq_needed_exp, s)) {
> - sdp->srcu_gp_seq_needed_exp = s;
> - needexp = true;
> - }
> - spin_unlock_irqrestore_rcu_node(sdp, flags);
> - if (needgp)
> - srcu_funnel_gp_start(ssp, sdp, s, do_norm);
> - else if (needexp)
> - srcu_funnel_exp_start(ssp, sdp->mynode, s);
> - srcu_read_unlock(ssp, idx);
> + srcu_gp_start_if_needed(ssp, rhp, do_norm);
> }
>
> /**
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
On 11/17/2020 6:10 AM, [email protected] wrote:
> From: "Paul E. McKenney" <[email protected]>
>
> There is a need for a polling interface for SRCU grace
> periods, so this commit supplies get_state_synchronize_srcu(),
> start_poll_synchronize_srcu(), and poll_state_synchronize_srcu() for this
> purpose. The first can be used if future grace periods are inevitable
> (perhaps due to a later call_srcu() invocation), the second if future
> grace periods might not otherwise happen, and the third to check if a
> grace period has elapsed since the corresponding call to either of the
> first two.
>
> As with get_state_synchronize_rcu() and cond_synchronize_rcu(),
> the return value from either get_state_synchronize_srcu() or
> start_poll_synchronize_srcu() must be passed in to a later call to
> poll_state_synchronize_srcu().
>
> Link: https://lore.kernel.org/rcu/[email protected]/
> Reported-by: Kent Overstreet <[email protected]>
> [ paulmck: Add EXPORT_SYMBOL_GPL() per kernel test robot feedback. ]
> Signed-off-by: Paul E. McKenney <[email protected]>
> ---
> kernel/rcu/srcutree.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 60 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index d930ece..015d80e 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -810,7 +810,8 @@ static void srcu_leak_callback(struct rcu_head *rhp)
> /*
> * Start an SRCU grace period, and also queue the callback if non-NULL.
> */
> -static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rhp, bool do_norm)
> +static unsigned long srcu_gp_start_if_needed(struct srcu_struct *ssp,
> + struct rcu_head *rhp, bool do_norm)
> {
> unsigned long flags;
> int idx;
> @@ -822,7 +823,8 @@ static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rh
> idx = srcu_read_lock(ssp);
> sdp = raw_cpu_ptr(ssp->sda);
> spin_lock_irqsave_rcu_node(sdp, flags);
> - rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> + if (rhp)
> + rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> rcu_segcblist_advance(&sdp->srcu_cblist,
> rcu_seq_current(&ssp->srcu_gp_seq));
> s = rcu_seq_snap(&ssp->srcu_gp_seq);
> @@ -841,6 +843,7 @@ static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rh
> else if (needexp)
> srcu_funnel_exp_start(ssp, sdp->mynode, s);
> srcu_read_unlock(ssp, idx);
> + return s;
> }
>
> /*
> @@ -882,7 +885,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> return;
> }
> rhp->func = func;
> - srcu_gp_start_if_needed(ssp, rhp, do_norm);
> + (void)srcu_gp_start_if_needed(ssp, rhp, do_norm);
> }
>
> /**
> @@ -1011,6 +1014,60 @@ void synchronize_srcu(struct srcu_struct *ssp)
> }
> EXPORT_SYMBOL_GPL(synchronize_srcu);
>
> +/**
> + * get_state_synchronize_srcu - Provide an end-of-grace-period cookie
> + * @ssp: srcu_struct to provide cookie for.
> + *
> + * This function returns a cookie that can be passed to
> + * poll_state_synchronize_srcu(), which will return true if a full grace
> + * period has elapsed in the meantime. It is the caller's responsibility
> + * to make sure that grace period happens, for example, by invoking
> + * call_srcu() after return from get_state_synchronize_srcu().
> + */
> +unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
> +{
> + // Any prior manipulation of SRCU-protected data must happen
> + // before the load from ->srcu_gp_seq.
> + smp_mb();
> + return rcu_seq_snap(&ssp->srcu_gp_seq);
> +}
> +EXPORT_SYMBOL_GPL(get_state_synchronize_srcu);
> +
> +/**
> + * start_poll_synchronize_srcu - Provide cookie and start grace period
> + * @ssp: srcu_struct to provide cookie for.
> + *
> + * This function returns a cookie that can be passed to
> + * poll_state_synchronize_srcu(), which will return true if a full grace
> + * period has elapsed in the meantime. Unlike get_state_synchronize_srcu(),
> + * this function also ensures that any needed SRCU grace period will be
> + * started. This convenience does come at a cost in terms of CPU overhead.
> + */
> +unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
> +{
> + return srcu_gp_start_if_needed(ssp, NULL, true);
> +}
> +EXPORT_SYMBOL_GPL(start_poll_synchronize_srcu);
> +
> +/**
> + * poll_state_synchronize_srcu - Has cookie's grace period ended?
> + * @ssp: srcu_struct to provide cookie for.
> + * @cookie: Return value from get_state_synchronize_srcu() or start_poll_synchronize_srcu().
> + *
> + * This function takes the cookie that was returned from either
> + * get_state_synchronize_srcu() or start_poll_synchronize_srcu(), and
> + * returns @true if an SRCU grace period elapsed since the time that the
> + * cookie was created.
> + */
> +bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
> +{
> + if (!rcu_seq_done(&ssp->srcu_gp_seq, cookie))
> + return false;
> + smp_mb(); // ^^^
Minor: Should this comment be more descriptive?
Thanks
Neeraj
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(poll_state_synchronize_srcu);
> +
> /*
> * Callback function for srcu_barrier() use.
> */
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
On Fri, Nov 20, 2020 at 05:31:43PM +0530, Neeraj Upadhyay wrote:
>
>
> On 11/17/2020 6:10 AM, [email protected] wrote:
> > From: "Paul E. McKenney" <[email protected]>
> >
> > There is a need for a polling interface for SRCU grace
> > periods, so this commit supplies get_state_synchronize_srcu(),
> > start_poll_synchronize_srcu(), and poll_state_synchronize_srcu() for this
> > purpose. The first can be used if future grace periods are inevitable
> > (perhaps due to a later call_srcu() invocation), the second if future
> > grace periods might not otherwise happen, and the third to check if a
> > grace period has elapsed since the corresponding call to either of the
> > first two.
> >
> > As with get_state_synchronize_rcu() and cond_synchronize_rcu(),
> > the return value from either get_state_synchronize_srcu() or
> > start_poll_synchronize_srcu() must be passed in to a later call to
> > poll_state_synchronize_srcu().
> >
> > Link: https://lore.kernel.org/rcu/[email protected]/
> > Reported-by: Kent Overstreet <[email protected]>
> > [ paulmck: Add EXPORT_SYMBOL_GPL() per kernel test robot feedback. ]
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > ---
> > kernel/rcu/srcutree.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 60 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index d930ece..015d80e 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -810,7 +810,8 @@ static void srcu_leak_callback(struct rcu_head *rhp)
> > /*
> > * Start an SRCU grace period, and also queue the callback if non-NULL.
> > */
> > -static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rhp, bool do_norm)
> > +static unsigned long srcu_gp_start_if_needed(struct srcu_struct *ssp,
> > + struct rcu_head *rhp, bool do_norm)
> > {
> > unsigned long flags;
> > int idx;
> > @@ -822,7 +823,8 @@ static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rh
> > idx = srcu_read_lock(ssp);
> > sdp = raw_cpu_ptr(ssp->sda);
> > spin_lock_irqsave_rcu_node(sdp, flags);
> > - rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> > + if (rhp)
> > + rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> > rcu_segcblist_advance(&sdp->srcu_cblist,
> > rcu_seq_current(&ssp->srcu_gp_seq));
> > s = rcu_seq_snap(&ssp->srcu_gp_seq);
> > @@ -841,6 +843,7 @@ static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rh
> > else if (needexp)
> > srcu_funnel_exp_start(ssp, sdp->mynode, s);
> > srcu_read_unlock(ssp, idx);
> > + return s;
> > }
> > /*
> > @@ -882,7 +885,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> > return;
> > }
> > rhp->func = func;
> > - srcu_gp_start_if_needed(ssp, rhp, do_norm);
> > + (void)srcu_gp_start_if_needed(ssp, rhp, do_norm);
> > }
> > /**
> > @@ -1011,6 +1014,60 @@ void synchronize_srcu(struct srcu_struct *ssp)
> > }
> > EXPORT_SYMBOL_GPL(synchronize_srcu);
> > +/**
> > + * get_state_synchronize_srcu - Provide an end-of-grace-period cookie
> > + * @ssp: srcu_struct to provide cookie for.
> > + *
> > + * This function returns a cookie that can be passed to
> > + * poll_state_synchronize_srcu(), which will return true if a full grace
> > + * period has elapsed in the meantime. It is the caller's responsibility
> > + * to make sure that grace period happens, for example, by invoking
> > + * call_srcu() after return from get_state_synchronize_srcu().
> > + */
> > +unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
> > +{
> > + // Any prior manipulation of SRCU-protected data must happen
> > + // before the load from ->srcu_gp_seq.
> > + smp_mb();
> > + return rcu_seq_snap(&ssp->srcu_gp_seq);
> > +}
> > +EXPORT_SYMBOL_GPL(get_state_synchronize_srcu);
> > +
> > +/**
> > + * start_poll_synchronize_srcu - Provide cookie and start grace period
> > + * @ssp: srcu_struct to provide cookie for.
> > + *
> > + * This function returns a cookie that can be passed to
> > + * poll_state_synchronize_srcu(), which will return true if a full grace
> > + * period has elapsed in the meantime. Unlike get_state_synchronize_srcu(),
> > + * this function also ensures that any needed SRCU grace period will be
> > + * started. This convenience does come at a cost in terms of CPU overhead.
> > + */
> > +unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
> > +{
> > + return srcu_gp_start_if_needed(ssp, NULL, true);
> > +}
> > +EXPORT_SYMBOL_GPL(start_poll_synchronize_srcu);
> > +
> > +/**
> > + * poll_state_synchronize_srcu - Has cookie's grace period ended?
> > + * @ssp: srcu_struct to provide cookie for.
> > + * @cookie: Return value from get_state_synchronize_srcu() or start_poll_synchronize_srcu().
> > + *
> > + * This function takes the cookie that was returned from either
> > + * get_state_synchronize_srcu() or start_poll_synchronize_srcu(), and
> > + * returns @true if an SRCU grace period elapsed since the time that the
> > + * cookie was created.
> > + */
> > +bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
> > +{
> > + if (!rcu_seq_done(&ssp->srcu_gp_seq, cookie))
> > + return false;
> > + smp_mb(); // ^^^
>
> Minor: Should this comment be more descriptive?
You have to read between the lines? ;-)
How about like this?
Thanx, Paul
------------------------------------------------------------------------
bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
{
if (!rcu_seq_done(&ssp->srcu_gp_seq, cookie))
return false;
// Ensure that the end of the SRCU grace period happens before
// any subsequent code that the caller might execute.
smp_mb(); // ^^^
return true;
}
On Fri, Nov 20, 2020 at 05:06:50PM +0530, Neeraj Upadhyay wrote:
>
>
> On 11/17/2020 6:10 AM, [email protected] wrote:
> > From: "Paul E. McKenney" <[email protected]>
> >
> > There is a need for a polling interface for SRCU grace periods.
> > This polling needs to initiate an SRCU grace period without having
> > to queue (and manage) a callback. This commit therefore splits the
> > Tree SRCU __call_srcu() function into callback-initialization and
> > queuing/start-grace-period portions, with the latter in a new function
> > named srcu_gp_start_if_needed(). This function may be passed a NULL
> > callback pointer, in which case it will refrain from queuing anything.
> >
> > Why have the new function mess with queuing? Locking considerations,
> > of course!
> >
> > Link: https://lore.kernel.org/rcu/[email protected]/
> > Reported-by: Kent Overstreet <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > ---
>
> Reviewed-by: Neeraj Upadhyay <[email protected]>
I applied both Reviewed-bys, thank you!
Thanx, Paul
> Thanks
> Neeraj
>
> > kernel/rcu/srcutree.c | 66 +++++++++++++++++++++++++++++----------------------
> > 1 file changed, 37 insertions(+), 29 deletions(-)
> >
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index 79b7081..d930ece 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -808,6 +808,42 @@ static void srcu_leak_callback(struct rcu_head *rhp)
> > }
> > /*
> > + * Start an SRCU grace period, and also queue the callback if non-NULL.
> > + */
> > +static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rhp, bool do_norm)
> > +{
> > + unsigned long flags;
> > + int idx;
> > + bool needexp = false;
> > + bool needgp = false;
> > + unsigned long s;
> > + struct srcu_data *sdp;
> > +
> > + idx = srcu_read_lock(ssp);
> > + sdp = raw_cpu_ptr(ssp->sda);
> > + spin_lock_irqsave_rcu_node(sdp, flags);
> > + rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> > + rcu_segcblist_advance(&sdp->srcu_cblist,
> > + rcu_seq_current(&ssp->srcu_gp_seq));
> > + s = rcu_seq_snap(&ssp->srcu_gp_seq);
> > + (void)rcu_segcblist_accelerate(&sdp->srcu_cblist, s);
> > + if (ULONG_CMP_LT(sdp->srcu_gp_seq_needed, s)) {
> > + sdp->srcu_gp_seq_needed = s;
> > + needgp = true;
> > + }
> > + if (!do_norm && ULONG_CMP_LT(sdp->srcu_gp_seq_needed_exp, s)) {
> > + sdp->srcu_gp_seq_needed_exp = s;
> > + needexp = true;
> > + }
> > + spin_unlock_irqrestore_rcu_node(sdp, flags);
> > + if (needgp)
> > + srcu_funnel_gp_start(ssp, sdp, s, do_norm);
> > + else if (needexp)
> > + srcu_funnel_exp_start(ssp, sdp->mynode, s);
> > + srcu_read_unlock(ssp, idx);
> > +}
> > +
> > +/*
> > * Enqueue an SRCU callback on the srcu_data structure associated with
> > * the current CPU and the specified srcu_struct structure, initiating
> > * grace-period processing if it is not already running.
> > @@ -838,13 +874,6 @@ static void srcu_leak_callback(struct rcu_head *rhp)
> > static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> > rcu_callback_t func, bool do_norm)
> > {
> > - unsigned long flags;
> > - int idx;
> > - bool needexp = false;
> > - bool needgp = false;
> > - unsigned long s;
> > - struct srcu_data *sdp;
> > -
> > check_init_srcu_struct(ssp);
> > if (debug_rcu_head_queue(rhp)) {
> > /* Probable double call_srcu(), so leak the callback. */
> > @@ -853,28 +882,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> > return;
> > }
> > rhp->func = func;
> > - idx = srcu_read_lock(ssp);
> > - sdp = raw_cpu_ptr(ssp->sda);
> > - spin_lock_irqsave_rcu_node(sdp, flags);
> > - rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
> > - rcu_segcblist_advance(&sdp->srcu_cblist,
> > - rcu_seq_current(&ssp->srcu_gp_seq));
> > - s = rcu_seq_snap(&ssp->srcu_gp_seq);
> > - (void)rcu_segcblist_accelerate(&sdp->srcu_cblist, s);
> > - if (ULONG_CMP_LT(sdp->srcu_gp_seq_needed, s)) {
> > - sdp->srcu_gp_seq_needed = s;
> > - needgp = true;
> > - }
> > - if (!do_norm && ULONG_CMP_LT(sdp->srcu_gp_seq_needed_exp, s)) {
> > - sdp->srcu_gp_seq_needed_exp = s;
> > - needexp = true;
> > - }
> > - spin_unlock_irqrestore_rcu_node(sdp, flags);
> > - if (needgp)
> > - srcu_funnel_gp_start(ssp, sdp, s, do_norm);
> > - else if (needexp)
> > - srcu_funnel_exp_start(ssp, sdp->mynode, s);
> > - srcu_read_unlock(ssp, idx);
> > + srcu_gp_start_if_needed(ssp, rhp, do_norm);
> > }
> > /**
> >
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of
> the Code Aurora Forum, hosted by The Linux Foundation
Hello!
This is V2 of the series provides a polling interface for SRCU grace periods. The
API is still as follows:
unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
Returns a "cookie" that can be thought of as a snapshot of
the specified SRCU instance's grace-period sequence number.
Also ensures that enough future grace periods happen to eventually
make the grace-period sequence number reach the cookie.
bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
Given a cookie from start_poll_synchronize_srcu(), returns true if
at least one full SRCU grace period has elapsed in the meantime.
Given finite SRCU readers in a well-behaved kernel, the following
code will complete in finite time:
cookie = start_poll_synchronize_srcu(&my_srcu);
while (!poll_state_synchronize_srcu(&my_srcu, cookie))
schedule_timeout_uninterruptible(1);
unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
Like start_poll_synchronize_srcu(), except that it does not start
any grace periods. This means that the following code is -not-
guaranteed to complete:
cookie = get_state_synchronize_srcu(&my_srcu);
while (!poll_state_synchronize_srcu(&my_srcu, cookie))
schedule_timeout_uninterruptible(1);
Use this if you know that something else will be starting the
needed SRCU grace periods. This might also be useful if you
had items that were likely to be reused before the SRCU grace
period elapsed, so that you avoid burning CPU on SRCU grace
periods that prove to be unnecessary. Or if you don't want
to have more than (say) 100 SRCU grace periods per seconds,
in which case you might use a timer to start the grace periods.
Or maybe you don't bother starting the SRCU grace period until
some sort of emergency situation has arisen. Or...
OK, maybe no one needs it, but rcutorture does need it, so here
it is anyway.
The patches in this version of the series are as follows:
1. Make Tiny SRCU use multi-bit grace-period counter.
2. Provide internal interface to start a Tiny SRCU grace period.
3. Provide internal interface to start a Tree SRCU grace period.
4. Provide polling interfaces for Tiny SRCU grace periods.
5. Provide polling interfaces for Tree SRCU grace periods.
6. Document polling interfaces for Tree SRCU grace periods.
Changes from v1:
o Added EXPORT_SYMBOL_GPL() to allow rcutorture testing when
rcutorture is built as a module.
o Applied review feedback from Neeraj Upadhyay.
o Updated RCU requirements documentation.
Thanx, Paul
------------------------------------------------------------------------
Documentation/RCU/Design/Requirements/Requirements.rst | 18 ++
include/linux/rcupdate.h | 2
include/linux/srcu.h | 3
include/linux/srcutiny.h | 5
kernel/rcu/srcutiny.c | 76 ++++++++-
kernel/rcu/srcutree.c | 133 ++++++++++++-----
6 files changed, 191 insertions(+), 46 deletions(-)
On Fri, 20 Nov 2020 16:58:09 -0800
"Paul E. McKenney" <[email protected]> wrote:
> OK, maybe no one needs it, but rcutorture does need it, so here
> it is anyway.
Ah, so there was a use case. I was scratching my head a bit before
reading this ;-)
-- Steve
On Fri, Nov 20, 2020 at 08:05:53PM -0500, Steven Rostedt wrote:
> On Fri, 20 Nov 2020 16:58:09 -0800
> "Paul E. McKenney" <[email protected]> wrote:
>
> > OK, maybe no one needs it, but rcutorture does need it, so here
> > it is anyway.
>
> Ah, so there was a use case. I was scratching my head a bit before
> reading this ;-)
Indeed, the overhead of start_poll_synchronize_srcu() is a killer for
some parts of rcutorture, plus those parts don't care if no grace period
gets started...
Thanx, Paul
On 11/21/2020 5:46 AM, Paul E. McKenney wrote:
> On Fri, Nov 20, 2020 at 05:31:43PM +0530, Neeraj Upadhyay wrote:
>>
>>
>> On 11/17/2020 6:10 AM, [email protected] wrote:
>>> From: "Paul E. McKenney" <[email protected]>
>>>
>>> There is a need for a polling interface for SRCU grace
>>> periods, so this commit supplies get_state_synchronize_srcu(),
>>> start_poll_synchronize_srcu(), and poll_state_synchronize_srcu() for this
>>> purpose. The first can be used if future grace periods are inevitable
>>> (perhaps due to a later call_srcu() invocation), the second if future
>>> grace periods might not otherwise happen, and the third to check if a
>>> grace period has elapsed since the corresponding call to either of the
>>> first two.
>>>
>>> As with get_state_synchronize_rcu() and cond_synchronize_rcu(),
>>> the return value from either get_state_synchronize_srcu() or
>>> start_poll_synchronize_srcu() must be passed in to a later call to
>>> poll_state_synchronize_srcu().
>>>
>>> Link: https://lore.kernel.org/rcu/[email protected]/
>>> Reported-by: Kent Overstreet <[email protected]>
>>> [ paulmck: Add EXPORT_SYMBOL_GPL() per kernel test robot feedback. ]
>>> Signed-off-by: Paul E. McKenney <[email protected]>
>>> ---
>>> kernel/rcu/srcutree.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 60 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
>>> index d930ece..015d80e 100644
>>> --- a/kernel/rcu/srcutree.c
>>> +++ b/kernel/rcu/srcutree.c
>>> @@ -810,7 +810,8 @@ static void srcu_leak_callback(struct rcu_head *rhp)
>>> /*
>>> * Start an SRCU grace period, and also queue the callback if non-NULL.
>>> */
>>> -static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rhp, bool do_norm)
>>> +static unsigned long srcu_gp_start_if_needed(struct srcu_struct *ssp,
>>> + struct rcu_head *rhp, bool do_norm)
>>> {
>>> unsigned long flags;
>>> int idx;
>>> @@ -822,7 +823,8 @@ static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rh
>>> idx = srcu_read_lock(ssp);
>>> sdp = raw_cpu_ptr(ssp->sda);
>>> spin_lock_irqsave_rcu_node(sdp, flags);
>>> - rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
>>> + if (rhp)
>>> + rcu_segcblist_enqueue(&sdp->srcu_cblist, rhp);
>>> rcu_segcblist_advance(&sdp->srcu_cblist,
>>> rcu_seq_current(&ssp->srcu_gp_seq));
>>> s = rcu_seq_snap(&ssp->srcu_gp_seq);
>>> @@ -841,6 +843,7 @@ static void srcu_gp_start_if_needed(struct srcu_struct *ssp, struct rcu_head *rh
>>> else if (needexp)
>>> srcu_funnel_exp_start(ssp, sdp->mynode, s);
>>> srcu_read_unlock(ssp, idx);
>>> + return s;
>>> }
>>> /*
>>> @@ -882,7 +885,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
>>> return;
>>> }
>>> rhp->func = func;
>>> - srcu_gp_start_if_needed(ssp, rhp, do_norm);
>>> + (void)srcu_gp_start_if_needed(ssp, rhp, do_norm);
>>> }
>>> /**
>>> @@ -1011,6 +1014,60 @@ void synchronize_srcu(struct srcu_struct *ssp)
>>> }
>>> EXPORT_SYMBOL_GPL(synchronize_srcu);
>>> +/**
>>> + * get_state_synchronize_srcu - Provide an end-of-grace-period cookie
>>> + * @ssp: srcu_struct to provide cookie for.
>>> + *
>>> + * This function returns a cookie that can be passed to
>>> + * poll_state_synchronize_srcu(), which will return true if a full grace
>>> + * period has elapsed in the meantime. It is the caller's responsibility
>>> + * to make sure that grace period happens, for example, by invoking
>>> + * call_srcu() after return from get_state_synchronize_srcu().
>>> + */
>>> +unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
>>> +{
>>> + // Any prior manipulation of SRCU-protected data must happen
>>> + // before the load from ->srcu_gp_seq.
>>> + smp_mb();
>>> + return rcu_seq_snap(&ssp->srcu_gp_seq);
>>> +}
>>> +EXPORT_SYMBOL_GPL(get_state_synchronize_srcu);
>>> +
>>> +/**
>>> + * start_poll_synchronize_srcu - Provide cookie and start grace period
>>> + * @ssp: srcu_struct to provide cookie for.
>>> + *
>>> + * This function returns a cookie that can be passed to
>>> + * poll_state_synchronize_srcu(), which will return true if a full grace
>>> + * period has elapsed in the meantime. Unlike get_state_synchronize_srcu(),
>>> + * this function also ensures that any needed SRCU grace period will be
>>> + * started. This convenience does come at a cost in terms of CPU overhead.
>>> + */
>>> +unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
>>> +{
>>> + return srcu_gp_start_if_needed(ssp, NULL, true);
>>> +}
>>> +EXPORT_SYMBOL_GPL(start_poll_synchronize_srcu);
>>> +
>>> +/**
>>> + * poll_state_synchronize_srcu - Has cookie's grace period ended?
>>> + * @ssp: srcu_struct to provide cookie for.
>>> + * @cookie: Return value from get_state_synchronize_srcu() or start_poll_synchronize_srcu().
>>> + *
>>> + * This function takes the cookie that was returned from either
>>> + * get_state_synchronize_srcu() or start_poll_synchronize_srcu(), and
>>> + * returns @true if an SRCU grace period elapsed since the time that the
>>> + * cookie was created.
>>> + */
>>> +bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
>>> +{
>>> + if (!rcu_seq_done(&ssp->srcu_gp_seq, cookie))
>>> + return false;
>>> + smp_mb(); // ^^^
>>
>> Minor: Should this comment be more descriptive?
>
> You have to read between the lines? ;-)
:)
> How about like this?
>
Looks good to me!
Thanks
Neeraj
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
> {
> if (!rcu_seq_done(&ssp->srcu_gp_seq, cookie))
> return false;
> // Ensure that the end of the SRCU grace period happens before
> // any subsequent code that the caller might execute.
> smp_mb(); // ^^^
> return true;
> }
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation