Re: [PATCH] kdb: Change timespec to use timespec64

[Arnd Bergmann]

On Thu, Jan 25, 2018 at 3:49 PM, Jason Wessel
<jason.wessel@xxxxxxxxxxxxx> wrote:
> On 01/25/2018 05:38 AM, Daniel Thompson wrote:
>>
>> On Thu, Jan 25, 2018 at 05:18:54PM +0800, Baolin Wang wrote:
>>>
>>> On 25 January 2018 at 16:55, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>>>>
>>>> On Thu, Jan 25, 2018 at 9:05 AM, Baolin Wang <baolin.wang@xxxxxxxxxx>
>>>> wrote:
>>>>>
>>>>> @@ -2554,7 +2554,7 @@ static int kdb_summary(int argc, const char
>>>>> **argv)
>>>>>          kdb_printf("domainname %s\n", init_uts_ns.name.domainname);
>>>>>          kdb_printf("ccversion  %s\n", __stringify(CCVERSION));
>>>>>
>>>>> -       now = __current_kernel_time();
>>>>> +       now = current_kernel_time64();
>>>>>          kdb_gmtime(&now, &tm);
>>>>>          kdb_printf("date       %04d-%02d-%02d %02d:%02d:%02d "
>>>>>                     "tz_minuteswest %d\n",
>>>>
>>>>
>>>> Thanks for picking this one up again, we should find a permanent
>>>> solution here.
>>>> Unfortunately you patch is incorrect, as we cannot safely call
>>>> current_kernel_time64()
>>>> from NMI context.
>>>
>>>
>>> Ah, thanks for pointing out the issue, since I do not know what
>>> context the function will be called in kdb.
>>>
>>>>
>>>> The __ prefix on __current_kernel_time() indicates that this is a
>>>> special call
>>>> that intentionally doesn't read the hardware time to avoid taking locks
>>>> that
>>>> might already be held in the context from which we entered the debugger.
>>>>
>>>> See https://patchwork.kernel.org/patch/10002097/ for my earlier patch.
>>>
>>>
>>> This patch had not been merged into mainline?
>>
>>
>> Not yet (and I'm afraid it's not in kgdb-next either) but the ack from
>> Jason is from
>> this kernel cycle so we'll see what can be done!
>>
>>
>
> I thought for what ever reason this was going through the time keeper
> subtree.   I added it immediately to kgdb-next so it will be evaluated in
> the linux-next tree in the next day or so, and we can get this merged in the
> merge window.

Ok, thanks a lot!

We should still come up with a patch for kdb_sysinfo(), which doesn't
have a problem with time overflow (monotonic time doesn't overflow)
but has an issue with locking and uses 'struct timespec'.

Baolin, could you respin your patch on top of Jason's tree and
replace ktime_get_ts64() with something based on ktime_get_fast_ns?

        Arnd