Maintain special bits at bottom of ->dynticks counter
Subsystems (2)
RFC PATCH v2 1/1 Maintain special bits at bottom of ->dynticks counter
Currently, IPIs are used to force other CPUs to invalidate their TLBs in response to a kernel virtual-memory mapping change. This works, but degrades both battery lifetime (for idle CPUs) and real-time response (for nohz_full CPUs), and in addition results in unnecessary IPIs due to the fact that CPUs executing in usermode are unaffected by stale kernel mappings. It would be better to cause a CPU executing in usermode to wait until it is entering kernel mode to do the flush, first to avoid interrupting usemode tasks and second to handle multiple flush requests with a single flush in the case of a long-running user task.
This commit therefore reserves a bit at the bottom of the ->dynticks counter, which is checked upon exit from extended quiescent states. If it is set, it is cleared and then a new rcu_eqs_special_exit() macro is invoked, which, if not supplied, is an empty single-pass do-while loop. If this bottom bit is set on -entry- to an extended quiescent state, then a WARN_ON_ONCE() triggers.
This bottom bit may be set using a new rcu_eqs_special_set() function, which returns true if the bit was set, or false if the CPU turned out to not be in an extended quiescent state. Please note that this function refuses to set the bit for a non-nohz_full CPU when that CPU is executing in usermode because usermode execution is tracked by RCU as a dyntick-idle extended quiescent state only for nohz_full CPUs.
Changes since v1: Fix ordering of atomic_and() and the call to rcu_eqs_special_exit() in rcu_dynticks_eqs_exit().
include/linux/rcutiny.h +5 -0
@@ -33,6 +33,11 @@ static inline int rcu_dynticks_snap(struct rcu_dynticks *rdtp)return 0 ; } +static inline bool rcu_eqs_special_set (int cpu) +{ +return false;/* Never flag non-existent other CPUs! */ +} +static inline unsigned long get_state_synchronize_rcu (void ) {return 0 ;
kernel/rcu/tree.c +59 -14
138 lines not on this page · raw message
kernel/rcu/tree.h +1 -0
@@ -595,6 +595,7 @@ extern struct rcu_state rcu_preempt_state;#endif /* #ifdef CONFIG_PREEMPT_RCU */ int rcu_dynticks_snap (struct rcu_dynticks *rdtp); +bool rcu_eqs_special_set (int cpu);#ifdef CONFIG_RCU_BOOST DECLARE_PER_CPU(unsigned int, rcu_cpu_kthread_status);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 24 lines · Paul E. McKenney
Currently, IPIs are used to force other CPUs to invalidate their TLBs in response to a kernel virtual-memory mapping change. This works, but degrades both battery lifetime (for idle CPUs) and real-time response (for
nohz_fullCPUs), and in addition results in unnecessary IPIs due to the fact that CPUs executing in usermode are unaffected by stale kernel mappings. It would be better to cause a CPU executing in usermode to wait until it is entering kernel mode to do the flush, first to avoid interrupting usemode tasks and second to handle multiple flush requests with a single flush in the case of a long-running user task.This commit therefore reserves a bit at the bottom of the ->dynticks counter, which is checked upon exit from extended quiescent states. If it is set, it is cleared and then a new
rcu_eqs_special_exit()macro is invoked, which, if not supplied, is an empty single-pass do-while loop. If this bottom bit is set on -entry- to an extended quiescent state, then aWARN_ON_ONCE()triggers.This bottom bit may be set using a new
rcu_eqs_special_set()function, which returns true if the bit was set, or false if the CPU turned out to not be in an extended quiescent state. Please note that this function refuses to set the bit for a non-nohz_full CPU when that CPU is executing in usermode because usermode execution is tracked by RCU as a dyntick-idle extended quiescent state only fornohz_fullCPUs.
Quoted 5 lines
Changes since v1: Fix ordering of
atomic_and()and the call torcu_eqs_special_exit()inrcu_dynticks_eqs_exit().Reported-by Andy Lutomirski <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>
I think I've asked this before, but why does this live in the guts of RCU?
Should we lift this state tracking stuff out and make RCU and NOHZ(_FULL) users of it, or doesn't that make sense (reason)?
In any case, small nit below:
kernel/rcu/tree.c static void rcu_dynticks_eqs_enter(void) · line 339
Quoted 9 lines
kernel/rcu/tree.c +9
-0+ seq =atomic_add_return (RCU_DYNTICK_CTRL_CTR , &rdtp->dynticks); +WARN_ON_ONCE (IS_ENABLED (CONFIG_RCU_EQS_DEBUG ) && + !(seq &RCU_DYNTICK_CTRL_CTR )); +if (seq &RCU_DYNTICK_CTRL_MASK ) { +atomic_and (~RCU_DYNTICK_CTRL_MASK , &rdtp->dynticks); +smp_mb__after_atomic ();/* _exit after clearing mask. */ +/* Prefer duplicate flushes to losing a flush. */ +rcu_eqs_special_exit (); + }
we have atomic_andnot() for just these occasions :-)
Quoted 10 lines
kernel/rcu/tree.c +10
-0+/* +* Set the special (bottom) bit of the specified CPU so that it +* will take special action (such as flushing its TLB) on the +* next exit from an extended quiescent state. Returns true if +* the bit was successfully set, or false if the CPU was not in +* an extended quiescent state. +*/ +bool rcu_eqs_special_set (int cpu) +{ +int old;
kernel/rcu/tree.c static bool rcu_dynticks_in_eqs_since(struct rcu_dynticks *rdtp, int snap) · line 436
Quoted 10 lines
kernel/rcu/tree.c +10
-0+int new; +struct rcu_dynticks *rdtp = &per_cpu (rcu_dynticks, cpu); + +do { + old =atomic_read (&rdtp->dynticks); +if (old &RCU_DYNTICK_CTRL_CTR ) +return false; + new = old |RCU_DYNTICK_CTRL_MASK ; + }while (atomic_cmpxchg (&rdtp->dynticks, old, new) != old); +return true;
Is that what we call atomic_fetch_or() ?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 28 lines · Peter Zijlstra
Quoted 27 lines · Paul E. McKenney
Currently, IPIs are used to force other CPUs to invalidate their TLBs in response to a kernel virtual-memory mapping change. This works, but degrades both battery lifetime (for idle CPUs) and real-time response (for
nohz_fullCPUs), and in addition results in unnecessary IPIs due to the fact that CPUs executing in usermode are unaffected by stale kernel mappings. It would be better to cause a CPU executing in usermode to wait until it is entering kernel mode to do the flush, first to avoid interrupting usemode tasks and second to handle multiple flush requests with a single flush in the case of a long-running user task.This commit therefore reserves a bit at the bottom of the ->dynticks counter, which is checked upon exit from extended quiescent states. If it is set, it is cleared and then a new
rcu_eqs_special_exit()macro is invoked, which, if not supplied, is an empty single-pass do-while loop. If this bottom bit is set on -entry- to an extended quiescent state, then aWARN_ON_ONCE()triggers.This bottom bit may be set using a new
rcu_eqs_special_set()function, which returns true if the bit was set, or false if the CPU turned out to not be in an extended quiescent state. Please note that this function refuses to set the bit for a non-nohz_full CPU when that CPU is executing in usermode because usermode execution is tracked by RCU as a dyntick-idle extended quiescent state only fornohz_fullCPUs.Changes since v1: Fix ordering of
atomic_and()and the call torcu_eqs_special_exit()inrcu_dynticks_eqs_exit().
Quoted 8 lines
Quoted 2 lines
Reported-by Andy Lutomirski <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>I think I've asked this before, but why does this live in the guts of RCU?
Should we lift this state tracking stuff out and make RCU and
NOHZ(_FULL)users of it, or doesn't that make sense (reason)?
The dyntick-idle stuff is pretty specific to RCU. And what precisely would be helped by moving it?
But that was an excellent question, as it reminded me of RCU's dyntick-idle's NMI handling, and I never did ask Andy if it was OK for rcu_eqs_special_exit() to be invoked when exiting NMI handler, which would currently happen. It would be easy for me to pass in a flag indicating whether or not the call is in NMI context, if that is needed.
It is of course not possible to detect this at rcu_eqs_special_set() time, because rcu_eqs_special_set() has no way of knowing that the next event that pulls the remote CPU out of idle will be an NMI.
Quoted 14 lines
In any case, small nit below:
kernel/rcu/tree.c static void rcu_dynticks_eqs_enter(void) · line 339
Quoted 9 lines
kernel/rcu/tree.c +9
-0+ seq =atomic_add_return (RCU_DYNTICK_CTRL_CTR , &rdtp->dynticks); +WARN_ON_ONCE (IS_ENABLED (CONFIG_RCU_EQS_DEBUG ) && + !(seq &RCU_DYNTICK_CTRL_CTR )); +if (seq &RCU_DYNTICK_CTRL_MASK ) { +atomic_and (~RCU_DYNTICK_CTRL_MASK , &rdtp->dynticks); +smp_mb__after_atomic ();/* _exit after clearing mask. */ +/* Prefer duplicate flushes to losing a flush. */ +rcu_eqs_special_exit (); + }we have
atomic_andnot()for just these occasions :-)
I suppose that that could generate more efficient code on some architectures. I have changed this.
Quoted 14 lines
Quoted 14 lines
kernel/rcu/tree.c +14
-0+/* +* Set the special (bottom) bit of the specified CPU so that it +* will take special action (such as flushing its TLB) on the +* next exit from an extended quiescent state. Returns true if +* the bit was successfully set, or false if the CPU was not in +* an extended quiescent state. +*/ +bool rcu_eqs_special_set (int cpu) +{ +int old; +int new; +struct rcu_dynticks *rdtp = &per_cpu (rcu_dynticks, cpu); + + do {
Quoted 8 lines
kernel/rcu/tree.c static bool rcu_dynticks_in_eqs_since(struct rcu_dynticks *rdtp, int snap) · line 436
Quoted 6 lines
kernel/rcu/tree.c +6
-0+ old =atomic_read (&rdtp->dynticks); +if (old &RCU_DYNTICK_CTRL_CTR ) +return false; + new = old |RCU_DYNTICK_CTRL_MASK ; + }while (atomic_cmpxchg (&rdtp->dynticks, old, new) != old); +return true;Is that what we call
atomic_fetch_or()?
I don't think so. The above code takes an early exit if the next bit up is set, which atomic_fetch_or() does not. If the CPU is not in an extended quiescent state (old & RCU_DYNTICK_CTRL_CTR), then this code returns false to indicate that TLB shootdown cannot wait. So it is more like a very specific form of atomic_fetch_or_unless().
Thanx, Paul
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 8 lines · Paul E. McKenney
Quoted 5 lines
I think I've asked this before, but why does this live in the guts of RCU?
Should we lift this state tracking stuff out and make RCU and
NOHZ(_FULL)users of it, or doesn't that make sense (reason)?The dyntick-idle stuff is pretty specific to RCU. And what precisely would be helped by moving it?
Maybe untangle the inter-dependencies somewhat. It just seems a wee bit odd to have arch TLB invalidate depend on RCU implementation details like this.
Quoted 25 lines
But that was an excellent question, as it reminded me of RCU's dyntick-idle's NMI handling, and I never did ask Andy if it was OK for
rcu_eqs_special_exit()to be invoked when exiting NMI handler, which would currently happen. It would be easy for me to pass in a flag indicating whether or not the call is in NMI context, if that is needed.It is of course not possible to detect this at
rcu_eqs_special_set()time, becausercu_eqs_special_set()has no way of knowing that the next event that pulls the remote CPU out of idle will be an NMI.Quoted 14 lines
In any case, small nit below:
kernel/rcu/tree.c static void rcu_dynticks_eqs_enter(void) · line 339
Quoted 9 lines
kernel/rcu/tree.c +9
-0+ seq =atomic_add_return (RCU_DYNTICK_CTRL_CTR , &rdtp->dynticks); +WARN_ON_ONCE (IS_ENABLED (CONFIG_RCU_EQS_DEBUG ) && + !(seq &RCU_DYNTICK_CTRL_CTR )); +if (seq &RCU_DYNTICK_CTRL_MASK ) { +atomic_and (~RCU_DYNTICK_CTRL_MASK , &rdtp->dynticks); +smp_mb__after_atomic ();/* _exit after clearing mask. */ +/* Prefer duplicate flushes to losing a flush. */ +rcu_eqs_special_exit (); + }we have
atomic_andnot()for just these occasions :-)
Quoted 2 lines
I suppose that that could generate more efficient code on some architectures. I have changed this.
Right, saves 1 instruction on a number of archs. Not the end of the world of course, but since we have the thing might as well use it.
Quoted 23 lines
Quoted 14 lines
Quoted 14 lines
kernel/rcu/tree.c +14
-0+/* +* Set the special (bottom) bit of the specified CPU so that it +* will take special action (such as flushing its TLB) on the +* next exit from an extended quiescent state. Returns true if +* the bit was successfully set, or false if the CPU was not in +* an extended quiescent state. +*/ +bool rcu_eqs_special_set (int cpu) +{ +int old; +int new; +struct rcu_dynticks *rdtp = &per_cpu (rcu_dynticks, cpu); + + do {Quoted 8 lines
kernel/rcu/tree.c static bool rcu_dynticks_in_eqs_since(struct rcu_dynticks *rdtp, int snap) · line 436
Quoted 6 lines
kernel/rcu/tree.c +6
-0+ old =atomic_read (&rdtp->dynticks); +if (old &RCU_DYNTICK_CTRL_CTR ) +return false; + new = old |RCU_DYNTICK_CTRL_MASK ; + }while (atomic_cmpxchg (&rdtp->dynticks, old, new) != old); +return true;Is that what we call
atomic_fetch_or()?
Quoted 4 lines
I don't think so. The above code takes an early exit if the next bit up is set, which
atomic_fetch_or()does not. If the CPU is not in an extended quiescent state (old &RCU_DYNTICK_CTRL_CTR), then this code returns false to indicate that TLB shootdown cannot wait.
Oh duh yes, reading be hard.
Quoted 1 line
So it is more like a very specific form of
atomic_fetch_or_unless().
Right, I actually have a similar construct in set_nr_if_polling().
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 7 lines · Peter Zijlstra
Quoted 6 lines · Paul E. McKenney
Quoted 5 lines
I think I've asked this before, but why does this live in the guts of RCU?
Should we lift this state tracking stuff out and make RCU and
NOHZ(_FULL)users of it, or doesn't that make sense (reason)?
Quoted 6 lines
Quoted 2 lines
The dyntick-idle stuff is pretty specific to RCU. And what precisely would be helped by moving it?
Maybe untangle the inter-dependencies somewhat. It just seems a wee bit odd to have arch TLB invalidate depend on RCU implementation details like this.
I don't know about that. After all, my lazy TLB invalidation work in DYNIX/ptx was a key stepping-stone to my first RCU implementation. ;-)
More seriously, I don't believe moving it out would make it less odd.
Quoted 25 lines
Quoted 25 lines
But that was an excellent question, as it reminded me of RCU's dyntick-idle's NMI handling, and I never did ask Andy if it was OK for
rcu_eqs_special_exit()to be invoked when exiting NMI handler, which would currently happen. It would be easy for me to pass in a flag indicating whether or not the call is in NMI context, if that is needed.It is of course not possible to detect this at
rcu_eqs_special_set()time, becausercu_eqs_special_set()has no way of knowing that the next event that pulls the remote CPU out of idle will be an NMI.Quoted 14 lines
In any case, small nit below:
kernel/rcu/tree.c static void rcu_dynticks_eqs_enter(void) · line 339
Quoted 9 lines
kernel/rcu/tree.c +9
-0+ seq =atomic_add_return (RCU_DYNTICK_CTRL_CTR , &rdtp->dynticks); +WARN_ON_ONCE (IS_ENABLED (CONFIG_RCU_EQS_DEBUG ) && + !(seq &RCU_DYNTICK_CTRL_CTR )); +if (seq &RCU_DYNTICK_CTRL_MASK ) { +atomic_and (~RCU_DYNTICK_CTRL_MASK , &rdtp->dynticks); +smp_mb__after_atomic ();/* _exit after clearing mask. */ +/* Prefer duplicate flushes to losing a flush. */ +rcu_eqs_special_exit (); + }we have
atomic_andnot()for just these occasions :-)
Quoted 5 lines
Quoted 2 lines
I suppose that that could generate more efficient code on some architectures. I have changed this.
Right, saves 1 instruction on a number of archs. Not the end of the world of course, but since we have the thing might as well use it.
Understood -- could be worth the extra two characters of source code. I have made this change locally, and will push it to -rcu.
Quoted 23 lines
Quoted 23 lines
Quoted 22 lines
Quoted 10 lines
kernel/rcu/tree.c +10
-0+/* +* Set the special (bottom) bit of the specified CPU so that it +* will take special action (such as flushing its TLB) on the +* next exit from an extended quiescent state. Returns true if +* the bit was successfully set, or false if the CPU was not in +* an extended quiescent state. +*/ +bool rcu_eqs_special_set (int cpu) +{ +int old;kernel/rcu/tree.c static bool rcu_dynticks_in_eqs_since(struct rcu_dynticks *rdtp, int snap) · line 436
Quoted 10 lines
kernel/rcu/tree.c +10
-0+int new; +struct rcu_dynticks *rdtp = &per_cpu (rcu_dynticks, cpu); + +do { + old =atomic_read (&rdtp->dynticks); +if (old &RCU_DYNTICK_CTRL_CTR ) +return false; + new = old |RCU_DYNTICK_CTRL_MASK ; + }while (atomic_cmpxchg (&rdtp->dynticks, old, new) != old); +return true;Is that what we call
atomic_fetch_or()?
Quoted 6 lines
Quoted 4 lines
I don't think so. The above code takes an early exit if the next bit up is set, which
atomic_fetch_or()does not. If the CPU is not in an extended quiescent state (old &RCU_DYNTICK_CTRL_CTR), then this code returns false to indicate that TLB shootdown cannot wait.Oh duh yes, reading be hard.
I know that feeling!
Quoted 3 lines
Quoted 1 line
So it is more like a very specific form of
atomic_fetch_or_unless().Right, I actually have a similar construct in
set_nr_if_polling().
I was going to suggest combining them, but set_nr_if_polling() needs two different exit checks, and with two different return values. Not sure it is worth it, but of course if someone does come up with an appropriate primitive, I can always switch to it.
Thanx, Paul
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 7 lines · Peter Zijlstra
Quoted 6 lines · Paul E. McKenney
Quoted 5 lines
I think I've asked this before, but why does this live in the guts of RCU?
Should we lift this state tracking stuff out and make RCU and
NOHZ(_FULL)users of it, or doesn't that make sense (reason)?
Quoted 6 lines
Quoted 2 lines
The dyntick-idle stuff is pretty specific to RCU. And what precisely would be helped by moving it?
Maybe untangle the inter-dependencies somewhat. It just seems a wee bit odd to have arch TLB invalidate depend on RCU implementation details like this.
This came out of a courtyard discussion at KS/LPC. The idea is that this optimzation requires an atomic op that could be shared with RCU and that we probably care a lot more about this optimization on kernels with context tracking enabled, so putting it in RCU has nice performance properties. Other than that, it doesn't make a huge amount of sense.
Amusingly, Darwin appears to do something similar without an atomic op, and I have no idea why that's safe.
--Andy
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 22 lines · Andy Lutomirski
Quoted 7 lines · Peter Zijlstra
Quoted 6 lines · Paul E. McKenney
Quoted 5 lines
I think I've asked this before, but why does this live in the guts of RCU?
Should we lift this state tracking stuff out and make RCU and
NOHZ(_FULL)users of it, or doesn't that make sense (reason)?Quoted 6 lines
Quoted 2 lines
The dyntick-idle stuff is pretty specific to RCU. And what precisely would be helped by moving it?
Maybe untangle the inter-dependencies somewhat. It just seems a wee bit odd to have arch TLB invalidate depend on RCU implementation details like this.
This came out of a courtyard discussion at KS/LPC. The idea is that this optimzation requires an atomic op that could be shared with RCU and that we probably care a lot more about this optimization on kernels with context tracking enabled, so putting it in RCU has nice performance properties. Other than that, it doesn't make a huge amount of sense.
Quoted 2 lines
Amusingly, Darwin appears to do something similar without an atomic op, and I have no idea why that's safe.
Given that they run on ARM, I have no idea either. Maybe they don't need to be quite as bulletproof on idle-duration detection? Rumor has it that their variant of RCU uses program-counter ranges, so they wouldn't have the RCU tie-in -- just checks of program-counter ranges and interesting dependencies on the compiler.
Thanx, Paul
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>