From: Cyril Hrubis <chrubis@suse.cz> To: Thomas Gleixner <tglx@linutronix.de> Cc: Li Wang <liwang@redhat.com>, ltp@lists.linux.it, Chunyu Hu <chuhu@redhat.com>, LKML <linux-kernel@vger.kernel.org>, Peter Zijlstra <peterz@infradead.org>, John Stultz <john.stultz@linaro.org>, Arnd Bergmann <arnd@kernel.org>, Vincenzo Frascino <vincenzo.frascino@arm.com>, Andy Lutomirski <luto@kernel.org>, "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>, Carlos O'Donell <carlos@redhat.com> Subject: Re: [PATCH 1/2] syscalls: avoid time() using __cvdso_gettimeofday in use-level's VDSO Date: Wed, 25 Nov 2020 13:35:34 +0100 [thread overview] Message-ID: <20201125123534.GA28684@yuki.lan> (raw) In-Reply-To: <875z5tllih.fsf@nanos.tec.linutronix.de> Hi! > This is a general problem and not really just for this particular test > case. > > Due to the internal implementation of ktime_get_real_seconds(), which is > a 2038 safe replacement for the former get_seconds() function, this > accumulation issue can be observed. (time(2) via syscall and newer > versions of VDSO use the same mechanism). > > clock_gettime(CLOCK_REALTIME, &ts); > sec = time(); > assert(sec >= ts.tv_sec); > > That assert can trigger for two reasons: > > 1) Clock was set between the clock_gettime() and time(). > > 2) The clock has advanced far enough that: > > timekeeper.tv_nsec + (clock_now_ns() - last_update_ns) > NSEC_PER_SEC > > #1 is just a property of clock REALTIME. There is nothing we can do > about that. > > #2 is due to the optimized get_seconds()/time() access which avoids to > read the clock. This can happen on bare metal as well, but is far > more likely to be exposed on virt. > > The same problem exists for CLOCK_XXX vs. CLOCK_XXX_COARSE > > clock_gettime(CLOCK_XXX, &ts); > clock_gettime(CLOCK_XXX_COARSE, &tc); > assert(tc.tv_sec >= ts.tv_sec); > > The _COARSE variants return their associated timekeeper.tv_sec,tv_nsec > pair without reading the clock. Same as #2 above just extended to clock > MONOTONIC. Good hint, I guess that easiest fix would be to switch to coarse timers for these tests. > There is no way to fix this except giving up on the fast accessors and > make everything take the slow path and read the clock, which might make > a lot of people unhappy. That's understandable and reasonable. Thanks a lot for the confirmation. > For clock REALTIME #1 is anyway an issue, so I think documenting this > proper is the right thing to do. > > Thoughts? I guess that ideally BUGS section for time(2) and clock_gettime(2) should be updated with this explanation. -- Cyril Hrubis chrubis@suse.cz
WARNING: multiple messages have this Message-ID (diff)
From: Cyril Hrubis <chrubis@suse.cz> To: ltp@lists.linux.it Subject: [LTP] [PATCH 1/2] syscalls: avoid time() using __cvdso_gettimeofday in use-level's VDSO Date: Wed, 25 Nov 2020 13:35:34 +0100 [thread overview] Message-ID: <20201125123534.GA28684@yuki.lan> (raw) In-Reply-To: <875z5tllih.fsf@nanos.tec.linutronix.de> Hi! > This is a general problem and not really just for this particular test > case. > > Due to the internal implementation of ktime_get_real_seconds(), which is > a 2038 safe replacement for the former get_seconds() function, this > accumulation issue can be observed. (time(2) via syscall and newer > versions of VDSO use the same mechanism). > > clock_gettime(CLOCK_REALTIME, &ts); > sec = time(); > assert(sec >= ts.tv_sec); > > That assert can trigger for two reasons: > > 1) Clock was set between the clock_gettime() and time(). > > 2) The clock has advanced far enough that: > > timekeeper.tv_nsec + (clock_now_ns() - last_update_ns) > NSEC_PER_SEC > > #1 is just a property of clock REALTIME. There is nothing we can do > about that. > > #2 is due to the optimized get_seconds()/time() access which avoids to > read the clock. This can happen on bare metal as well, but is far > more likely to be exposed on virt. > > The same problem exists for CLOCK_XXX vs. CLOCK_XXX_COARSE > > clock_gettime(CLOCK_XXX, &ts); > clock_gettime(CLOCK_XXX_COARSE, &tc); > assert(tc.tv_sec >= ts.tv_sec); > > The _COARSE variants return their associated timekeeper.tv_sec,tv_nsec > pair without reading the clock. Same as #2 above just extended to clock > MONOTONIC. Good hint, I guess that easiest fix would be to switch to coarse timers for these tests. > There is no way to fix this except giving up on the fast accessors and > make everything take the slow path and read the clock, which might make > a lot of people unhappy. That's understandable and reasonable. Thanks a lot for the confirmation. > For clock REALTIME #1 is anyway an issue, so I think documenting this > proper is the right thing to do. > > Thoughts? I guess that ideally BUGS section for time(2) and clock_gettime(2) should be updated with this explanation. -- Cyril Hrubis chrubis@suse.cz
next prev parent reply other threads:[~2020-11-25 12:34 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-23 8:31 [LTP] [PATCH 1/2] syscalls: avoid time() using __cvdso_gettimeofday in use-level's VDSO Li Wang 2020-11-23 8:31 ` [LTP] [PATCH 2/2] syscalls: shift to time() if __NR_time not support Li Wang 2020-11-24 2:56 ` Yang Xu 2020-11-24 7:57 ` Li Wang 2020-11-24 10:24 ` Yang Xu 2020-11-24 15:38 ` [LTP] [PATCH 1/2] syscalls: avoid time() using __cvdso_gettimeofday in use-level's VDSO Cyril Hrubis 2020-11-25 11:32 ` Thomas Gleixner 2020-11-25 11:32 ` [LTP] " Thomas Gleixner 2020-11-25 12:35 ` Cyril Hrubis [this message] 2020-11-25 12:35 ` Cyril Hrubis 2020-11-26 11:36 ` Vincenzo Frascino 2020-11-26 11:36 ` [LTP] " Vincenzo Frascino
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=20201125123534.GA28684@yuki.lan \ --to=chrubis@suse.cz \ --cc=arnd@kernel.org \ --cc=carlos@redhat.com \ --cc=chuhu@redhat.com \ --cc=john.stultz@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=liwang@redhat.com \ --cc=ltp@lists.linux.it \ --cc=luto@kernel.org \ --cc=mtk.manpages@gmail.com \ --cc=peterz@infradead.org \ --cc=tglx@linutronix.de \ --cc=vincenzo.frascino@arm.com \ /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.