From: Vincenzo Frascino <vincenzo.frascino@arm.com> To: Thomas Gleixner <tglx@linutronix.de>, Cyril Hrubis <chrubis@suse.cz>, Li Wang <liwang@redhat.com> Cc: 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>, 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: Thu, 26 Nov 2020 11:36:54 +0000 [thread overview] Message-ID: <c77c4306-6a7e-01f5-c338-ec1c8ef2c0c6@arm.com> (raw) In-Reply-To: <875z5tllih.fsf@nanos.tec.linutronix.de> Hi Thomas. On 11/25/20 11:32 AM, Thomas Gleixner wrote: [...] >>> Here we propose to use '__NR_time' to invoke syscall directly that makes >>> test all get real seconds via ktime_get_real_second. > > 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. > > 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. > > For clock REALTIME #1 is anyway an issue, so I think documenting this > proper is the right thing to do. > > Thoughts? > I completely agree with your analysis and the fact that we should document this information. My proposal would be to use either the vDSO document present in the kernel [1] or the man pages for time(2) and clock_gettime(2). Probably the second would be more accessible to user space developers. [1] Documentation/ABI/stable/vdso > Thanks, > > tglx > -- Regards, Vincenzo
WARNING: multiple messages have this Message-ID (diff)
From: Vincenzo Frascino <vincenzo.frascino@arm.com> To: ltp@lists.linux.it Subject: [LTP] [PATCH 1/2] syscalls: avoid time() using __cvdso_gettimeofday in use-level's VDSO Date: Thu, 26 Nov 2020 11:36:54 +0000 [thread overview] Message-ID: <c77c4306-6a7e-01f5-c338-ec1c8ef2c0c6@arm.com> (raw) In-Reply-To: <875z5tllih.fsf@nanos.tec.linutronix.de> Hi Thomas. On 11/25/20 11:32 AM, Thomas Gleixner wrote: [...] >>> Here we propose to use '__NR_time' to invoke syscall directly that makes >>> test all get real seconds via ktime_get_real_second. > > 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. > > 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. > > For clock REALTIME #1 is anyway an issue, so I think documenting this > proper is the right thing to do. > > Thoughts? > I completely agree with your analysis and the fact that we should document this information. My proposal would be to use either the vDSO document present in the kernel [1] or the man pages for time(2) and clock_gettime(2). Probably the second would be more accessible to user space developers. [1] Documentation/ABI/stable/vdso > Thanks, > > tglx > -- Regards, Vincenzo
next prev parent reply other threads:[~2020-11-26 11: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 2020-11-25 12:35 ` [LTP] " Cyril Hrubis 2020-11-26 11:36 ` Vincenzo Frascino [this message] 2020-11-26 11:36 ` 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=c77c4306-6a7e-01f5-c338-ec1c8ef2c0c6@arm.com \ --to=vincenzo.frascino@arm.com \ --cc=arnd@kernel.org \ --cc=carlos@redhat.com \ --cc=chrubis@suse.cz \ --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 \ /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.