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 592446044B
Subject: Re: [PATCH v2] timers: Clear must_forward_clk inside base lock
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     john.stultz@linaro.org, sboyd@kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
References: <1533186903-28419-1-git-send-email-gkohli@codeaurora.org>
 <alpine.DEB.2.21.1808020826580.1835@nanos.tec.linutronix.de>
From:   "Kohli, Gaurav" <gkohli@codeaurora.org>
Message-ID: <fce3d68c-341d-e720-773f-0867e7770c27@codeaurora.org>
Date:   Thu, 2 Aug 2018 12:34:21 +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: <alpine.DEB.2.21.1808020826580.1835@nanos.tec.linutronix.de>
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


On 8/2/2018 12:04 PM, Thomas Gleixner wrote:
> On Thu, 2 Aug 2018, Gaurav Kohli wrote:
>> Timer wheel base->must_forward_clock is indicating that
>> the base clock might be stale due to a long idle sleep.
>> The forwarding of base clock takes place in softirq of timer
> 
> of the base clock takes place in the timer softirq ...
> 
>> or when a timer is enqueued to base which is idle. While migrate
> 
> to a base ..
> 
>> timer from remote CPU to the new base which is idle, then
> 
> See below.
> 
>> following race can happen:
>>
>>    CPU0                                  CPU1
>>    run_timer_softirq                     timers_dead_cpu
>>
>> 					base = lock_timer_base(timer);
>>    base->must_forward_clk = false
>> 					if (base->must_forward_clk)
>> 				         forward(base); >>skip
>>
>> 					migrate_timer_list
> 
> I don't know why you insist on migrate_timer_list() being part of the
> picture here.

Hi Thomas,

Thanks for comment.
I agree this can come with normal enqueue of timer as well, will make it 
more generic to avoid confuzion and upload new patch for same.

Regards
Gaurav


> 
> It's only _ONE_ particular way to observe that issue. But it's not the only
> way. ANY remote enqueue which hits the situation on the other CPU (CPU0 in
> the example) has this problem. Tying it to migrate_timer_list() just
> because you observed it that way is actively misleading. Surely you can add
> a sentence that you observed it in that case, but that's supplemental
> information.
> 
> Thanks,
> 
> 	tglx
> 

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