From: Dmitry Safonov <0x7f454c46@gmail.com>
To: Thomas Gleixner <tglx@linutronix.de>, Dmitry Safonov <dima@arista.com>
Cc: linux-kernel@vger.kernel.org, Andrei Vagin <avagin@gmail.com>,
Adrian Reber <adrian@lisas.de>, Andrei Vagin <avagin@openvz.org>,
Andy Lutomirski <luto@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
Christian Brauner <christian.brauner@ubuntu.com>,
Cyrill Gorcunov <gorcunov@openvz.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
Jann Horn <jannh@google.com>, Jeff Dike <jdike@addtoit.com>,
Oleg Nesterov <oleg@redhat.com>,
Pavel Emelyanov <xemul@virtuozzo.com>,
Shuah Khan <shuah@kernel.org>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
containers@lists.linux-foundation.org, criu@openvz.org,
linux-api@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCHv4 06/28] timerfd/timens: Take into account ns clock offsets
Date: Sun, 16 Jun 2019 18:43:49 +0100 [thread overview]
Message-ID: <bf28eefd-390b-5209-09c1-db9874030369@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1906141534090.1722@nanos.tec.linutronix.de>
On 6/14/19 2:37 PM, Thomas Gleixner wrote:
> On Wed, 12 Jun 2019, Dmitry Safonov wrote:
>> ---
>> fs/timerfd.c | 3 +++
>> include/linux/time_namespace.h | 18 ++++++++++++++++++
>> kernel/time_namespace.c | 27 +++++++++++++++++++++++++++
>> 3 files changed, 48 insertions(+)
>
> Again, please split that into:
>
> 1) Introduce the new function
>
> 2) Make use of it
Will do
>
>> diff --git a/fs/timerfd.c b/fs/timerfd.c
>> index 6a6fc8aa1de7..9b0c2f65e7e8 100644
>> --- a/fs/timerfd.c
>> +++ b/fs/timerfd.c
>> @@ -26,6 +26,7 @@
>> #include <linux/syscalls.h>
>> #include <linux/compat.h>
>> #include <linux/rcupdate.h>
>> +#include <linux/time_namespace.h>
>>
>> struct timerfd_ctx {
>> union {
>> @@ -196,6 +197,8 @@ static int timerfd_setup(struct timerfd_ctx *ctx, int flags,
>> }
>>
>> if (texp != 0) {
>> + if (flags & TFD_TIMER_ABSTIME)
>> + texp = timens_ktime_to_host(clockid, texp);
>> if (isalarm(ctx)) {
>> if (flags & TFD_TIMER_ABSTIME)
>> alarm_start(&ctx->t.alarm, texp);
>> diff --git a/include/linux/time_namespace.h b/include/linux/time_namespace.h
>> index 1dda8af6b9fe..d32b55fad953 100644
>> --- a/include/linux/time_namespace.h
>> +++ b/include/linux/time_namespace.h
>> @@ -56,6 +56,19 @@ static inline void timens_add_boottime(struct timespec64 *ts)
>> *ts = timespec64_add(*ts, ns_offsets->boottime);
>> }
>>
>> +ktime_t do_timens_ktime_to_host(clockid_t clockid, ktime_t tim,
>> + struct timens_offsets *offsets);
>> +static inline ktime_t timens_ktime_to_host(clockid_t clockid, ktime_t tim)
>> +{
>> + struct timens_offsets *offsets = current->nsproxy->time_ns->offsets;
>> +
>> + if (!offsets) /* fast-path for the root time namespace */
>
> Can you please avoid tail comments. They break the reading flow. Aside of
> that I don't see the value of documenting the obvious.
>
>> +ktime_t do_timens_ktime_to_host(clockid_t clockid, ktime_t tim, struct timens_offsets *ns_offsets)
>
> Please line break the arguments
>
> ktime_t do_timens_ktime_to_host(clockid_t clockid, ktime_t tim,
> struct timens_offsets *ns_offsets)
Sure
>
>> +{
>> + ktime_t koff;
>> +
>> + switch (clockid) {
>> + case CLOCK_MONOTONIC:
>> + koff = timespec64_to_ktime(ns_offsets->monotonic);
>> + break;
>> + case CLOCK_BOOTTIME:
>> + case CLOCK_BOOTTIME_ALARM:
>> + koff = timespec64_to_ktime(ns_offsets->boottime);
>> + break;
>> + default:
>> + return tim;
>> + }
>> +
>> + /* tim - off has to be in [0, KTIME_MAX) */
>
> Please be more elaborate why the below conditions can happen at all.
>
>> + if (tim < koff)
>> + tim = 0;
>> + else if (KTIME_MAX - tim < -koff)
>> + tim = KTIME_MAX;
>> + else
>> + tim = ktime_sub(tim, koff);
Thanks,
Dmitry
next prev parent reply other threads:[~2019-06-16 17:43 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-12 19:25 [PATCHv4 00/28] kernel: Introduce Time Namespace Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 01/28] ns: " Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 02/28] timens: Add timens_offsets Dmitry Safonov
2019-06-14 13:11 ` Thomas Gleixner
2019-06-14 14:32 ` Dmitry Safonov
2019-07-29 22:26 ` Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 03/28] posix-clocks: add another call back to return clock time in ktime_t Dmitry Safonov
2019-06-14 13:32 ` Thomas Gleixner
2019-06-14 14:39 ` Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 04/28] timens: Introduce CLOCK_MONOTONIC offsets Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 05/28] timens: Introduce CLOCK_BOOTTIME offset Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 06/28] timerfd/timens: Take into account ns clock offsets Dmitry Safonov
2019-06-14 13:37 ` Thomas Gleixner
2019-06-16 17:43 ` Dmitry Safonov [this message]
2019-06-12 19:26 ` [PATCHv4 07/28] posix-timers/timens: Take into account " Dmitry Safonov
2019-06-14 13:42 ` Thomas Gleixner
2019-06-16 17:45 ` Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 08/28] timens/kernel: Take into account timens clock offsets in clock_nanosleep Dmitry Safonov
2019-06-14 13:49 ` Thomas Gleixner
2019-06-12 19:26 ` [PATCHv4 09/28] timens: Shift /proc/uptime Dmitry Safonov
2019-06-14 13:50 ` Thomas Gleixner
2019-06-16 17:48 ` Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 10/28] x86/vdso2c: Correct err messages on file opening Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 11/28] x86/vdso2c: Convert iterator to unsigned Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 12/28] x86/vdso/Makefile: Add vobjs32 Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 13/28] x86/vdso: Restrict splitting VVAR VMA Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 14/28] x86/vdso: Rename vdso_image {.data=>.text} Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 15/28] x86/vdso: Add offsets page in vvar Dmitry Safonov
2019-06-14 13:58 ` Thomas Gleixner
2019-06-16 17:49 ` Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 16/28] x86/vdso: Allocate timens vdso Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 17/28] x86/vdso: Switch image on setns()/unshare()/clone() Dmitry Safonov
2019-06-14 14:05 ` Thomas Gleixner
2019-06-16 17:51 ` Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 18/28] vdso: introduce timens_static_branch Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 19/28] timens: Add align for timens_offsets Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 20/28] timens/fs/proc: Introduce /proc/pid/timens_offsets Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 21/28] selftest/timens: Add Time Namespace test for supported clocks Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 22/28] selftest/timens: Add a test for timerfd Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 23/28] selftest/timens: Add a test for clock_nanosleep() Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 24/28] selftest/timens: Add procfs selftest Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 25/28] selftest/timens: Add timer offsets test Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 26/28] x86/vdso: Align VDSO functions by CPU L1 cache line Dmitry Safonov
2019-06-14 14:13 ` Thomas Gleixner
2019-06-23 5:26 ` Andrei Vagin
2019-06-12 19:26 ` [PATCHv4 27/28] selftests: Add a simple perf test for clock_gettime() Dmitry Safonov
2019-06-12 19:26 ` [PATCHv4 28/28] selftest/timens: Check that a right vdso is mapped after fork and exec Dmitry Safonov
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=bf28eefd-390b-5209-09c1-db9874030369@gmail.com \
--to=0x7f454c46@gmail.com \
--cc=adrian@lisas.de \
--cc=arnd@arndb.de \
--cc=avagin@gmail.com \
--cc=avagin@openvz.org \
--cc=christian.brauner@ubuntu.com \
--cc=containers@lists.linux-foundation.org \
--cc=criu@openvz.org \
--cc=dima@arista.com \
--cc=ebiederm@xmission.com \
--cc=gorcunov@openvz.org \
--cc=hpa@zytor.com \
--cc=jannh@google.com \
--cc=jdike@addtoit.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=oleg@redhat.com \
--cc=shuah@kernel.org \
--cc=tglx@linutronix.de \
--cc=vincenzo.frascino@arm.com \
--cc=x86@kernel.org \
--cc=xemul@virtuozzo.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).