last_qsctr is used in rcu_check_quiescent_state() exclusively.
We can reset qsctr at the start of the grace period, and then
just test qsctr against 0.
On top of the 'rcu: eliminate rcu_ctrlblk.lock', see
http://marc.theaimsgroup.com/?l=linux-kernel&m=110156786721526
Signed-off-by: Oleg Nesterov <[email protected]>
--- 2.6.10-rc2/include/linux/rcupdate.h~ 2004-11-27 21:32:49.000000000 +0300
+++ 2.6.10-rc2/include/linux/rcupdate.h 2004-11-28 23:15:20.510856936 +0300
@@ -88,8 +88,6 @@ struct rcu_data {
/* 1) quiescent state handling : */
long quiescbatch; /* Batch # for grace period */
long qsctr; /* User-mode/idle loop etc. */
- long last_qsctr; /* value of qsctr at beginning */
- /* of rcu grace period */
int qs_pending; /* core waits for quiesc state */
/* 2) batch handling */
--- 2.6.10-rc2/kernel/rcupdate.c~ 2004-11-28 23:13:03.324712400 +0300
+++ 2.6.10-rc2/kernel/rcupdate.c 2004-11-28 23:15:20.511856784 +0300
@@ -215,9 +215,9 @@ static void rcu_check_quiescent_state(st
struct rcu_state *rsp, struct rcu_data *rdp)
{
if (rdp->quiescbatch != rcp->cur) {
- /* new grace period: record qsctr value. */
+ /* new grace period: reset qsctr value. */
rdp->qs_pending = 1;
- rdp->last_qsctr = rdp->qsctr;
+ rdp->qsctr = 0;
rdp->quiescbatch = rcp->cur;
return;
}
@@ -229,12 +229,7 @@ static void rcu_check_quiescent_state(st
if (!rdp->qs_pending)
return;
- /*
- * Races with local timer interrupt - in the worst case
- * we may miss one quiescent state of that CPU. That is
- * tolerable. So no need to disable interrupts.
- */
- if (rdp->qsctr == rdp->last_qsctr)
+ if (rdp->qsctr == 0)
return;
rdp->qs_pending = 0;
Oleg Nesterov wrote:
>last_qsctr is used in rcu_check_quiescent_state() exclusively.
>We can reset qsctr at the start of the grace period, and then
>just test qsctr against 0.
>
>On top of the 'rcu: eliminate rcu_ctrlblk.lock', see
>http://marc.theaimsgroup.com/?l=linux-kernel&m=110156786721526
>
>Signed-off-by: Oleg Nesterov <[email protected]>
>
>
>
Compile and boot tested with 2.6.10-rc2 on a 2-cpu system.
Signed-Off-By: Manfred Spraul <[email protected]>
On Wed, Dec 29, 2004 at 04:37:31PM +0100, Manfred Spraul wrote:
> Oleg Nesterov wrote:
>
> >last_qsctr is used in rcu_check_quiescent_state() exclusively.
> >We can reset qsctr at the start of the grace period, and then
> >just test qsctr against 0.
> >
> >
> >
> It seems the patch got lost, I've updated it a bit and resent it to Andrew.
>
> But: I think there is the potential for an even larger cleanup, although
> this would be more a rewrite:
> Get rid of rcu_check_quiescent_state and instead use something like this
> in rcu_qsctr_inc:
>
> static inline void rcu_qsctr_inc(int cpu)
> {
> struct rcu_data *rdp = &per_cpu(rcu_data, cpu);
> if (rdp->quiescbatch != rcp->cur) {
> /* a new grace period is running. And we are at a quiescent
> * point, so complete it
> */
> spin_lock(&rsp->lock);
> rdp->quiescbatch = rcp->cur;
> cpu_quiet(rdp->cpu, rcp, rsp);
> spin_unlock(&rsp->lock);
> }
> }
>
> It's just an idea, it needs testing on big systems - does reading from
> the global rcp from every schedule call cause any problems? The cache
> line is virtually read-only, so it shouldn't cause trashing, but who knows?
Hello, Manfred,
The main concern I have with this is not cache thrashing of rcp->cur,
but shrinking the grace periods on large systems, which can result in
extra overhead per callback, since the shorter grace periods will tend
to have fewer callbacks. We saw this problem on some of the early
RCU-infrastructure patches.
Another approach would be to conditionally compile the two versions,
though that might make the code more complex.
Thanx, Paul