2020-05-02 18:31:48

by Prasad Sodagudi

[permalink] [raw]
Subject: [PATCH v3 2/2] sched: Add a check for cpu unbound deferrable timers

Add a check to find expired unbound deferrable timers
and trigger softirq for handling timers. This way a CPU
can process all the expired deferrable timers whenever
it is out off idle state due to an interrupt.

Signed-off-by: Prasad Sodagudi <[email protected]>
---
include/linux/timer.h | 3 +++
kernel/time/tick-sched.c | 8 +++++++-
kernel/time/timer.c | 29 ++++++++++++++++++++++++++++-
3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/include/linux/timer.h b/include/linux/timer.h
index 0dc19a8..e85dd2d 100644
--- a/include/linux/timer.h
+++ b/include/linux/timer.h
@@ -172,6 +172,9 @@ extern int del_timer(struct timer_list * timer);
extern int mod_timer(struct timer_list *timer, unsigned long expires);
extern int mod_timer_pending(struct timer_list *timer, unsigned long expires);
extern int timer_reduce(struct timer_list *timer, unsigned long expires);
+#ifdef CONFIG_SMP
+extern bool check_pending_deferrable_timers(int cpu);
+#endif

/*
* The jiffies value which is added to now, when there is no timer
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 3e2dc9b..16aec80 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -23,6 +23,7 @@
#include <linux/module.h>
#include <linux/irq_work.h>
#include <linux/posix-timers.h>
+#include <linux/timer.h>
#include <linux/context_tracking.h>
#include <linux/mm.h>

@@ -1274,8 +1275,13 @@ static inline void tick_nohz_irq_enter(void)
now = ktime_get();
if (ts->idle_active)
tick_nohz_stop_idle(ts, now);
- if (ts->tick_stopped)
+ if (ts->tick_stopped) {
tick_nohz_update_jiffies(now);
+#ifdef CONFIG_SMP
+ if (check_pending_deferrable_timers(smp_processor_id()))
+ raise_softirq_irqoff(TIMER_SOFTIRQ);
+#endif
+ }
}

#else
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 1bf9b49..5947c63 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -221,6 +221,7 @@ static DECLARE_WORK(timer_update_work, timer_update_keys);

#ifdef CONFIG_SMP
struct timer_base timer_base_deferrable;
+static atomic_t deferrable_pending;
unsigned int sysctl_timer_migration = 1;

DEFINE_STATIC_KEY_FALSE(timers_migration_enabled);
@@ -1610,6 +1611,31 @@ static u64 cmp_next_hrtimer_event(u64 basem, u64 expires)
return DIV_ROUND_UP_ULL(nextevt, TICK_NSEC) * TICK_NSEC;
}

+
+#ifdef CONFIG_SMP
+/*
+ * check_pending_deferrable_timers - Check for unbound deferrable timer expiry
+ * @cpu - Current CPU
+ *
+ * The function checks whether any global deferrable pending timers
+ * are exipired or not. This function does not check cpu bounded
+ * diferrable pending timers expiry.
+ *
+ * The function returns true when a cpu unbounded deferrable timer is expired.
+ */
+bool check_pending_deferrable_timers(int cpu)
+{
+ if (cpu == tick_do_timer_cpu ||
+ tick_do_timer_cpu == TICK_DO_TIMER_NONE) {
+ if (time_after_eq(jiffies, timer_base_deferrable.clk)
+ && !atomic_cmpxchg(&deferrable_pending, 0, 1)) {
+ return true;
+ }
+ }
+ return false;
+}
+#endif
+
/**
* get_next_timer_interrupt - return the time (clock mono) of the next timer
* @basej: base time jiffies
@@ -1801,7 +1827,8 @@ static __latent_entropy void run_timer_softirq(struct softirq_action *h)
if (IS_ENABLED(CONFIG_NO_HZ_COMMON)) {
__run_timers(this_cpu_ptr(&timer_bases[BASE_DEF]));
#ifdef CONFIG_SMP
- if (tick_do_timer_cpu == TICK_DO_TIMER_NONE ||
+ if ((atomic_cmpxchg(&deferrable_pending, 1, 0) &&
+ tick_do_timer_cpu == TICK_DO_TIMER_NONE) ||
tick_do_timer_cpu == smp_processor_id())
__run_timers(&timer_base_deferrable);
#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2020-05-04 19:17:05

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] sched: Add a check for cpu unbound deferrable timers

Hi Prasad,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/timers/core]
[also build test ERROR on tip/auto-latest tip/timers/nohz v5.7-rc4 next-20200501]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Prasad-Sodagudi/timer-make-deferrable-cpu-unbound-timers-really-not-bound-to-a-cpu/20200503-025049
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 4479730e9263befbb9ce9563a09563db2acb8f7c
config: riscv-nommu_virt_defconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=riscv

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

kernel/time/timer.c: In function 'get_timer_cpu_base':
kernel/time/timer.c:848:11: error: 'timer_base_deferrable' undeclared (first use in this function)
848 | base = &timer_base_deferrable;
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/timer.c:848:11: note: each undeclared identifier is reported only once for each function it appears in
kernel/time/timer.c: In function 'get_timer_this_cpu_base':
kernel/time/timer.c:867:11: error: 'timer_base_deferrable' undeclared (first use in this function)
867 | base = &timer_base_deferrable;
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/timer.c: In function 'run_timer_softirq':
>> kernel/time/timer.c:1829:24: error: 'deferrable_pending' undeclared (first use in this function)
1829 | if ((atomic_cmpxchg(&deferrable_pending, 1, 0) &&
| ^~~~~~~~~~~~~~~~~~
kernel/time/timer.c:1832:18: error: 'timer_base_deferrable' undeclared (first use in this function)
1832 | __run_timers(&timer_base_deferrable);
| ^~~~~~~~~~~~~~~~~~~~~

vim +/deferrable_pending +1829 kernel/time/timer.c

1817
1818 /*
1819 * This function runs timers and the timer-tq in bottom half context.
1820 */
1821 static __latent_entropy void run_timer_softirq(struct softirq_action *h)
1822 {
1823 struct timer_base *base = this_cpu_ptr(&timer_bases[BASE_STD]);
1824
1825 __run_timers(base);
1826 if (IS_ENABLED(CONFIG_NO_HZ_COMMON)) {
1827 __run_timers(this_cpu_ptr(&timer_bases[BASE_DEF]));
1828 #ifdef CONFIG_SMP
> 1829 if ((atomic_cmpxchg(&deferrable_pending, 1, 0) &&
1830 tick_do_timer_cpu == TICK_DO_TIMER_NONE) ||
1831 tick_do_timer_cpu == smp_processor_id())
1832 __run_timers(&timer_base_deferrable);
1833 #endif
1834 }
1835 }
1836

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (3.17 kB)
.config.gz (6.63 kB)
Download all attachments

