From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752513AbdJSJJK (ORCPT ); Thu, 19 Oct 2017 05:09:10 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:56370 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752031AbdJSJJF (ORCPT ); Thu, 19 Oct 2017 05:09:05 -0400 X-Google-Smtp-Source: ABhQp+QM5M9pHWVAyVIpEjM39egIRDupzH+BPfKZlMCjj7EPvXlzO4JvEfQ6lIZF2qWiV6gEJXfwJA== Subject: Re: [PATCH 1/3] clocksource/mips-gic-timer: Fix rcu_sched timeouts from multithreading To: Matt Redfearn , Thomas Gleixner Cc: linux-mips@linux-mips.org, Matt Redfearn , "# v3 . 19 +" , linux-kernel@vger.kernel.org References: <1507730474-8577-1-git-send-email-matt.redfearn@mips.com> From: Daniel Lezcano Message-ID: <935fff7a-674a-a9c5-904f-6ed4ec6366e2@linaro.org> Date: Thu, 19 Oct 2017 11:09:02 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1507730474-8577-1-git-send-email-matt.redfearn@mips.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/10/2017 16:01, Matt Redfearn wrote: > When the MIPS GIC clockevent code was written, it appears to have > inherited the 0x300 cycle min delta from the MIPS CPU timer driver. This > is suboptimal for two reasons. > > Firstly, the CPU timer counts once every other cycle (i.e. half the > clock rate). The GIC counts once per clock. Assuming that the GIC and > CPU share the same clock this means the GIC is counting twice as fast, > and so the min delta should be (at least) doubled. Fix this by doubling > the min delta to 0x600. > > Secondly, the fixed min delta ignores the fact that with MIPS > multithreading active, execution resource within a core is shared > between the hardware threads within that core. An inconvenienly timed > switch of executing thread within gic_next_event, between the read and > write of updated count, can result in the CPU writing an event in the > past, and subsequently not receiving a tick interrupt until the counter > wraps. This stalls the CPU from the RCU scheduler. Other CPUs detect > this and print rcu_sched timeout messages in the kernel log. It can > lead to other issues as well if the CPU is holding locks or other > resources at the point at which it stalls. Fix this by scaling the min > delta for the timer based on the number of threads in the core > (smp_num_siblings). This accounts for the greater average runtime of > CPUs within a multithreading core. > > Signed-off-by: Matt Redfearn > Fixes: b695d8e6ad6f ("clocksource: mips-gic: Use clockevents_config_and_register") > Cc: # v3.19 + > > Signed-off-by: Matt Redfearn > --- Matt, I'm dropping your series. The first patch fails to compile because of the smp_num_siblings variable undefined when compile testing on another arch (probably a header is missing). The issue the patch address could be fixed in the time framework as stated by Thomas. As spotted by Thomas also, the local_irq_save() is not needed in the set_next_event() function, so the second patch is pointless and a patch removing those local_irq_save()/restore() would make more sense. The third patch does not longer apply after removing the two above. -- Daniel -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog