Do not keep timekeeping CPU tick running for non-nohz_full= CPUs

If a non-nohz_full= CPU is non-idle, it will have a scheduling-clock interrupt, and therefore doesn't need the timekeeping CPU to keep its scheduling-clock interrupt going. This commit therefore ignores the idle state of non-nohz_full CPUs when determining whether or not the timekeeping CPU can safely turn off its scheduling-clock interrupt.

Signed-off-by Paul E. McKenney <[email protected]>

kernel/rcu/tree_plugin.h +9 -4

index ddad959a9132..eaa32e4c228d 100644
@@ -2789,8 +2789,13 @@ static void rcu_sysidle_exit(struct rcu_dynticks *rdtp, int irq)
 	 * system-idle state.  This means that the timekeeping CPU must
 	 * invoke rcu_sysidle_force_exit() directly if it does anything
 	 * more than take a scheduling-clock interrupt.
+	 *
+	 * In addition if we are not a nohz_full= CPU, then when we are
+	 * non-idle we have our own tick, so we don't need the timekeeping
+	 * CPU to keep a tick on our behalf.  We assume that the timekeeping
+	 * CPU is also a nohz_full= CPU.
 	 */
-	if (smp_processor_id() == tick_do_timer_cpu)
+	if (!tick_nohz_full_cpu(smp_processor_id()))
 		return;
 
 	/* Update system-idle state: We are clearly no longer fully idle! */

@@ -2810,11 +2815,11 @@ static void rcu_sysidle_check_cpu(struct rcu_data *rdp, bool *isidle,
 
 	/*
 	 * If some other CPU has already reported non-idle, if this is
-	 * not the flavor of RCU that tracks sysidle state, or if this
-	 * is an offline or the timekeeping CPU, nothing to do.
+	 * not the flavor of RCU that tracks sysidle state, or if this is
+	 * an offline or !nohz_full= or the timekeeping CPU, nothing to do.
 	 */
 	if (!*isidle || rdp->rsp != rcu_sysidle_state ||
-	    cpu_is_offline(rdp->cpu) || rdp->cpu == tick_do_timer_cpu)
+	    cpu_is_offline(rdp->cpu) || !tick_nohz_full_cpu(rdp->cpu))
 		return;
 	if (rcu_gp_in_progress(rdp->rsp))
 		WARN_ON_ONCE(smp_processor_id() != tick_do_timer_cpu);

If a non-nohz_full= CPU is non-idle, it will have a scheduling-clock interrupt, and therefore doesn't need the timekeeping CPU to keep its scheduling-clock interrupt going. This commit therefore ignores the idle state of non-nohz_full CPUs when determining whether or not the timekeeping CPU can safely turn off its scheduling-clock interrupt.

Signed-off-by Paul E. McKenney <[email protected]>

kernel/rcu/tree_plugin.h +9 -4

index ddad959a9132..eaa32e4c228d 100644
@@ -2789,8 +2789,13 @@ static void rcu_sysidle_exit(struct rcu_dynticks *rdtp, int irq)
 	 * system-idle state.  This means that the timekeeping CPU must
 	 * invoke rcu_sysidle_force_exit() directly if it does anything
 	 * more than take a scheduling-clock interrupt.
+	 *
+	 * In addition if we are not a nohz_full= CPU, then when we are
+	 * non-idle we have our own tick, so we don't need the timekeeping
+	 * CPU to keep a tick on our behalf.  We assume that the timekeeping
+	 * CPU is also a nohz_full= CPU.
 	 */
-	if (smp_processor_id() == tick_do_timer_cpu)
+	if (!tick_nohz_full_cpu(smp_processor_id()))
 		return;
 
 	/* Update system-idle state: We are clearly no longer fully idle! */

