From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Gleixner Subject: Re: [PATCH v1] timer: Forward timer base before migrating timers Date: Thu, 18 Jan 2018 11:45:42 +0100 (CET) Message-ID: References: <20180117124442.24904-1-clingutla@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Return-path: Received: from Galois.linutronix.de ([146.0.238.70]:48374 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755900AbeARKpt (ORCPT ); Thu, 18 Jan 2018 05:45:49 -0500 In-Reply-To: <20180117124442.24904-1-clingutla@codeaurora.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Lingutla Chandrasekhar Cc: linux-kernel@vger.kernel.org, neeraju@codeaurora.org, linux-arm-msm@vger.kernel.org On Wed, 17 Jan 2018, Lingutla Chandrasekhar wrote: > In case when timers are migrated to a CPU, after it exits > idle, but before timer base is forwarded, either from > run_timer_softirq()/mod_timer()/add_timer_on(), it's > possible that migrated timers are queued, based on older > clock value. This can cause delays in handling those timers. > > For example, consider below sequence of events: > > - CPU0 timer1 expires = 59969 and base->clk = 59131. So, > timer is queued at level 2, with next expiry for this timer > = 60032 (due to granularity addition). > - CPU1 enters idle @60007, with next timer expiry @60020. > - CPU1 exits idle. > - CPU0 is hotplugged at 60009, and timers are migrated to > CPU1, with new base->clk = 60007. timer1 is queued, > based on 60007 at level 0, for immediate handling (in > next timer softirq handling). > - CPU1's base->clk is forwarded to 60009, so, in next sched > timer interrupt, timer1 is not handled. > > The issue happens as timer wheel collects expired timers > starting from the current clk's index onwards, but migrated > timers, if enqueued, based on older clk value can result > in their index being less than clk's current index. > This can only happen if new base->clk is ahead of > timer->expires, resulting in timer being queued at > new base->clk's current index. > > Signed-off-by: Lingutla Chandrasekhar > Signed-off-by: Neeraj Upadhyay That's still wrong. The SOB chain should be: SOB: Author SOB: Handler/Transporter SOB: Committer So if you wrote te patch then Neeraj's SOB is wrong. If Neeraj wrote the patch then the patch should contain a From: Neeraj line right at the top of the changelog. If you both developed the patch then please use; Co-developed-by: Neeraj Signed-off-by: Neeraj Signed-off-by; You That would make you the Author in the git commit, but still preserve the co-authorship. Please clarify what applies to this particular patch. Thanks, tglx