From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752598AbdLFVuq (ORCPT ); Wed, 6 Dec 2017 16:50:46 -0500 Received: from mail1.windriver.com ([147.11.146.13]:38887 "EHLO mail1.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752172AbdLFVuo (ORCPT ); Wed, 6 Dec 2017 16:50:44 -0500 Subject: Re: [PATCH] kdb: use __ktime_get_real_seconds instead of __current_kernel_time To: Arnd Bergmann , Thomas Gleixner CC: , John Stultz , Stephen Boyd , Andy Shevchenko , Deepa Dinamani , Ingo Molnar , Andrew Morton , , References: <20171012140659.2749268-1-arnd@arndb.de> From: Jason Wessel Message-ID: <06d83e21-4de3-a86a-fe2d-9006369836e7@windriver.com> Date: Wed, 6 Dec 2017 15:50:14 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20171012140659.2749268-1-arnd@arndb.de> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/12/2017 09:06 AM, Arnd Bergmann wrote: > kdb is the only user of the __current_kernel_time() interface, which is > not y2038 safe and should be removed at some point. > > The kdb code also goes to great lengths to print the time in a > human-readable format from 'struct timespec', again using a non-y2038-safe > re-implementation of the generic time_to_tm() code. > > Using __current_kernel_time() here is necessary since the regular > accessors that require a sequence lock might hang when called during the > xtime update. However, this is safe in the particular case since kdb is > only interested in the tv_sec field that is updated atomically. > > In order to make this y2038-safe, I'm converting the code to the generic > time64_to_tm helper, but that introduces the problem that we have no > interface like __current_kernel_time() that provides a 64-bit timestamp > in a lockless, safe and architecture-independent way. I have multiple > ideas for how to solve that: > > - __ktime_get_real_seconds() is lockless, but can return > incorrect results on 32-bit architectures in the special case that > we are in the process of changing the time across the epoch, either > during the timer tick that overflows the seconds in 2038, or while > calling settimeofday. > > - ktime_get_real_fast_ns() would work in this context, but does > require a call into the clocksource driver to return a high-resolution > timestamp. This may have undesired side-effects in the debugger, > since we want to limit the interactions with the rest of the kernel. > > - Adding a ktime_get_real_fast_seconds() based on tk_fast_mono > plus tkr->base_real without the tk_clock_read() delta. Not sure about > the value of adding yet another interface here. > > - Changing the existing ktime_get_real_seconds() to use > tk_fast_mono on 32-bit architectures rather than xtime_sec. I think > this could work, but am not entirely sure if this is an improvement. > > I picked the first of those for simplicity here. It's technically > not correct but probably good enough as the time is only used for the > debugging output and the race will likely never be hit in practice. > Another downside is having to move the declaration into a public header > file. > > Let me know if anyone has a different preference. It all seems reasonable to me. Separately I created the same patch because I didn't see this mail first. The only difference was that I added a comment about the __ktime_get_real_seconds() not taking the lock because it was done that way in other places in the header file. === extern time64_t ktime_get_real_seconds(void); +/* does not take xtime_lock */ +extern time64_t __ktime_get_real_seconds(void); === Acked-by: Jason Wessel Thanks for your work on the 2038 problems. :-) Cheers, Jason.