2013-05-03 16:48:10

by Frederic Weisbecker

[permalink] [raw]
Subject: [GIT PULL] nohz: Bunch of fixes

Ingo,

Please pull the timers/nohz-hz1 branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
timers/nohz-hz1

HEAD: a7f4d69bb1d5e1f573b680355a2ad51566338887

It fixes a Kconfig dependency issue and forces a minimum of 1 tick every seconds
to keep handling the scheduler_tick() duties, even at a very low granularity. This
is a workaround until we can handle all these duties through on-demand driven
solutions rather than using periodic events, as per your suggestion.

I just noted two things:

* update_cpu_load_active() seem to rely on the fixed periodic tick at the
HZ rate. There is certainly something to tweak there to make it really correct
with dynamick ticks. I have the feeling that modifying pending_updates won't work
as it seems to decay assuming the time to catch up was idle.

* I'll probably have to disable CONFIG_CFS_BANDWIDTH when full dynticks
is enabled. First of all it modifies rq->nr_running without using inc/dec_nr_running
standard API, which is required for full dynticks. And second, I need to triple
check it's safe to use with 1 tick per second.

Ah and please note the merge commit (c032862fba51a3ca504752d3a25186b324c5ce83)
that was needed to get latest RCU and sched updates for the Kconfig fix.

Thanks,
Frederic
---

Frederic Weisbecker (2):
rcu: Fix full dynticks' dependency on wide RCU nocb mode
sched: Keep at least 1 tick per second for active dynticks tasks


include/linux/sched.h | 1 +
init/Kconfig | 4 ++--
kernel/sched/core.c | 30 ++++++++++++++++++++++++++++++
kernel/sched/idle_task.c | 1 +
kernel/sched/sched.h | 10 ++++++++++
kernel/time/Kconfig | 1 -
kernel/time/tick-sched.c | 7 +++++++
7 files changed, 51 insertions(+), 3 deletions(-)


2013-05-03 16:48:12

by Frederic Weisbecker

[permalink] [raw]
Subject: [PATCH 2/2] sched: Keep at least 1 tick per second for active dynticks tasks

The scheduler doesn't yet fully support environments
with a single task running without a periodic tick.

In order to ensure we still maintain the duties of scheduler_tick(),
keep at least 1 tick per second.

This makes sure that we keep the progression of various scheduler
accounting and background maintainance even with a very low granularity.
Examples include cpu load, sched average, CFS entity vruntime,
avenrun and events such as load balancing, amongst other details
handled in sched_class::task_tick().

This limitation will be removed in the future once we get
these individual items to work in full dynticks CPUs.

Suggested-by: Ingo Molnar <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Hakan Akkan <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Li Zhong <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Paul Gortmaker <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
include/linux/sched.h | 1 +
kernel/sched/core.c | 30 ++++++++++++++++++++++++++++++
kernel/sched/idle_task.c | 1 +
kernel/sched/sched.h | 10 ++++++++++
kernel/time/tick-sched.c | 7 +++++++
5 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index ebf7095..af008d7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1862,6 +1862,7 @@ static inline void wake_up_nohz_cpu(int cpu) { }

#ifdef CONFIG_NO_HZ_FULL
extern bool sched_can_stop_tick(void);
+extern u64 scheduler_tick_max_deferment(void);
#else
static inline bool sched_can_stop_tick(void) { return false; }
#endif
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e94842d..3bdf986 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2736,8 +2736,35 @@ void scheduler_tick(void)
rq->idle_balance = idle_cpu(cpu);
trigger_load_balance(rq, cpu);
#endif
+ rq_last_tick_reset(rq);
}