@@ -2810,11 +2815,11 @@ static void rcu_sysidle_check_cpu(struct rcu_data *rdp, bool *isidle,
 
 	/*
 	 * If some other CPU has already reported non-idle, if this is
-	 * not the flavor of RCU that tracks sysidle state, or if this
-	 * is an offline or the timekeeping CPU, nothing to do.
+	 * not the flavor of RCU that tracks sysidle state, or if this is
+	 * an offline or !nohz_full= or the timekeeping CPU, nothing to do.
 	 */
 	if (!*isidle || rdp->rsp != rcu_sysidle_state ||
-	    cpu_is_offline(rdp->cpu) || rdp->cpu == tick_do_timer_cpu)
+	    cpu_is_offline(rdp->cpu) || !tick_nohz_full_cpu(rdp->cpu))
 		return;
 	if (rcu_gp_in_progress(rdp->rsp))
 		WARN_ON_ONCE(smp_processor_id() != tick_do_timer_cpu);

This can be changed by allowing timekeeping duty from all non-nohz_full CPUs, that's the initial direction I took, but it involved a lot of complications and scalability issues.

Quoted 3 lines

This can be changed by allowing timekeeping duty from all non-nohz_full CPUs, that's the initial direction I took, but it involved a lot of complications and scalability issues.

How so, currently any CPU can be timekeeper, how is any !nohz_full cpu different?

Quoted 7 lines · Paul E. McKenney

If a non-nohz_full= CPU is non-idle, it will have a scheduling-clock interrupt, and therefore doesn't need the timekeeping CPU to keep its scheduling-clock interrupt going. This commit therefore ignores the idle state of non-nohz_full CPUs when determining whether or not the timekeeping CPU can safely turn off its scheduling-clock interrupt.

Signed-off-by Paul E. McKenney <[email protected]>

Unfortunately that's not how things work. Running a CPU tick doesn't necessarily imply to run the timekeeping duty.

Only the timekeeper can update the timekeeping. There is an exception though:
the timekeeping is also updated by dynticks idle CPUs when they wake up in an interrupt from idle.

Here is in practice why it doesn't work:

So lets say CPU 0 is timekeeper, CPU 1 a non-nohz-full CPU and all others are full-nohz. CPU 0 is sleeping. CPU 1 wakes up from idle, so it has an uptodate timekeeping but then if it continues to execute further without waking up CPU 0, it risks stale timestamps.

This can be changed by allowing timekeeping duty from all non-nohz_full CPUs, that's the initial direction I took, but it involved a lot of complications and scalability issues.

Quoted 35 lines

kernel/rcu/tree_plugin.h +9 -4

index ddad959a9132..eaa32e4c228d 100644
@@ -2789,8 +2789,13 @@ static void rcu_sysidle_exit(struct rcu_dynticks *rdtp, int irq)
 	 * system-idle state.  This means that the timekeeping CPU must
 	 * invoke rcu_sysidle_force_exit() directly if it does anything
 	 * more than take a scheduling-clock interrupt.
+	 *
+	 * In addition if we are not a nohz_full= CPU, then when we are
+	 * non-idle we have our own tick, so we don't need the timekeeping
+	 * CPU to keep a tick on our behalf.  We assume that the timekeeping
+	 * CPU is also a nohz_full= CPU.
 	 */
-	if (smp_processor_id() == tick_do_timer_cpu)
+	if (!tick_nohz_full_cpu(smp_processor_id()))
 		return;
 
 	/* Update system-idle state: We are clearly no longer fully idle! */

@@ -2810,11 +2815,11 @@ static void rcu_sysidle_check_cpu(struct rcu_data *rdp, bool *isidle,
 
 	/*
 	 * If some other CPU has already reported non-idle, if this is
-	 * not the flavor of RCU that tracks sysidle state, or if this
-	 * is an offline or the timekeeping CPU, nothing to do.
+	 * not the flavor of RCU that tracks sysidle state, or if this is
+	 * an offline or !nohz_full= or the timekeeping CPU, nothing to do.
 	 */
 	if (!*isidle || rdp->rsp != rcu_sysidle_state ||
-	    cpu_is_offline(rdp->cpu) || rdp->cpu == tick_do_timer_cpu)
+	    cpu_is_offline(rdp->cpu) || !tick_nohz_full_cpu(rdp->cpu))
 		return;
 	if (rcu_gp_in_progress(rdp->rsp))
 		WARN_ON_ONCE(smp_processor_id() != tick_do_timer_cpu);

