Messages in this thread | | | Date | Tue, 8 Dec 2020 12:16:44 +0100 | From | Peter Zijlstra <> | Subject | Re: tick/sched: Make jiffies update quick check more robust |
| |
On Mon, Dec 07, 2020 at 03:41:47PM +0100, Thomas Gleixner wrote: > On Mon, Dec 07 2020 at 10:59, Peter Zijlstra wrote: > >> + if (IS_ENABLED(CONFIG_64BIT)) { > >> + if (ktime_before(now, smp_load_acquire(&tick_next_period))) > >> + return; > > > > Explicit ACQUIRE > > > >> + } else { > >> + unsigned int seq; > >> + > >> + /* > >> + * Avoid contention on jiffies_lock and protect the quick > >> + * check with the sequence count. > >> + */ > >> + do { > >> + seq = read_seqcount_begin(&jiffies_seq); > >> + nextp = tick_next_period; > >> + } while (read_seqcount_retry(&jiffies_seq, seq)); > >> + > >> + if (ktime_before(now, nextp)) > >> + return; > > > > Actually has an implicit ACQUIRE: > > > > read_seqcount_retry() implies smp_rmb(), which ensures > > LOAD->LOAD order, IOW any later load must happen after our > > @tick_next_period load. > > > > Then it has a control dependency on ktime_before(,nextp), which > > ensures LOAD->STORE order. > > > > Combined we have a LOAD->{LOAD,STORE} order on the > > @tick_next_period load, IOW ACQUIRE.
It's actually the whole of:
+ } while (read_seqcount_retry(&jiffies_seq, seq));
That implies the ACQUIRE, don't need the rest.
> >> + } > >> > >> + /* Quick check failed, i.e. update is required. */ > >> raw_spin_lock(&jiffies_lock); > > > > Another ACQUIRE, which means the above ACQUIRE only ensures we load the > > lock value after? > > > > Or are we trying to guarantee the caller is sure to observe the new > > jiffies value if we return? > > The guarantee we need on 64bit for the check w/o seqcount is: > > CPU0 CPU1 > > if (ktime_before(now, tick_next_period)) > return; > > raw_spin_lock(&jiffies_lock); > .... > jiffies_64 += ticks; > > tick_next_period = next; if (ktime_before(now, tick_next_period)) > return; > > When CPU1 returns because it observes the new value in tick_next_period > then it has to be guaranteed that jiffies_64 is observable as well.
Right, it does that. Good.
| |