+#ifdef CONFIG_NO_HZ_FULL
+/**
+ * scheduler_tick_max_deferment
+ *
+ * Keep at least one tick per second when a single
+ * active task is running because the scheduler doesn't
+ * yet completely support full dynticks environment.
+ *
+ * This makes sure that uptime, CFS vruntime, load
+ * balancing, etc... continue to move forward, even
+ * with a very low granularity.
+ */
+u64 scheduler_tick_max_deferment(void)
+{
+ struct rq *rq = this_rq();
+ unsigned long next, now = ACCESS_ONCE(jiffies);
+
+ next = rq->last_sched_tick + HZ;
+
+ if (time_before_eq(next, now))
+ return 0;
+
+ return jiffies_to_usecs(next - now) * NSEC_PER_USEC;
+}
+#endif
+
notrace unsigned long get_parent_ip(unsigned long addr)
{
if (in_lock_functions(addr)) {
@@ -6993,6 +7020,9 @@ void __init sched_init(void)
#ifdef CONFIG_NO_HZ_COMMON
rq->nohz_flags = 0;
#endif
+#ifdef CONFIG_NO_HZ_FULL
+ rq->last_sched_tick = 0;
+#endif
#endif
init_rq_hrtick(rq);
atomic_set(&rq->nr_iowait, 0);
diff --git a/kernel/sched/idle_task.c b/kernel/sched/idle_task.c
index b8ce773..d8da010 100644
--- a/kernel/sched/idle_task.c
+++ b/kernel/sched/idle_task.c
@@ -17,6 +17,7 @@ select_task_rq_idle(struct task_struct *p, int sd_flag, int flags)
static void pre_schedule_idle(struct rq *rq, struct task_struct *prev)
{
idle_exit_fair(rq);
+ rq_last_tick_reset(rq);
}

static void post_schedule_idle(struct rq *rq)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 24dc298..ce39224 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -410,6 +410,9 @@ struct rq {
u64 nohz_stamp;
unsigned long nohz_flags;
#endif
+#ifdef CONFIG_NO_HZ_FULL
+ unsigned long last_sched_tick;
+#endif
int skip_clock_update;

/* capture load from *all* tasks on this cpu: */
@@ -1090,6 +1093,13 @@ static inline void dec_nr_running(struct rq *rq)
rq->nr_running--;
}

+static inline void rq_last_tick_reset(struct rq *rq)
+{
+#ifdef CONFIG_NO_HZ_FULL
+ rq->last_sched_tick = jiffies;
+#endif
+}
+
extern void update_rq_clock(struct rq *rq);

extern void activate_task(struct rq *rq, struct task_struct *p, int flags);
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 1c9f53b..07929c6 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -600,6 +600,13 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
time_delta = KTIME_MAX;
}

+#ifdef CONFIG_NO_HZ_FULL
+ if (!ts->inidle) {
+ time_delta = min(time_delta,
+ scheduler_tick_max_deferment());
+ }
+#endif
+
/*
* calculate the expiry time for the next timer wheel
* timer. delta_jiffies >= NEXT_TIMER_MAX_DELTA signals
--
1.7.5.4

2013-05-03 16:48:42

by Frederic Weisbecker

[permalink] [raw]
Subject: [PATCH 1/2] rcu: Fix full dynticks' dependency on wide RCU nocb mode

Commit 0637e029392386e6996f5d6574aadccee8315efa
("nohz: Select wide RCU nocb for full dynticks") intended
to force CONFIG_RCU_NOCB_CPU_ALL=y when full dynticks is
enabled.

However this option is part of a choice menu and Kconfig's
"select" instruction has no effect on such targets.

Fix this by using reverse dependencies on the targets we
don't want instead.

Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Hakan Akkan <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Li Zhong <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Paul Gortmaker <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
init/Kconfig | 4 ++--
kernel/time/Kconfig | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 66f67af..b3fdf9a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -687,7 +687,7 @@ choice

config RCU_NOCB_CPU_NONE
bool "No build_forced no-CBs CPUs"
- depends on RCU_NOCB_CPU
+ depends on RCU_NOCB_CPU && !NO_HZ_FULL
help
This option does not force any of the CPUs to be no-CBs CPUs.
Only CPUs designated by the rcu_nocbs= boot parameter will be
@@ -695,7 +695,7 @@ config RCU_NOCB_CPU_NONE

config RCU_NOCB_CPU_ZERO
bool "CPU 0 is a build_forced no-CBs CPU"
- depends on RCU_NOCB_CPU
+ depends on RCU_NOCB_CPU && !NO_HZ_FULL
help
This option forces CPU 0 to be a no-CBs CPU. Additional CPUs
may be designated as no-CBs CPUs using the rcu_nocbs= boot
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index a2ddd65..e4c07b0 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -109,7 +109,6 @@ config NO_HZ_FULL
select NO_HZ_COMMON
select RCU_USER_QS
select RCU_NOCB_CPU
- select RCU_NOCB_CPU_ALL
select VIRT_CPU_ACCOUNTING_GEN
select CONTEXT_TRACKING_FORCE
select IRQ_WORK
--
1.7.5.4

2013-05-03 16:58:51

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 1/2] rcu: Fix full dynticks' dependency on wide RCU nocb mode

On Fri, May 03, 2013 at 06:47:55PM +0200, Frederic Weisbecker wrote:
> Commit 0637e029392386e6996f5d6574aadccee8315efa
> ("nohz: Select wide RCU nocb for full dynticks") intended
> to force CONFIG_RCU_NOCB_CPU_ALL=y when full dynticks is
> enabled.
>
> However this option is part of a choice menu and Kconfig's
> "select" instruction has no effect on such targets.
>
> Fix this by using reverse dependencies on the targets we
> don't want instead.
>
> Signed-off-by: Frederic Weisbecker <[email protected]>
> Cc: Christoph Lameter <[email protected]>
> Cc: Hakan Akkan <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Li Zhong <[email protected]>
> Cc: Paul E. McKenney <[email protected]>
> Cc: Paul Gortmaker <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Thomas Gleixner <[email protected]>

Reviewed-by: Paul E. McKenney <[email protected]>

> ---
> init/Kconfig | 4 ++--
> kernel/time/Kconfig | 1 -
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 66f67af..b3fdf9a 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -687,7 +687,7 @@ choice
>
> config RCU_NOCB_CPU_NONE
> bool "No build_forced no-CBs CPUs"
> - depends on RCU_NOCB_CPU
> + depends on RCU_NOCB_CPU && !NO_HZ_FULL
> help
> This option does not force any of the CPUs to be no-CBs CPUs.
> Only CPUs designated by the rcu_nocbs= boot parameter will be
> @@ -695,7 +695,7 @@ config RCU_NOCB_CPU_NONE
>
> config RCU_NOCB_CPU_ZERO
> bool "CPU 0 is a build_forced no-CBs CPU"
> - depends on RCU_NOCB_CPU
> + depends on RCU_NOCB_CPU && !NO_HZ_FULL
> help
> This option forces CPU 0 to be a no-CBs CPU. Additional CPUs
> may be designated as no-CBs CPUs using the rcu_nocbs= boot
> diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
> index a2ddd65..e4c07b0 100644
> --- a/kernel/time/Kconfig
> +++ b/kernel/time/Kconfig
> @@ -109,7 +109,6 @@ config NO_HZ_FULL
> select NO_HZ_COMMON
> select RCU_USER_QS
> select RCU_NOCB_CPU
> - select RCU_NOCB_CPU_ALL
> select VIRT_CPU_ACCOUNTING_GEN
> select CONTEXT_TRACKING_FORCE
> select IRQ_WORK
> --
> 1.7.5.4
>

2013-05-04 06:37:41

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 1/2] rcu: Fix full dynticks' dependency on wide RCU nocb mode

2013/5/3 Paul E. McKenney <[email protected]>:
> On Fri, May 03, 2013 at 06:47:55PM +0200, Frederic Weisbecker wrote:
>> Commit 0637e029392386e6996f5d6574aadccee8315efa
>> ("nohz: Select wide RCU nocb for full dynticks") intended
>> to force CONFIG_RCU_NOCB_CPU_ALL=y when full dynticks is
>> enabled.
>>
>> However this option is part of a choice menu and Kconfig's
>> "select" instruction has no effect on such targets.
>>
>> Fix this by using reverse dependencies on the targets we
>> don't want instead.
>>
>> Signed-off-by: Frederic Weisbecker <[email protected]>
>> Cc: Christoph Lameter <[email protected]>
>> Cc: Hakan Akkan <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Kevin Hilman <[email protected]>
>> Cc: Li Zhong <[email protected]>
>> Cc: Paul E. McKenney <[email protected]>
>> Cc: Paul Gortmaker <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Steven Rostedt <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>
> Reviewed-by: Paul E. McKenney <[email protected]>

Nice!

Ingo, if you haven't yet pulled, please rather pick the
"timers/nohz-hz1-v2" branch that has the Reviewed-by: tag from Paul.

Thanks.

2013-05-04 06:41:56

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL] nohz: Bunch of fixes


* Frederic Weisbecker <[email protected]> wrote:

> Ingo,
>
> Please pull the timers/nohz-hz1 branch that can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> timers/nohz-hz1
>
> HEAD: a7f4d69bb1d5e1f573b680355a2ad51566338887
>
> It fixes a Kconfig dependency issue and forces a minimum of 1 tick every seconds
> to keep handling the scheduler_tick() duties, even at a very low granularity. This
> is a workaround until we can handle all these duties through on-demand driven
> solutions rather than using periodic events, as per your suggestion.
>
> I just noted two things:
>
> * update_cpu_load_active() seem to rely on the fixed periodic tick at the
> HZ rate. There is certainly something to tweak there to make it really correct
> with dynamick ticks. I have the feeling that modifying pending_updates won't work
> as it seems to decay assuming the time to catch up was idle.
>
> * I'll probably have to disable CONFIG_CFS_BANDWIDTH when full dynticks
> is enabled. First of all it modifies rq->nr_running without using inc/dec_nr_running
> standard API, which is required for full dynticks. And second, I need to triple
> check it's safe to use with 1 tick per second.
>
> Ah and please note the merge commit (c032862fba51a3ca504752d3a25186b324c5ce83)
> that was needed to get latest RCU and sched updates for the Kconfig fix.
>
> Thanks,
> Frederic
> ---
>
> Frederic Weisbecker (2):
> rcu: Fix full dynticks' dependency on wide RCU nocb mode
> sched: Keep at least 1 tick per second for active dynticks tasks
>
>
> include/linux/sched.h | 1 +
> init/Kconfig | 4 ++--
> kernel/sched/core.c | 30 ++++++++++++++++++++++++++++++
> kernel/sched/idle_task.c | 1 +
> kernel/sched/sched.h | 10 ++++++++++
> kernel/time/Kconfig | 1 -
> kernel/time/tick-sched.c | 7 +++++++
> 7 files changed, 51 insertions(+), 3 deletions(-)

Pulled, thanks Frederic!

Ingo