lkml.org 
[lkml]   [2013]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 09/13] nohz: Allow timekeeper's tick to stop when all full dynticks CPUs are idle
    On Tue, Dec 17, 2013 at 03:51:17PM -0800, Paul E. McKenney wrote:
    > On Tue, Dec 17, 2013 at 11:51:28PM +0100, Frederic Weisbecker wrote:
    > > +/*
    > > + * Fetch max deferment for the current clockevent source until it overflows.
    > > + * Also in full dynticks environment, make sure the current timekeeper
    > > + * stays periodic until some other CPU can take its timekeeping duty
    > > + * or until all full dynticks go to sleep.
    > > + */
    > > +static u64 tick_timekeeping_max_deferment(struct tick_sched *ts)
    > > +{
    > > + int cpu;
    > > + u64 ret = KTIME_MAX;
    > > +
    > > + /*
    > > + * Fast path for full dynticks off-case: skip to
    > > + * clockevent max deferment
    > > + */
    > > + if (!tick_nohz_full_enabled())
    > > + return timekeeping_max_deferment();
    > > +
    > > + cpu = smp_processor_id();
    > > +
    > > + /* Full dynticks CPU don't take timekeeping duty */
    > > + if (!tick_timekeeping_cpu(cpu))
    > > + return timekeeping_max_deferment();
    > > +
    > > + /*
    > > + * If we are the timekeeper and all full dynticks CPUs are idle,
    > > + * then we can finally sleep.
    > > + */
    > > + if (tick_do_timer_cpu == cpu ||
    > > + (tick_do_timer_cpu == TICK_DO_TIMER_NONE && ts->do_timer_last == 1)) {
    > > + if (!rcu_sys_is_idle()) {
    >
    > So multiple CPUs could call rcu_sys_is_idle()? Seems like this could
    > happen if tick_do_timer_cpu == TICK_DO_TIMER_NONE. This would be OK only
    > if tick_timekeeping_cpu() returns true for one and only one of the CPUs
    > at any given range of time -- and also that no one calls rcu_sys_is_idle()
    > during a timekeeping CPU handoff.

    Hmm yeah I fear we can have concurrent callers of this at a same time range.

    >
    > If two different CPUs call rcu_sys_is_idle() anywhere nearly concurrently
    > on a small system (CONFIG_NO_HZ_FULL_SYSIDLE_SMALL), rcu_sys_is_idle()
    > will break and you will have voided your warranty. ;-)

    So it breaks because of concurrent state machine stepping on each other toes, right?
    Like one CPU has reached RCU_SYSIDLE_SHORT and another comes and see only
    RCU_SYSIDLE_NONE, so it can for example overwite to SHORT while the other CPU
    can be already far further the cmpxchg() sequence?

    Aye, I need to think further on how to cope with that...

    >
    > Also, if tick_timekeeping_cpu() doesn't think that there is a timekeeping
    > CPU, rcu_sys_is_idle() will always return false. I think that this is
    > what you want to happen, just checking.

    Ah right but that should be fine. tick_timekeeping_cpu() works for all potential
    timekeepers. Basically it's !tick_nohz_full_cpu(cpu).

    >
    > > + /*
    > > + * Stop tick for 1 jiffy. In practice we stay periodic
    > > + * but that let us possibly delegate our timekeeping duty
    > > + * to stop the tick for real in the future.
    > > + */
    > > + ret = tick_period.tv64;
    > > + }
    >
    > Do we need to set tick_do_timer_cpu to cpu? Or is that handled elsewhere?
    > (If this is the boot-safety feature deleted below, could we please have
    > the comment back here?)

    This is done in the patch that calls ..kick_timekeeping() from sysidle_exit().

    Do you have another case in mind?


    \
     
     \ /
      Last update: 2013-12-18 16:01    [W:2.126 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site