From: Thomas Gleixner <tglx@linutronix.de> To: Peter Zijlstra <peterz@infradead.org> Cc: Jan Stancek <jstancek@redhat.com>, linux-kernel@vger.kernel.org, ltp@lists.linux.it, viro@zeniv.linux.org.uk, kstewart@linuxfoundation.org, gregkh@linuxfoundation.org, rfontana@redhat.com Subject: Re: [PATCH] kernel: use ktime_get_real_ts64() to calculate acct.ac_btime Date: Thu, 7 Nov 2019 13:40:47 +0100 (CET) [thread overview] Message-ID: <alpine.DEB.2.21.1911071335320.4256@nanos.tec.linutronix.de> (raw) In-Reply-To: <20191107123224.GA6315@hirez.programming.kicks-ass.net> On Thu, 7 Nov 2019, Peter Zijlstra wrote: > Lets start by saying this accounting stuff is terrible crap and it > deserves to fail and burn. No argument about that. > And what does btime want? As implemented it jumps around if you ask the > question twice with an adjtime() call or suspend in between. Of course, > if we take an actual CLOCK_REALTIME timestamp at fork() the value > doesn't change, but then it can be in the future (DST,adjtime()), which > is exactly the reason why CLOCK_REALTIME is absolute shit for timestamps > (logging, accounting, etc.). > > And your 'fix' is pretty terible too. Arguably ktime_get_seconds() wants > fixing for not having the ns accumulation and actually differing from > tv_sec, but now you accrue one source of ns while still disregarding > another (also, I friggin hate timespec, it's a terrible interface for > time). > > All in all, I'm tempted to just declare this stuff broken and -EWONTFIX, > but if we have to do something, something like the below is at least > internally consistent. Kinda :) > + mono = ktime_get_ns(); > + real = ktime_get_real_ns(); > + /* > + * Compute btime by subtracting the elapsed time from the current > + * CLOCK_REALTIME. > + * > + * XXX totally buggered, because it changes results across > + * adjtime() calls and suspend/resume. > + */ > + delta = mono - tsk->start_time; // elapsed in ns > + btime = real - delta; // real ns - elapsed ns > + do_div(btime, NSEC_PER_SEC); // truncated to seconds > + stats->ac_btime = btime; That has pretty much the same problem as just storing the CLOCK_REALTIME start time at fork and additionally it is wreckaged vs. suspend resume. So a CLOCK_REALTIME time stamp at fork would at least be correct vs. suspend resume. The same result is achieved by: boot = ktime_get_boot_ns(); delta = boot = tsk->real_start_time; Typing real_start_time makes me really cringe. Thanks, tglx
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de> To: ltp@lists.linux.it Subject: [LTP] [PATCH] kernel: use ktime_get_real_ts64() to calculate acct.ac_btime Date: Thu, 7 Nov 2019 13:40:47 +0100 (CET) [thread overview] Message-ID: <alpine.DEB.2.21.1911071335320.4256@nanos.tec.linutronix.de> (raw) In-Reply-To: <20191107123224.GA6315@hirez.programming.kicks-ass.net> On Thu, 7 Nov 2019, Peter Zijlstra wrote: > Lets start by saying this accounting stuff is terrible crap and it > deserves to fail and burn. No argument about that. > And what does btime want? As implemented it jumps around if you ask the > question twice with an adjtime() call or suspend in between. Of course, > if we take an actual CLOCK_REALTIME timestamp at fork() the value > doesn't change, but then it can be in the future (DST,adjtime()), which > is exactly the reason why CLOCK_REALTIME is absolute shit for timestamps > (logging, accounting, etc.). > > And your 'fix' is pretty terible too. Arguably ktime_get_seconds() wants > fixing for not having the ns accumulation and actually differing from > tv_sec, but now you accrue one source of ns while still disregarding > another (also, I friggin hate timespec, it's a terrible interface for > time). > > All in all, I'm tempted to just declare this stuff broken and -EWONTFIX, > but if we have to do something, something like the below is at least > internally consistent. Kinda :) > + mono = ktime_get_ns(); > + real = ktime_get_real_ns(); > + /* > + * Compute btime by subtracting the elapsed time from the current > + * CLOCK_REALTIME. > + * > + * XXX totally buggered, because it changes results across > + * adjtime() calls and suspend/resume. > + */ > + delta = mono - tsk->start_time; // elapsed in ns > + btime = real - delta; // real ns - elapsed ns > + do_div(btime, NSEC_PER_SEC); // truncated to seconds > + stats->ac_btime = btime; That has pretty much the same problem as just storing the CLOCK_REALTIME start time at fork and additionally it is wreckaged vs. suspend resume. So a CLOCK_REALTIME time stamp at fork would at least be correct vs. suspend resume. The same result is achieved by: boot = ktime_get_boot_ns(); delta = boot = tsk->real_start_time; Typing real_start_time makes me really cringe. Thanks, tglx
next prev parent reply other threads:[~2019-11-07 12:40 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-01 23:39 [PATCH] kernel: use ktime_get_real_ts64() to calculate acct.ac_btime Jan Stancek 2019-11-01 23:39 ` [LTP] " Jan Stancek 2019-11-07 8:56 ` Thomas Gleixner 2019-11-07 8:56 ` [LTP] " Thomas Gleixner 2019-11-07 12:49 ` Peter Zijlstra 2019-11-07 12:49 ` [LTP] " Peter Zijlstra 2019-11-07 12:32 ` Peter Zijlstra 2019-11-07 12:32 ` [LTP] " Peter Zijlstra 2019-11-07 12:40 ` Thomas Gleixner [this message] 2019-11-07 12:40 ` Thomas Gleixner 2019-11-07 12:46 ` Peter Zijlstra 2019-11-07 12:46 ` [LTP] " Peter Zijlstra 2019-11-07 12:55 ` Peter Zijlstra 2019-11-07 12:55 ` [LTP] " Peter Zijlstra 2019-11-07 14:37 ` Thomas Gleixner 2019-11-07 14:37 ` [LTP] " Thomas Gleixner 2019-11-11 9:41 ` Jan Stancek 2019-11-11 9:41 ` [LTP] " Jan Stancek
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.DEB.2.21.1911071335320.4256@nanos.tec.linutronix.de \ --to=tglx@linutronix.de \ --cc=gregkh@linuxfoundation.org \ --cc=jstancek@redhat.com \ --cc=kstewart@linuxfoundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=ltp@lists.linux.it \ --cc=peterz@infradead.org \ --cc=rfontana@redhat.com \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.