So we really have to have -all- the CPUs be idle to turn off the timekeeper.

This won't make the battery-powered embedded guys happy...

Other thoughts on this? We really should not be setting CONFIG_NO_HZ_FULL_SYSIDLE by default until this is solved.

Quoted 1 line · Paul E. McKenney

So we really have to have -all- the CPUs be idle to turn off the timekeeper.

That seems to be pretty unavoidable any which way around.

Quoted 4 lines

This won't make the battery-powered embedded guys happy...

Other thoughts on this? We really should not be setting CONFIG_NO_HZ_FULL_SYSIDLE by default until this is solved.

What are those same guys doing with nohz_full to begin with?

Quoted 4 lines · Peter Zijlstra

Quoted 1 line · Paul E. McKenney

So we really have to have -all- the CPUs be idle to turn off the timekeeper.

That seems to be pretty unavoidable any which way around.

Hmmm... The exception would be the likely common case where none of the CPUs are flagged as nohz_full= CPUs. If we handled that case as if CONFIG_NO_HZ_FULL=n, we would have handled almost all of the problem.

Quoted 6 lines

Quoted 4 lines

This won't make the battery-powered embedded guys happy...

Other thoughts on this? We really should not be setting CONFIG_NO_HZ_FULL_SYSIDLE by default until this is solved.

What are those same guys doing with nohz_full to begin with?

If CONFIG_NO_HZ_FULL_SYSIDLE=y is the default, my main concern is for people who didn't really want it, and who thus did not set the nohz_full= boot parameter. Hence my suggestion above that we treat that case as if CONFIG_NO_HZ_FULL=n (and thus also as if CONFIG_NO_HZ_FULL_SYSIDLE=n).

There have been some people saying that they want only a subset of their CPUs in nohz_full= state, and these guys seem to want to run a mixed workload. For example, they have HPC (or RT) workloads on the nohz_full= CPUs, and also want normal high-throughput processing on the remaining CPUs. If software was trivial (and making other unlikely assumptions about the perfection of the world and the invalidity of Murphy's lawy), we would want the timekeeping CPU to be able to move among the non-nohz_full= CPUs.

However, this should be a small fraction of the users, and many of these guys would probably be open to making a few changes. Thus, a less-proactive approach should allow us to solve their actual problems, as opposed to the problems that we speculate that they might encounter. ;-)

Quoted 6 lines · Paul E. McKenney

Quoted 4 lines · Peter Zijlstra

Quoted 1 line · Paul E. McKenney

So we really have to have -all- the CPUs be idle to turn off the timekeeper.

That seems to be pretty unavoidable any which way around.

Quoted 4 lines

Hmmm... The exception would be the likely common case where none of the CPUs are flagged as nohz_full= CPUs. If we handled that case as if CONFIG_NO_HZ_FULL=n, we would have handled almost all of the problem.

You mean that is not currently the case? Yes that seems like a fairly sane thing to do.

Quoted 11 lines

Quoted 6 lines

Quoted 4 lines

This won't make the battery-powered embedded guys happy...

Other thoughts on this? We really should not be setting CONFIG_NO_HZ_FULL_SYSIDLE by default until this is solved.

What are those same guys doing with nohz_full to begin with?

If CONFIG_NO_HZ_FULL_SYSIDLE=y is the default, my main concern is for people who didn't really want it, and who thus did not set the nohz_full= boot parameter. Hence my suggestion above that we treat that case as if CONFIG_NO_HZ_FULL=n (and thus also as if CONFIG_NO_HZ_FULL_SYSIDLE=n).

