From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752411AbcAFSLi (ORCPT ); Wed, 6 Jan 2016 13:11:38 -0500 Received: from www.linutronix.de ([62.245.132.108]:37834 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751708AbcAFSLg (ORCPT ); Wed, 6 Jan 2016 13:11:36 -0500 Date: Wed, 6 Jan 2016 19:10:42 +0100 (CET) From: Thomas Gleixner To: Prarit Bhargava cc: John Stultz , lkml , Ingo Molnar , Xunlei Pang , Peter Zijlstra , Baolin Wang , Arnd Bergmann Subject: Re: [PATCH 1/2] kernel, timekeeping, add trylock option to ktime_get_with_offset() In-Reply-To: <568D5797.8000904@redhat.com> Message-ID: References: <1452085234-10667-1-git-send-email-prarit@redhat.com> <1452085234-10667-2-git-send-email-prarit@redhat.com> <568D5797.8000904@redhat.com> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001,URIBL_BLOCKED=0.001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 6 Jan 2016, Prarit Bhargava wrote: > On 01/06/2016 12:33 PM, John Stultz wrote: > > On Wed, Jan 6, 2016 at 9:28 AM, John Stultz wrote: > >> On Wed, Jan 6, 2016 at 5:00 AM, Prarit Bhargava wrote: > >>> -ktime_t ktime_get_with_offset(enum tk_offsets offs) > >>> +ktime_t ktime_get_with_offset(enum tk_offsets offs, int trylock) > >>> { > >>> struct timekeeper *tk = &tk_core.timekeeper; > >>> unsigned int seq; > >>> ktime_t base, *offset = offsets[offs]; > >>> s64 nsecs; > >>> + unsigned long flags = 0; > >>> + > >>> + if (unlikely(!timekeeping_initialized)) > >>> + return ktime_set(0, 0); > >>> > >>> WARN_ON(timekeeping_suspended); > >>> > >>> + if (trylock && !raw_spin_trylock_irqsave(&timekeeper_lock, flags)) > >>> + return ktime_set(KTIME_MAX, 0); > >> > >> Wait.. this doesn't make sense. The timekeeper lock is only for reading. > > > > Only for writing.. sorry.. still drinking my coffee. > > > >> What I was suggesting to you off line is to have something that avoids > >> spinning on the seqcounter should if a bug occurs and we IPI all the > >> cpus, that we don't deadlock or block any printk messages. > > > > And more clearly here, if a cpu takes a write on the seqcounter in > > update_wall_time() and at that point another cpu hits a bug, and IPIs > > the cpus, the system would deadlock. That's really what I want to > > avoid. > > Right -- but the only time that the seq_lock is taken for writing is when the > timekeeper_lock is acquired (including update_wall_time()). This means that > > if (!raw_spin_trylock_irqsave(&timekeeper_lock, flags)) > > is equivalent to > > if (tk_core.seq & 1) // sequence_t is odd when writing > > The problem with the latter is that it is possible that there is no > protection from a writer setting tk_core.seq odd AFTER I've read it, > and the protection for that AFAICT comes from the timekeeper_lock. > > That means I need to check to see if the timekeeper_lock is locked. And > the patch does exactly that -- checks to see if the lock is available, and > if not avoids spinning on the seq_lock. And no, we don't want that in every code path. We already have the concept of the fast timekeeper, which is lockless and NMI safe. It's useable for tracing and perf, so it can be used for printk as well. It supports clock monotonic today, which is good enough for printk, but it could be extended to other clocks if really required. Thanks, tglx