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 BCD5C6071C
Subject: Re: [PATCH] timers: Clear must_forward_clk inside base lock
To:     john.stultz@linaro.org, tglx@linutronix.de, sboyd@kernel.org
Cc:     linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
References: <1532594522-28045-1-git-send-email-gkohli@codeaurora.org>
From:   "Kohli, Gaurav" <gkohli@codeaurora.org>
Message-ID: <063840fb-3360-4208-2950-908ef76791c2@codeaurora.org>
Date:   Tue, 31 Jul 2018 09:59:28 +0530
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <1532594522-28045-1-git-send-email-gkohli@codeaurora.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi John, Thomas,

Can you please review below patch and update your comments:

Regards
Gaurav

On 7/26/2018 2:12 PM, Gaurav Kohli wrote:
> While migrating timer to new base, there is a need
> to update base clk by calling forward_timer_base to
> avoid stale clock , but at the same time if run_timer
> is exectuing in new core it may set must_forward_clk
> to false and due to this forward base logic may fail as
> per below check:
> 
> if (likely(!base->must_forward_clk))
>                  return;
> 
> So preventing the same by putting must_forward_clk inside
> base lock.
> 
> Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>
> 
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index cc2d23e..675241d 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -1657,6 +1657,19 @@ static inline void __run_timers(struct timer_base *base)
>   
>   	raw_spin_lock_irq(&base->lock);
>   
> +	/*
> +	 * must_forward_clk must be cleared before running timers so that any
> +	 * timer functions that call mod_timer will not try to forward the
> +	 * base. idle trcking / clock forwarding logic is only used with
> +	 * BASE_STD timers.
> +	 *
> +	 * The deferrable base does not do idle tracking at all, so we do
> +	 * not forward it. This can result in very large variations in
> +	 * granularity for deferrable timers, but they can be deferred for
> +	 * long periods due to idle.
> +	 */
> +	base->must_forward_clk = false;
> +
>   	while (time_after_eq(jiffies, base->clk)) {
>   
>   		levels = collect_expired_timers(base, heads);
> @@ -1676,19 +1689,6 @@ static __latent_entropy void run_timer_softirq(struct softirq_action *h)
>   {
>   	struct timer_base *base = this_cpu_ptr(&timer_bases[BASE_STD]);
>   
> -	/*
> -	 * must_forward_clk must be cleared before running timers so that any
> -	 * timer functions that call mod_timer will not try to forward the
> -	 * base. idle trcking / clock forwarding logic is only used with
> -	 * BASE_STD timers.
> -	 *
> -	 * The deferrable base does not do idle tracking at all, so we do
> -	 * not forward it. This can result in very large variations in
> -	 * granularity for deferrable timers, but they can be deferred for
> -	 * long periods due to idle.
> -	 */
> -	base->must_forward_clk = false;
> -
>   	__run_timers(base);
>   	if (IS_ENABLED(CONFIG_NO_HZ_COMMON))
>   		__run_timers(this_cpu_ptr(&timer_bases[BASE_DEF]));
> 

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