ack

Quoted 8 lines

There have been some people saying that they want only a subset of their CPUs in nohz_full= state, and these guys seem to want to run a mixed workload. For example, they have HPC (or RT) workloads on the nohz_full= CPUs, and also want normal high-throughput processing on the remaining CPUs. If software was trivial (and making other unlikely assumptions about the perfection of the world and the invalidity of Murphy's lawy), we would want the timekeeping CPU to be able to move among the non-nohz_full= CPUs.

Yeah, I don't see a problem with that, but then I'm not entirely sure why we use RCU to track system idle state.

Quoted 4 lines

However, this should be a small fraction of the users, and many of these guys would probably be open to making a few changes. Thus, a less-proactive approach should allow us to solve their actual problems, as opposed to the problems that we speculate that they might encounter. ;-)

But you still haven't talked about the battery people... I don't think nohz_full is something they should care about / use.

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Paul E. McKenney

Quoted 4 lines · Peter Zijlstra

Quoted 1 line · Paul E. McKenney

So we really have to have -all- the CPUs be idle to turn off the timekeeper.

That seems to be pretty unavoidable any which way around.

Hmmm... The exception would be the likely common case where none of the CPUs are flagged as nohz_full= CPUs. If we handled that case as if CONFIG_NO_HZ_FULL=n, we would have handled almost all of the problem.

Quoted 2 lines

You mean that is not currently the case? Yes that seems like a fairly sane thing to do.

Hard to say -- need to see where Frederic is putting the call to rcu_sys_is_idle(). On the RCU side, I could potentially lower overhead by checking tick_nohz_full_enabled() in a few functions.

Quoted 13 lines · Paul E. McKenney

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Paul E. McKenney

Quoted 4 lines · Peter Zijlstra

Quoted 1 line · Paul E. McKenney

So we really have to have -all- the CPUs be idle to turn off the timekeeper.

That seems to be pretty unavoidable any which way around.

Hmmm... The exception would be the likely common case where none of the CPUs are flagged as nohz_full= CPUs. If we handled that case as if CONFIG_NO_HZ_FULL=n, we would have handled almost all of the problem.

Quoted 6 lines

Quoted 2 lines

You mean that is not currently the case? Yes that seems like a fairly sane thing to do.

Hard to say -- need to see where Frederic is putting the call to rcu_sys_is_idle(). On the RCU side, I could potentially lower overhead by checking tick_nohz_full_enabled() in a few functions.

Yeah you definetly can.

Just put this in the very beginning of rcu_sys_is_idle():

            if (tick_nohz_full_enabled())
                return true;

That imply perhaps a more appropriate name like rcu_sys_need_timekeeper(), with inverted condition.

Quoted 21 lines · Frederic Weisbecker

Quoted 19 lines · Paul E. McKenney

Quoted 7 lines · Peter Zijlstra

Quoted 6 lines · Paul E. McKenney

Quoted 4 lines · Peter Zijlstra

Quoted 1 line · Paul E. McKenney

So we really have to have -all- the CPUs be idle to turn off the timekeeper.

That seems to be pretty unavoidable any which way around.

Quoted 7 lines

Quoted 4 lines

Hmmm... The exception would be the likely common case where none of the CPUs are flagged as nohz_full= CPUs. If we handled that case as if CONFIG_NO_HZ_FULL=n, we would have handled almost all of the problem.

You mean that is not currently the case? Yes that seems like a fairly sane thing to do.

Hard to say -- need to see where Frederic is putting the call to rcu_sys_is_idle(). On the RCU side, I could potentially lower overhead by checking tick_nohz_full_enabled() in a few functions.

Quoted 6 lines

Yeah you definetly can.

Just put this in the very beginning of rcu_sys_is_idle():

            if (tick_nohz_full_enabled())
                return true;

That would be !tick_nohz_full_enabled(), right? But please see below.

Quoted 3 lines

