Messages in this thread | | | Date | Fri, 30 Nov 2012 18:38:16 +0100 | Subject | Re: [PATCH] nohz/cpuset: Make a CPU stick with do_timer() duty in the presence of nohz cpusets | From | Frederic Weisbecker <> |
| |
2012/11/28 Hakan Akkan <hakanakkan@gmail.com>: > +static int check_drop_timer_duty(int cpu) > +{ > + int curr_handler, prev_handler, new_handler; > + int nrepeat = -1; > + bool drop_recheck; > + > +repeat: > + WARN_ON_ONCE(++nrepeat > 1); > + drop_recheck = false; > + curr_handler = cpu; > + new_handler = TICK_DO_TIMER_NONE; > + > +#ifdef CONFIG_CPUSETS_NO_HZ > + if (atomic_read(&nr_cpus_user_nohz) > 0) {
Note atomic_read() is not SMP ordered. If another CPU does atomic_add() or atomic_add_return(), you may not see the new value as expected with atomic_read(). Doing atomic_add_return(0, &atomic_thing) returns the fully ordered result.
You also need to do that to ensure full ordering against tick_cpu_sched.user_nohz.
On the current layout we have:
(Write side) (Read side)
ts->user_nohz = 1; atomic_inc(&nr_cpus_user_nohz)
if (atomic_read(&nr_cpus_user_nohz)) if (per_cpu(tick_cpu_sched, curr_handler).user_nohz) ....
If you want to make sure that you see the expected value on user_nohz from the read side, you need to correctly order the write and read against nr_cpus_user_nohz.
For this you can use atomic_add_return() which implies the full barrier:
(Write side) (Read side)
ts->user_nohz = 1; atomic_inc_return(&nr_cpus_user_nohz)
if (atomic_add_return(0, &nr_cpus_user_nohz)) if (per_cpu(tick_cpu_sched, curr_handler).user_nohz) ....
I have much more comments on this patch, I will come back on this soon.
Thanks.
| |