[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <af700944-694c-822b-4633-7de945f9d228@gmail.com>
Date: Fri, 14 Jun 2019 15:39:13 +0100
From: Dmitry Safonov <0x7f454c46@...il.com>
To: Thomas Gleixner <tglx@...utronix.de>,
Dmitry Safonov <dima@...sta.com>
Cc: linux-kernel@...r.kernel.org, Andrei Vagin <avagin@...il.com>,
Adrian Reber <adrian@...as.de>,
Andrei Vagin <avagin@...nvz.org>,
Andy Lutomirski <luto@...nel.org>,
Arnd Bergmann <arnd@...db.de>,
Christian Brauner <christian.brauner@...ntu.com>,
Cyrill Gorcunov <gorcunov@...nvz.org>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
Jann Horn <jannh@...gle.com>, Jeff Dike <jdike@...toit.com>,
Oleg Nesterov <oleg@...hat.com>,
Pavel Emelyanov <xemul@...tuozzo.com>,
Shuah Khan <shuah@...nel.org>,
Vincenzo Frascino <vincenzo.frascino@....com>,
containers@...ts.linux-foundation.org, criu@...nvz.org,
linux-api@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCHv4 03/28] posix-clocks: add another call back to return
clock time in ktime_t
Hi Thomas,
Thanks much for the review,
On 6/14/19 2:32 PM, Thomas Gleixner wrote:
> Dmitry,
>
> On Wed, 12 Jun 2019, Dmitry Safonov wrote:
>
>> From: Andrei Vagin <avagin@...il.com>
>>
>> The callsite in common_timer_get() has already a comment:
>> /*
>> * The timespec64 based conversion is suboptimal, but it's not
>> * worth to implement yet another callback.
>> */
>> kc->clock_get(timr->it_clock, &ts64);
>> now = timespec64_to_ktime(ts64);
>>
>> Now we are going to add time namespaces and we need to be able to get:
>
> Please avoid 'we' and try to describe the changes in a neutral technical
> form, e.g.:
>
> The upcoming support for time namespaces requires to have access to:
>
>> * clock value in a task time namespace to return it from the clock_gettime
>> syscall.
>
> - The time in a tasks time namespace for sys_clock_gettime()
>
>> * clock valuse in the root time namespace to use it in
>> common_timer_get().
>
> - The time in the root name space for common_timer_get()
>
>> It looks like another reason why we need a separate callback to return
>> clock value in ktime_t.
>
> That adds a valid reason to finally implement a separate callback which
> returns the time in ktime_t format.
>
> Hmm?
Agree, the patch has become bigger than wanted and the message could
have been better in technical sense. Will split, add kernel doc and fix
the commit message(s).
[..]
> TBH, this patch is way to big. It changes too many things at once. Can you
> please structure it this way:
>
> 1) Rename k_clock::clock_get to k_clock::clock_get_timespec and fix up all
> struct initializers
>
> 2) Rename the clock_get_timespec functions per instance
>
> 3) Add the new callback
>
> 4) Add the new functions per instance and add them to the corresponding
> struct initializers
>
> 5) Use the new callback
>
Thanks,
Dima
Powered by blists - more mailing lists