Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 6D99EC433D2
MIME-Version: 1.0
References: <1584326350-30275-1-git-send-email-psodagud@codeaurora.org> <1584326350-30275-3-git-send-email-psodagud@codeaurora.org>
In-Reply-To: <1584326350-30275-3-git-send-email-psodagud@codeaurora.org>
From:   Pavan Kondeti <pkondeti@codeaurora.org>
Date:   Mon, 16 Mar 2020 16:57:19 +0530
Message-ID: <CAEU1=PnmhRGmTHPikj5y0OVcp2NO81JgSBYyg4o=CPaNkuF-5Q@mail.gmail.com>
Subject: Re: [PATCH 2/2] sched: Add a check for cpu unbound deferrable timers
To:     Prasad Sodagudi <psodagud@codeaurora.org>
Cc:     John Stultz <john.stultz@linaro.org>,
        Thomas Gleixner <tglx@linutronix.de>, sboyd@kernel.org,
        LKML <linux-kernel@vger.kernel.org>, saravanak@google.com,
        tsoni@codeaurora.org, Tejun Heo <tj@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Prasad,

On Mon, Mar 16, 2020 at 8:42 AM Prasad Sodagudi <psodagud@codeaurora.org> wrote:
>
> Add a check for cpu unbound deferrable timer expiry and raise
> softirq for handling the expired timers so that the CPU can
> process the cpu unbound deferrable times as early as possible
> when a cpu tries to enter/exit idle loop.
>
> Signed-off-by: Prasad Sodagudi <psodagud@codeaurora.org>
> ---
>  include/linux/timer.h    |  3 +++
>  kernel/time/tick-sched.c |  6 ++++++
>  kernel/time/timer.c      | 29 ++++++++++++++++++++++++++++-
>  3 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/timer.h b/include/linux/timer.h
> index 1e6650e..25a1837 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 a792d21..fe0836a 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>
>
> @@ -948,6 +949,11 @@ static void __tick_nohz_idle_stop_tick(struct tick_sched *ts)
>         ktime_t expires;
>         int cpu = smp_processor_id();
>
> +#ifdef CONFIG_SMP
> +       if (check_pending_deferrable_timers(cpu))
> +               raise_softirq_irqoff(TIMER_SOFTIRQ);
> +#endif
> +
>         /*
>          * If tick_nohz_get_sleep_length() ran tick_nohz_next_event(), the
>          * tick timer expiration time is known already.

Since this is called outside interrupt context, ksoftirqd task is always woken
up to serve the TIMER softirq. Given that, this CPU exited idle due to an
interrupt, we could probably check this condition and raise the softirq prior
to irq_exit. To make the discussion easier, providing the diff. This will make
sure that TIMER softirq is served after serving the IRQ and avoids waking
ksoftirqd. This should also cover the case, Thoms mentioned in the other
email.

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index a792d21..d9d1915 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1268,8 +1268,19 @@ 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);
+               /*
+                * TODO: check the possibility of doing this only
+                * when jiffies is updated.
+                *
+                * __raise_softirq_irqoff() marks the softirq pending.
+                * The softirq is served after this IRQ is served from
+                * irq_exit()
+                */
+               if (check_pending_deferrable_timers())
+                       __raise_softirq_irqoff(TIMER_SOFTIRQ);
+       }
 }

 #else


-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project