Re: [PATCH][RFC v4] timekeeping: ignore the bogus sleep time if pm_trace is enabled

[Chen Yu]

On Sat, Aug 27, 2016 at 03:08:56PM +0800, Xunlei Pang wrote:
> On 2016/08/18 at 18:43, Chen Yu wrote:
> > 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, please refer to:
> > https://patchwork.kernel.org/patch/9286365/
> > 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.
> >
> > So this patch ignores the sleep time if pm_trace is enabled in
> > the following situation:
> > 1. rtc is used as persist clock to compensate for sleep time,
> >    (because system does not have a nonstop clocksource) or
> > 2. rtc is used to calculate the sleep time in rtc_resume.
> >
> > Cc: Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>
> > Cc: John Stultz <john.stultz@xxxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Xunlei Pang <xlpang@xxxxxxxxxx>
> > Cc: Zhang Rui <rui.zhang@xxxxxxxxx>
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Cc: linux-pm@xxxxxxxxxxxxxxx
> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > Reported-by: Janek Kozicki <cosurgi@xxxxxxxxx>
> > Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> > ---
> 
> I suddenly think of a way to avoid adding this ugly __weak auxiliary function.
> 
> Add a special treatment for read_persistent_clock() in arch/x86/kernel/rtc.c as follows,
> void read_persistent_clock(struct timespec *ts)
> {
>     x86_platform.get_wallclock(ts);
> 
>     /* Make rtc-based persistent clock unusable if pm_trace is enabled. */
>     if (pm_trace_is_enabled() &&
>         x86_platform.get_wallclock == mach_get_cmos_time) {
>         ts->tv_sec = 0;
>         ts->tv_nsec = 0;
>     }
> }
> 
> In this way, we can avoid the touch of timekeeping core, after all ptrace is currently x86-specific.
> 
> What do you think?
>
Good point! Will send another version based on this idea.

Thanks,
Yu