Re: [PATCH RFC nohz_full v2 6/7] nohz_full: Add full-system-idlestate machine
From: Paul E. McKenney
Date: Mon Jul 01 2013 - 14:10:57 EST
On Mon, Jul 01, 2013 at 06:35:31PM +0200, Frederic Weisbecker wrote:
>
On Fri, Jun 28, 2013 at 01:10:21PM -0700, Paul E. McKenney wrote:
>
> /*
>
> + * Unconditionally force exit from full system-idle state. This is
>
> + * invoked when a normal CPU exits idle, but must be called separately
>
> + * for the timekeeping CPU (tick_do_timer_cpu). The reason for this
>
> + * is that the timekeeping CPU is permitted to take scheduling-clock
>
> + * interrupts while the system is in system-idle state, and of course
>
> + * rcu_sysidle_exit() has no way of distinguishing a scheduling-clock
>
> + * interrupt from any other type of interrupt.
>
> + */
>
> +void rcu_sysidle_force_exit(void)
>
> +{
>
> + int oldstate = ACCESS_ONCE(full_sysidle_state);
>
> + int newoldstate;
>
> +
>
> + /*
>
> + * Each pass through the following loop attempts to exit full
>
> + * system-idle state. If contention proves to be a problem,
>
> + * a trylock-based contention tree could be used here.
>
> + */
>
> + while (oldstate > RCU_SYSIDLE_SHORT) {
>
> + newoldstate = cmpxchg(&full_sysidle_state,
>
> + oldstate, RCU_SYSIDLE_NOT);
>
> + if (oldstate == newoldstate &&
>
> + oldstate == RCU_SYSIDLE_FULL_NOTED) {
>
> + rcu_kick_nohz_cpu(tick_do_timer_cpu);
>
> + return; /* We cleared it, done! */
>
> + }
>
> + oldstate = newoldstate;
>
> + }
>
> + smp_mb(); /* Order initial oldstate fetch vs. later non-idle work. */
>
> +}
>
> +
>
> +/*
>
> * Invoked to note entry to irq or task transition from idle. Note that
>
> * usermode execution does -not- count as idle here! The caller must
>
> * have disabled interrupts.
>
> @@ -2474,6 +2506,214 @@ static void rcu_sysidle_exit(struct rcu_dynticks *rdtp, int irq)
>
> atomic_inc(&rdtp->dynticks_idle);
>
> smp_mb__after_atomic_inc();
>
> WARN_ON_ONCE(!(atomic_read(&rdtp->dynticks_idle) & 0x1));
>
> +
>
> + /*
>
> + * If we are the timekeeping CPU, we are permitted to be non-idle
>
> + * during a system-idle state. This must be the case, because
>
> + * the timekeeping CPU has to take scheduling-clock interrupts
>
> + * during the time that the system is transitioning to full
>
> + * 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.
>
> + */
>
> + if (smp_processor_id() == tick_do_timer_cpu)
>
> + return;
>
> +
>
> + /* Update system-idle state: We are clearly no longer fully idle! */
>
> + rcu_sysidle_force_exit();
>
> +}
>
> +
>
> +/*
>
> + * Check to see if the current CPU is idle. Note that usermode execution
>
> + * does not count as idle. The caller must have disabled interrupts.
>
> + */
>
> +static void rcu_sysidle_check_cpu(struct rcu_data *rdp, bool *isidle,
>
> + unsigned long *maxj)
>
> +{
>
> + int cur;
>
> + int curnmi;
>
> + unsigned long j;
>
> + struct rcu_dynticks *rdtp = rdp->dynticks;
>
> +
>
> + /*
>
> + * 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.
>
> + */
>
> + if (!*isidle || rdp->rsp != rcu_sysidle_state ||
>
> + cpu_is_offline(rdp->cpu) || rdp->cpu == tick_do_timer_cpu)
>
> + return;
>
> + /* WARN_ON_ONCE(smp_processor_id() != tick_do_timer_cpu); */
>
> +
>
> + /*
>
> + * Pick up current idle and NMI-nesting counters, check. We check
>
> + * for NMIs using RCU's main ->dynticks counter. This works because
>
> + * any time ->dynticks has its low bit set, ->dynticks_idle will
>
> + * too -- unless the only reason that ->dynticks's low bit is set
>
> + * is due to an NMI from idle. Which is exactly the case we need
>
> + * to account for.
>
> + */
>
> + cur = atomic_read(&rdtp->dynticks_idle);
>
> + curnmi = atomic_read(&rdtp->dynticks);
>
> + if ((cur & 0x1) || (curnmi & 0x1)) {
>
>
I think you wanted to ignore NMIs this time because they don't read walltime?
>
>
By the way they can still read jiffies, but unlike irq_enter(), nmi_enter()
>
don't catch up with missing jiffies update. So the behaviour doesn't change
>
compared to !NO_HZ_FULL.
You are right, I missed this when ripping out NMI handling. Will fix!
>
> + *isidle = 0; /* We are not idle! */
>
> + return;
>
> + }
>
> + smp_mb(); /* Read counters before timestamps. */
>
> +
>
> + /* Pick up timestamps. */
>
> + j = ACCESS_ONCE(rdtp->dynticks_idle_jiffies);
>
> + /* If this CPU entered idle more recently, update maxj timestamp. */
>
> + if (ULONG_CMP_LT(*maxj, j))
>
> + *maxj = j;
>
>
So I'm a bit confused with the ordering so I'm probably going to ask a silly question.
>
>
What makes sure that we are not reading a stale value of rdtp->dynticks_idle
>
in the following scenario:
>
>
CPU 0 CPU 1
>
>
//CPU 1 idle
>
//rdtp(1)->dynticks_idle == 0
>
>
sysidle_check_cpu(CPU 1) {
>
rdtp(1)->dynticks_idle == 0
>
}
>
cmpxchg(full_sysidle_state,
>
...RCU_SYSIDLE_SHORT)
>
rcu_irq_exit() {
rcu_irq_enter(), right?
>
rdtp(1)->dynticks_idle = 1
>
smp_mb()
>
rcu_sysidle_force_exit() {
>
full_sysidle_state == RCU_SYSIDLE_SHORT
>
// no cmpxchg
>
smp_mb()
>
...
>
>
[1]
>
sysidle_check_cpu(CPU 1) {
>
rdtp(1)->dynticks_idle == 0
>
}
>
>
cmpxchg(RCU_SYSIDLE_FULL, ...)
You know, I had an RCU_SYSIDLE_LONG state for this purpose, but later
convinced myself that I didn't need it. :-/
Time to go put it back in, and thank you for your careful review!
Thanx, Paul
>
[2]
>
sysidle_check_cpu(CPU 1) {
>
rdtp(1)->dynticks_idle == 0
>
}
>
>
cmpxchg(RCU_SYSIDLE_FULL_NOTED, ...)
>
>
>
I mean in [1] and [2] I can't see something in the ordering that guarantees that we see
>
the new value rdtp(1)->dynticks_idle == 1.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
http://www.tux.org/lkml/