Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756699AbcK2J3g (ORCPT ); Tue, 29 Nov 2016 04:29:36 -0500 Received: from mga09.intel.com ([134.134.136.24]:32648 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756342AbcK2J3a (ORCPT ); Tue, 29 Nov 2016 04:29:30 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,715,1473145200"; d="scan'208";a="36805368" Date: Tue, 29 Nov 2016 17:38:28 +0800 From: Chen Yu To: Ingo Molnar Cc: John Stultz , lkml , "Rafael J. Wysocki" , Xunlei Pang , Ingo Molnar , Len Brown , "H. Peter Anvin" , Pavel Machek , Thomas Gleixner , Prarit Bhargava , Richard Cochran Subject: Re: [PATCH 2/7] timekeeping: Ignore the bogus sleep time if pm_trace is enabled Message-ID: <20161129093828.GA6815@yu-desktop-1.sh.intel.com> References: <1480372524-15181-1-git-send-email-john.stultz@linaro.org> <1480372524-15181-3-git-send-email-john.stultz@linaro.org> <20161129071955.GB29412@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161129071955.GB29412@gmail.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2810 Lines: 83 On Tue, Nov 29, 2016 at 08:19:55AM +0100, Ingo Molnar wrote: > > * John Stultz wrote: > > > From: Chen Yu > > > > Previously we encountered some memory overflow issues due to > > the bogus sleep time brought by inconsistent rtc, which is > > triggered when pm_trace is enabled, and we have fixed it > > in recent kernel. However it's improper in the first place > > to call __timekeeping_inject_sleeptime() in case that pm_trace > > is enabled simply because that "hash" time value will wreckage > > the timekeeping subsystem. > > s/ > > Previously we encountered memory overflow issues due to > bogus sleep time brought by an inconsistent RTC, which is > triggered when pm_trace is enabled, and we have fixed it > in recent kernels. However it's improper in the first place > to call __timekeeping_inject_sleeptime() in case pm_trace > is enabled simply because the "hash" time value will wreckage > the timekeeping subsystem. > > Half a dozen typos ... > > > This patch is originally written by Thomas, which would bypass > > the bogus rtc interval when pm_trace is enabled. > > Meanwhile, if system succeed to resume back with pm_trace set, the > > users are warned to adjust the bogus rtc either by 'ntpdate' or > > 'rdate', by resetting pm_trace_rtc_abused to false, otherwise above > > tools might not work as expected. > > s/ > > This patch was originally written by Thomas, which would bypass > the bogus RTC interval when pm_trace is enabled. > Meanwhile, if the system succeeds to resume back with pm_trace set, > users are warned to adjust the bogus RTC either by 'ntpdate' or > 'rdate', by resetting pm_trace_rtc_abused to false, otherwise above > tools might not work as expected. > > > + /* > > + * If pm_trace abused the RTC as storage set the timespec to 0 > > + * which tells the caller that this RTC value is bogus. > > + */ > > s/ > /* > * If pm_trace abused the RTC as storage, set the timespec to 0, > * which tells the caller that this RTC value is bogus. > */ > > > @@ -74,6 +75,9 @@ > > > > #define DEVSEED (7919) > > > > +bool pm_trace_rtc_abused __read_mostly; > > +EXPORT_SYMBOL(pm_trace_rtc_abused); > > EXPORT_SYMBOL_GPL() > > > +static int pm_trace_notify(struct notifier_block *nb, > > + unsigned long mode, void *_unused) > > Please no nonsensical linebreaks in the middle of an argument list. > > > +{ > > + switch (mode) { > > + case PM_POST_HIBERNATION: > > + case PM_POST_SUSPEND: > > + if (pm_trace_rtc_abused) { > > + pm_trace_rtc_abused = false; > > + pr_warn("Possible incorrect RTC due to pm_trace, please use 'ntpdate' or 'rdate' to reset.\n"); > > > s/to reset./to reset it. > > Thanks, > > Ingo Thanks Ingo, I've sent out a new versin based on your comments. Yu