2020-05-06 14:06:53

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] sched: Add a check for cpu unbound deferrable timers

Prasad Sodagudi <[email protected]> writes:

> Subject: sched: Add...

How is this related to the scheduler?

> Add a check to find expired unbound deferrable timers
> and trigger softirq for handling timers. This way a CPU
> can process all the expired deferrable timers whenever
> it is out off idle state due to an interrupt.

A little bit more context would be useful.

> Signed-off-by: Prasad Sodagudi <[email protected]>
> ---
> include/linux/timer.h | 3 +++
> kernel/time/tick-sched.c | 8 +++++++-
> kernel/time/timer.c | 29 ++++++++++++++++++++++++++++-
> 3 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/timer.h b/include/linux/timer.h
> index 0dc19a8..e85dd2d 100644
> --- a/include/linux/timer.h
> +++ b/include/linux/timer.h
> @@ -172,6 +172,9 @@ extern int del_timer(struct timer_list * timer);
> extern int mod_timer(struct timer_list *timer, unsigned long expires);
> extern int mod_timer_pending(struct timer_list *timer, unsigned long expires);
> extern int timer_reduce(struct timer_list *timer, unsigned long expires);
> +#ifdef CONFIG_SMP

This #ifdef is useless because ...

> +extern bool check_pending_deferrable_timers(int cpu);
> +#endif
>
> /*
> * The jiffies value which is added to now, when there is no timer
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 3e2dc9b..16aec80 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -23,6 +23,7 @@
> #include <linux/module.h>
> #include <linux/irq_work.h>
> #include <linux/posix-timers.h>
> +#include <linux/timer.h>
> #include <linux/context_tracking.h>
> #include <linux/mm.h>
>
> @@ -1274,8 +1275,13 @@ static inline void tick_nohz_irq_enter(void)
> now = ktime_get();
> if (ts->idle_active)
> tick_nohz_stop_idle(ts, now);
> - if (ts->tick_stopped)
> + if (ts->tick_stopped) {
> tick_nohz_update_jiffies(now);
> +#ifdef CONFIG_SMP
> + if (check_pending_deferrable_timers(smp_processor_id()))
> + raise_softirq_irqoff(TIMER_SOFTIRQ);
> +#endif

... you failed to provide a stub function which avoids this #ifdef

> + }
> }
>
> #else
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 1bf9b49..5947c63 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -221,6 +221,7 @@ static DECLARE_WORK(timer_update_work, timer_update_keys);
>
> #ifdef CONFIG_SMP
> struct timer_base timer_base_deferrable;
> +static atomic_t deferrable_pending;
> unsigned int sysctl_timer_migration = 1;
>
> DEFINE_STATIC_KEY_FALSE(timers_migration_enabled);
> @@ -1610,6 +1611,31 @@ static u64 cmp_next_hrtimer_event(u64 basem, u64 expires)
> return DIV_ROUND_UP_ULL(nextevt, TICK_NSEC) * TICK_NSEC;
> }
>
> +
> +#ifdef CONFIG_SMP
> +/*
> + * check_pending_deferrable_timers - Check for unbound deferrable timer expiry
> + * @cpu - Current CPU

Bogus doc format.

> + *
> + * The function checks whether any global deferrable pending timers
> + * are exipired or not. This function does not check cpu bounded
> + * diferrable pending timers expiry.

Editors have spell check.

> + *
> + * The function returns true when a cpu unbounded deferrable timer is expired.
> + */
> +bool check_pending_deferrable_timers(int cpu)
> +{
> + if (cpu == tick_do_timer_cpu ||
> + tick_do_timer_cpu == TICK_DO_TIMER_NONE) {

The second line conditional wants to be aligned with the first line
conditional.

> + if (time_after_eq(jiffies, timer_base_deferrable.clk)
> + && !atomic_cmpxchg(&deferrable_pending, 0, 1)) {

Ditto. Aside of that this deferrable pending magic lacks any form of
explanation.

> + return true;
> + }
> + }
> + return false;
> +}
> +#endif
> +
> /**
> * get_next_timer_interrupt - return the time (clock mono) of the next timer
> * @basej: base time jiffies
> @@ -1801,7 +1827,8 @@ static __latent_entropy void run_timer_softirq(struct softirq_action *h)
> if (IS_ENABLED(CONFIG_NO_HZ_COMMON)) {
> __run_timers(this_cpu_ptr(&timer_bases[BASE_DEF]));
> #ifdef CONFIG_SMP
> - if (tick_do_timer_cpu == TICK_DO_TIMER_NONE ||
> + if ((atomic_cmpxchg(&deferrable_pending, 1, 0) &&

How is that supposed to compile with NOHZ=n?

For every version of these patches the 0-day robot is complaining about
exactly the same problem:

Your testing is solely done with one config, i.e. the config you are
interested in.

Is it really so hard to compile test for a total of 4 combinations of
SMP and NOHZ?

Thanks,

tglx