Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757014AbbHZW5j (ORCPT ); Wed, 26 Aug 2015 18:57:39 -0400 Received: from g4t3427.houston.hp.com ([15.201.208.55]:45952 "EHLO g4t3427.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753177AbbHZW5h (ORCPT ); Wed, 26 Aug 2015 18:57:37 -0400 Message-ID: <1440629853.32300.9.camel@j-VirtualBox> Subject: Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention From: Jason Low To: Frederic Weisbecker Cc: Linus Torvalds , Peter Zijlstra , Ingo Molnar , Thomas Gleixner , Oleg Nesterov , "Paul E. McKenney" , Linux Kernel Mailing List , Davidlohr Bueso , Steven Rostedt , Andrew Morton , Terry Rudd , Rik van Riel , Scott J Norton , jason.low2@hp.com Date: Wed, 26 Aug 2015 15:57:33 -0700 In-Reply-To: <20150826223152.GC11992@lerouge> References: <1440559068-29680-1-git-send-email-jason.low2@hp.com> <1440559068-29680-4-git-send-email-jason.low2@hp.com> <20150826223152.GC11992@lerouge> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2135 Lines: 43 On Thu, 2015-08-27 at 00:31 +0200, Frederic Weisbecker wrote: > On Wed, Aug 26, 2015 at 10:53:35AM -0700, Linus Torvalds wrote: > > On Tue, Aug 25, 2015 at 8:17 PM, Jason Low wrote: > > > > > > This patch addresses this by having the thread_group_cputimer structure > > > maintain a boolean to signify when a thread in the group is already > > > checking for process wide timers, and adds extra logic in the fastpath > > > to check the boolean. > > > > It is not at all obvious why the unlocked read of that variable is > > safe, and why there is no race with another thread just about to end > > its check_process_timers(). > > The risk is when a next timer is going to expire soon after we relaxed > the "checking" variable due to a recent expiration. The thread which > expires the next timer may still see a stale value on the "checking" > state and therefore delay the timer firing until the new value is seen. > So the worst that can happen is that the timer firing gets delayed for > X jiffies (I guess in practice it's only 1 jiffy). > > That said, posix cpu timers already suffer such race because > sig->cputimer.running itself is checked outside the sighand lock anyway. Right, in the worst case, the next thread that comes along will handle it. The current code already expects that level of granularity, so this shouldn't change anything much. For example, there is almost always a delay between a timer expiring and when a thread actually calls into check_process_timers(). > > I can well imagine that this is all perfectly safe and fine, but I'd > > really like to see that comment about _why_ that's the case, and why a > > completely unlocked access without even memory barriers is fine. > > Agreed, there should be a comment about that in the code (that is already full > of undocumented subtleties). Okay, I will add more comments about this. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/