That imply perhaps a more appropriate name like rcu_sys_need_timekeeper(), with inverted condition.

Ah, I thought that you already avoided invoking rcu_sys_is_idle() when !tick_nohz_full_enabled(), so I didn't add a check to that function. Are you planning to change this? Or am I having eyesight problems?

Quoted 4 lines · Peter Zijlstra

Quoted 1 line · Paul E. McKenney

So we really have to have -all- the CPUs be idle to turn off the timekeeper.

That seems to be pretty unavoidable any which way around.

Hmmm... The exception would be the likely common case where none of the CPUs are flagged as nohz_full= CPUs. If we handled that case as if CONFIG_NO_HZ_FULL=n, we would have handled almost all of the problem.

Quoted 7 lines

Quoted 5 lines

This won't make the battery-powered embedded guys happy...

Other thoughts on this? We really should not be setting CONFIG_NO_HZ_FULL_SYSIDLE by default until this is solved.

What are those same guys doing with nohz_full to begin with?

If CONFIG_NO_HZ_FULL_SYSIDLE=y is the default, my main concern is for people who didn't really want it, and who thus did not set the nohz_full= boot parameter. Hence my suggestion above that we treat that case as if CONFIG_NO_HZ_FULL=n (and thus also as if CONFIG_NO_HZ_FULL_SYSIDLE=n).

There have been some people saying that they want only a subset of their CPUs in nohz_full= state, and these guys seem to want to run a mixed workload. For example, they have HPC (or RT) workloads on the nohz_full= CPUs, and also want normal high-throughput processing on the remaining CPUs. If software was trivial (and making other unlikely assumptions about the perfection of the world and the invalidity of Murphy's lawy), we would want the timekeeping CPU to be able to move among the non-nohz_full= CPUs.

However, this should be a small fraction of the users, and many of these guys would probably be open to making a few changes. Thus, a less-proactive approach should allow us to solve their actual problems, as opposed to the problems that we speculate that they might encounter. ;-)

Thanx, Paul

Quoted 22 lines · Frederic Weisbecker

Quoted 7 lines · Paul E. McKenney

If a non-nohz_full= CPU is non-idle, it will have a scheduling-clock interrupt, and therefore doesn't need the timekeeping CPU to keep its scheduling-clock interrupt going. This commit therefore ignores the idle state of non-nohz_full CPUs when determining whether or not the timekeeping CPU can safely turn off its scheduling-clock interrupt.

Signed-off-by Paul E. McKenney <[email protected]>

Unfortunately that's not how things work. Running a CPU tick doesn't necessarily imply to run the timekeeping duty.

Only the timekeeper can update the timekeeping. There is an exception though:
the timekeeping is also updated by dynticks idle CPUs when they wake up in an interrupt from idle.

Here is in practice why it doesn't work:

So lets say CPU 0 is timekeeper, CPU 1 a non-nohz-full CPU and all others are full-nohz. CPU 0 is sleeping. CPU 1 wakes up from idle, so it has an uptodate timekeeping but then if it continues to execute further without waking up CPU 0, it risks stale timestamps.

Quoted 3 lines

This can be changed by allowing timekeeping duty from all non-nohz_full CPUs, that's the initial direction I took, but it involved a lot of complications and scalability issues.

So we really have to have -all- the CPUs be idle to turn off the timekeeper. This won't make the battery-powered embedded guys happy...

Other thoughts on this? We really should not be setting CONFIG_NO_HZ_FULL_SYSIDLE by default until this is solved.

Quoted 23 lines · Paul E. McKenney

Quoted 22 lines · Frederic Weisbecker

Quoted 7 lines · Paul E. McKenney

If a non-nohz_full= CPU is non-idle, it will have a scheduling-clock interrupt, and therefore doesn't need the timekeeping CPU to keep its scheduling-clock interrupt going. This commit therefore ignores the idle state of non-nohz_full CPUs when determining whether or not the timekeeping CPU can safely turn off its scheduling-clock interrupt.

