From: Thomas Gleixner <tglx@linutronix.de>
To: Prasad Sodagudi <psodagud@codeaurora.org>,
john.stultz@linaro.org, sboyd@kernel.org, tj@kernel.org
Cc: linux-kernel@vger.kernel.org, saravanak@google.com,
psodagud@codeaurora.org, pkondeti@codeaurora.org
Subject: Re: [PATCH v3 2/2] sched: Add a check for cpu unbound deferrable timers
Date: Wed, 06 May 2020 16:03:34 +0200 [thread overview]
Message-ID: <877dxpkvnt.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <1588444137-18651-3-git-send-email-psodagud@codeaurora.org>
Prasad Sodagudi <psodagud@codeaurora.org> 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 <psodagud@codeaurora.org>
> ---
> 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
prev parent reply other threads:[~2020-05-06 14:03 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-02 18:28 [PATCH v3 0/2] timer: make deferrable cpu unbound timers really not bound to a cpu Prasad Sodagudi
2020-05-02 18:28 ` [PATCH v3 1/2] " Prasad Sodagudi
2020-05-04 18:11 ` kbuild test robot
2020-05-05 0:08 ` kbuild test robot
2020-05-06 13:28 ` Thomas Gleixner
2020-05-13 19:53 ` psodagud
2020-05-13 20:28 ` Thomas Gleixner
2020-05-13 20:55 ` psodagud
2020-05-13 21:34 ` Thomas Gleixner
2020-05-02 18:28 ` [PATCH v3 2/2] sched: Add a check for cpu unbound deferrable timers Prasad Sodagudi
2020-05-04 19:11 ` kbuild test robot
2020-05-06 14:03 ` Thomas Gleixner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877dxpkvnt.fsf@nanos.tec.linutronix.de \
--to=tglx@linutronix.de \
--cc=john.stultz@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pkondeti@codeaurora.org \
--cc=psodagud@codeaurora.org \
--cc=saravanak@google.com \
--cc=sboyd@kernel.org \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).