From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Horman Date: Thu, 29 Aug 2013 01:53:28 +0000 Subject: Re: [PATCH] clocksource: em_sti: Adjust clock event rating to fix SMP broadcast Message-Id: <20130829015328.GA5199@verge.net.au> List-Id: References: <1375251940-7809-1-git-send-email-horms+renesas@verge.net.au> <51F94A35.2020907@codeaurora.org> <51F9777B.2080500@codeaurora.org> <20130805014541.GD28465@verge.net.au> In-Reply-To: <20130805014541.GD28465@verge.net.au> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-arm-kernel@lists.infradead.org On Mon, Aug 05, 2013 at 10:45:42AM +0900, Simon Horman wrote: > On Wed, Jul 31, 2013 at 01:45:47PM -0700, Stephen Boyd wrote: > > On 07/31/13 12:17, Magnus Damm wrote: > > > Hi Stephen, > > > > > > On Thu, Aug 1, 2013 at 2:32 AM, Stephen Boyd wrote: > > >> On 07/30/13 23:25, Simon Horman wrote: > > >>> From: Magnus Damm > > >>> > > >>> Update the STI rating from 200 to 80 to fix SMP operation with > > >>> the ARM broadcast timer. This breakage was introduced by: > > >>> > > >>> f7db706 ARM: 7674/1: smp: Avoid dummy clockevent being preferred over real hardware clock-event > > >>> > > >>> Without this fix SMP operation is broken on EMEV2 since no > > >>> broadcast timer interrupts trigger on the secondary CPU cores. > > >>> > > >>> Signed-off-by: Magnus Damm > > >>> Signed-off-by: Simon Horman > > >>> --- > > >> This looks suspicious. Are you're purposefully deflating the rating so > > >> that the STI timer fills in the broadcast position? Why not make the STI > > >> cpumask be all possible CPUs? Presumably the interrupt can target all > > >> CPUs since it isn't a per-cpu interrupt and doing this would cause the > > >> STI to fill in the broadcast slot, leaving the per-cpu dummys in the > > >> tick position. > > > While letting the timer broadcast to all CPUs sounds interesting the > > > STI driver has so far only been used to drive a single CPU core. This > > > used to work well for us but has since some time unfortunately been > > > broken. I agree that it may be suboptimal with a single timer like STI > > > and using IPI for broadcast, but for more efficient SMP we already > > > have TWD or arch timer. > > > > I think there is some confusion. The mask field says what CPUs the timer > > can possibly interrupt and for non-percpu interrupts this should be all > > possible CPUs (unless we're talking clusters, etc. but I don't think we > > are). Can you please give the output of /proc/timer_list or confirm that > > the STI is your broadcast source? If so you should probably be marking > > the cpumask for all possible CPUs so that the clockevent core knows to > > prefer this clockevent for the broadcast source and not a per-cpu > > source. Then you can leave the rating as is. > > >From my point of view this seems to be a case of fixing a regression > by adjusting the code so it has its previous working behaviour. > I think that any work on altering the behaviour of the code > through masks should be considered as future work. Hi Magnus, I think this problem still needs a resolution.