Signed-off-by Paul E. McKenney <[email protected]>

Unfortunately that's not how things work. Running a CPU tick doesn't necessarily imply to run the timekeeping duty.

Only the timekeeper can update the timekeeping. There is an exception though:
the timekeeping is also updated by dynticks idle CPUs when they wake up in an interrupt from idle.

Here is in practice why it doesn't work:

So lets say CPU 0 is timekeeper, CPU 1 a non-nohz-full CPU and all others are full-nohz. CPU 0 is sleeping. CPU 1 wakes up from idle, so it has an uptodate timekeeping but then if it continues to execute further without waking up CPU 0, it risks stale timestamps.

Quoted 6 lines

Quoted 3 lines

This can be changed by allowing timekeeping duty from all non-nohz_full CPUs, that's the initial direction I took, but it involved a lot of complications and scalability issues.

So we really have to have -all- the CPUs be idle to turn off the timekeeper. This won't make the battery-powered embedded guys happy...

I can imagine all sorts of solutions to solve this. None of them look simple though. And I'm really convinced this isn't worth until some user comes up and report me that 1) he seriously uses full dynticks and 2) he needs non-full-nohz CPUs other than CPU 0.

If 1 and 2 ever happen, I'll gladly work on this.

Quoted 2 lines

Other thoughts on this? We really should not be setting CONFIG_NO_HZ_FULL_SYSIDLE by default until this is solved.

Well it's better to save energy when all CPUs are idle than never.

Quoted 36 lines · Frederic Weisbecker

Quoted 23 lines · Paul E. McKenney

Quoted 22 lines · Frederic Weisbecker

Quoted 7 lines · Paul E. McKenney

If a non-nohz_full= CPU is non-idle, it will have a scheduling-clock interrupt, and therefore doesn't need the timekeeping CPU to keep its scheduling-clock interrupt going. This commit therefore ignores the idle state of non-nohz_full CPUs when determining whether or not the timekeeping CPU can safely turn off its scheduling-clock interrupt.

Signed-off-by Paul E. McKenney <[email protected]>

Unfortunately that's not how things work. Running a CPU tick doesn't necessarily imply to run the timekeeping duty.

Only the timekeeper can update the timekeeping. There is an exception though:
the timekeeping is also updated by dynticks idle CPUs when they wake up in an interrupt from idle.

Here is in practice why it doesn't work:

So lets say CPU 0 is timekeeper, CPU 1 a non-nohz-full CPU and all others are full-nohz. CPU 0 is sleeping. CPU 1 wakes up from idle, so it has an uptodate timekeeping but then if it continues to execute further without waking up CPU 0, it risks stale timestamps.

Quoted 6 lines

Quoted 3 lines

This can be changed by allowing timekeeping duty from all non-nohz_full CPUs, that's the initial direction I took, but it involved a lot of complications and scalability issues.

So we really have to have -all- the CPUs be idle to turn off the timekeeper. This won't make the battery-powered embedded guys happy...

I can imagine all sorts of solutions to solve this. None of them look simple though. And I'm really convinced this isn't worth until some user comes up and report me that 1) he seriously uses full dynticks and 2) he needs non-full-nohz CPUs other than CPU 0.

Quoted 1 line

If 1 and 2 ever happen, I'll gladly work on this.

Does the thought of special-casing the situation where CONFIG_NO_HZ_FULL=y, CONFIG_NO_HZ_FULL_SYSIDLE=y, and there are no nohz_full= CPUs make sense?

Quoted 5 lines

Quoted 3 lines

Other thoughts on this? We really should not be setting CONFIG_NO_HZ_FULL_SYSIDLE by default until this is solved.

Well it's better to save energy when all CPUs are idle than never.

Fair point!

Thanx, Paul

Yes. Distros seem to want to make full dynticks available for users but they also want the off case (when nohz_full= isn't passed) to keep the lowest overhead as possible.