lkml.org 
[lkml]   [2011]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] specific do_timer_cpu value for nohz off mode
On Thu, Dec 01, 2011 at 02:56:23PM -0800, Andrew Morton wrote:
> On Thu, 1 Dec 2011 10:37:40 -0600
> Dimitri Sivanich <sivanich@sgi.com> wrote:
>
> > +static ssize_t sysfs_store_do_timer_cpu(struct sys_device *dev,
> > + struct sysdev_attribute *attr,
> > + const char *buf, size_t size)
> > +{
> > + struct sysdev_ext_attribute *ea = SYSDEV_TO_EXT_ATTR(attr);
> > + unsigned int new;
> > + int rv;
> > +
> > +#ifdef CONFIG_NO_HZ
> > + /* nohz mode not supported */
> > + if (tick_nohz_enabled)
> > + return -EINVAL;
> > +#endif
> > +
> > + rv = kstrtouint(buf, 0, &new);
> > + if (rv)
> > + return rv;
> > +
> > + /* Protect against cpu-hotplug */
> > + get_online_cpus();
> > +
> > + if (new >= nr_cpu_ids || !cpu_online(new)) {
> > + put_online_cpus();
> > + return -ERANGE;
> > + }
> > +
> > + *(unsigned int *)(ea->var) = new;
> > +
> > + put_online_cpus();
> > +
> > + return size;
> > +}
>
> OK, I think this fixes one race. We modify tick_do_timer_cpu inside
> get_online_cpus(). If that cpu goes offline then
> tick_handover_do_timer() will correctly hand the timer functions over
> to a new CPU, and tick_handover_do_timer() runs in the CPU hotplug
> handler which I assume is locked by get_online_cpus(). Please check
> all this.

Yes, _cpu_down() runs cpu_hotplug_begin(), which locks and holds the mutex
that get_online_cpus() needs in order to update the refcount
(cpu_hotplug_begin doesn't exit until refcount==0).

The notification that calls tick_handover_do_timer() is done in both the
CPU_DYING and CPU_DYING_FROZEN (CPU_TASKS_FROZEN), but I believe this always
comes from _cpu_down() in either case.

>
> Now, the above code can alter tick_do_timer_cpu while a timer interrupt
> is actually executing on another CPU. Will this disrupt aything? I
> think it might cause problems. If we take an interrupt on CPU 5 and
> that CPU enters tick_periodic() and another CPU alters
> tick_do_timer_cpu from 5 to 4 at exactly the correct time, tick_periodic()
> might fail to run do_timer(). Or it might run do_timer() on both CPUs 4 and
> 5 concurrently?
>

Well, we do have to take the write_seqlock() in tick_periodic, so there's
no danger of do_timer running exactly concurrently.

But yes, we may end up with 2 jiffies ticks occurring close together
(when 5 runs do_timer while 4 waits for the seqlock), or we might end up
missing a jiffies update for almost a full tick (when it changes from 5
to 4 immediately after 4 has done the 'tick_do_timer_cpu == cpu' check).

So at that time, we could be off +- almost a tick. The question is, how
critical is that? When you down a cpu, the same sort of thing could
happen via tick_handover_do_timer(), which itself does nothing more than
change tick_do_timer_cpu.


\
 
 \ /
  Last update: 2011-12-02 21:17    [from the cache]
©2003-2011 Jasper Spaans