From patchwork Mon Jun 20 15:18:52 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 9187781 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2A77460871 for ; Mon, 20 Jun 2016 15:18:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 184F2271FD for ; Mon, 20 Jun 2016 15:18:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0CB5E2787D; Mon, 20 Jun 2016 15:18:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8864D26992 for ; Mon, 20 Jun 2016 15:18:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752374AbcFTPSA (ORCPT ); Mon, 20 Jun 2016 11:18:00 -0400 Received: from mout.kundenserver.de ([217.72.192.74]:57986 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753636AbcFTPRw (ORCPT ); Mon, 20 Jun 2016 11:17:52 -0400 Received: from wuerfel.localnet ([78.42.132.4]) by mrelayeu.kundenserver.de (mreue103) with ESMTPSA (Nemesis) id 0LiJAx-1bkpS22cHD-00nRJx; Mon, 20 Jun 2016 17:16:53 +0200 From: Arnd Bergmann To: Jeff Moyer Cc: Jens Axboe , Steven Rostedt , Ingo Molnar , y2038@lists.linaro.org, Hannes Reinecke , Mike Christie , Shaohua Li , linux-block@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] blktrace: avoid using timespec Date: Mon, 20 Jun 2016 17:18:52 +0200 Message-ID: <3667244.FPOaAfVsUJ@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-22-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: References: <20160617145849.3771756-1-arnd@arndb.de> <3744206.k1Hv7RyTzG@wuerfel> MIME-Version: 1.0 X-Provags-ID: V03:K0:f0CrZPvSr4Pud6N35THt2RqOrJMyHdT9pPcfHJksZz6umFs+iRZ FZ73XaftPx8/U3EeOowOpD0pqid+ENH8TfIrH3Hw6jW+QQ01JLE7Gk2+cCqqI7OJZcCVrta ElfvK9yGZAddM0PbZIHab/ZZaxRjRWbIVmS7hQKiL+hd66b94u9UC6hD9SV5keG8pJ6rS2L 3zSNOp2sWqRvFTbQVF38Q== X-UI-Out-Filterresults: notjunk:1; V01:K0:fTVoZQOvPJI=:/nRzMBhnfSNhrUfqoIM9xT Hls1AFH1JfC7vVACiYaiEvOCyIasckOQ5gdfhO0Iyu36/sDuVlHXOWS5yARySnC0uibIbTabO w9jek0qr1fcw60THHpspg2HR4n4vSex5+VparbIJg62hEsGHo+VGSsLyxvH4o95h5x4uH9z/W VcCTHbwZ5sBziD742wQHrT5CX/QKz4ssV2X3V+JLY21mkZdlVReDmGJEstkkL49l3pgdmz5BY vu70amvTVpKld4hooDemluCZBKdIa7a2LzrAVFu4RICNASEwFKQ/5W/MYA8Lrt0OUJFc36kPP 02DFFM1jYeZXOuR4NUK63kCHDb6N58LKbJMDH9z/tj1VsYatfV57FWSzeXtmeeXf6EfrEBoiG 70knG8rPNbRGnqFXXfrQPmpEY/VQSdAUoBL3m4WEczEj6X5DxEDIoL4rih781NO6VmVip2OlW M44jfZscKBf/tsqFSLXTQM+NOUszCwG3uXNDXhHShG8X99VZ/Ty5Wi1YER7D3O/1M0BXXp4hz 6BP21KXsKZ/1VeX+tCoBUNIGMdIcppQF+73oHlaGuToQ6ItuGCl9rDWkPCTH2iMgHTRt8RCH2 Ner0ATifeLjpcZHbpnlP8D+zjmlEcg0mK7VVPnHXYGnXYPy12fC7j9BVtqwv1SGu/vsZL+SjT /4wkjXJG3eDf5b2YeIiVkS1QuINAEvgh4qNpTZyz8Fyg+aSgCXiNMuZrpW03wGn1xZQPunqRE QnHQ4JYsbh90pG2f Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Monday, June 20, 2016 10:59:14 AM CEST Jeff Moyer wrote: > Arnd Bergmann writes: > > > On Friday, June 17, 2016 5:54:16 PM CEST Jeff Moyer wrote: > >> Jens Axboe writes: > >> > >> > On 06/17/2016 05:36 PM, Steven Rostedt wrote: > >> >> > >> >> Jens, > >> >> > >> >> You want to take this, or do you want me to? > >> > > >> > I'll add it to my 4.8 tree, thanks Arnd. > >> > >> + /* need to check user space to see if this breaks in y2038 or y2106 */ > >> > >> Userspace just uses it to print the timestamp, right? So do we need the > >> comment? > > > If we have more details, the comment should describe what happens and > > when it overflows. If you have the source at hand, maybe you can answer > > these: > > As far as I can tell, that value is only ever consulted when an > undocumented format option is given to blkparse. I don't think this > matters very much. Ok. > > How does it print the timestamp? Does it print the raw seconds value > > using %u (correct) or %d (incorrect), or does it convert it into > > year/month/day/hour/min/sec? > > It converts it, but only prints hour/min/sec (and nsec): > > struct timespec abs_start_time; > > ... > static void handle_notify(struct blk_io_trace *bit) > { > ... > __u32 two32[2]; > ... > abs_start_time.tv_sec = two32[0]; > abs_start_time.tv_nsec = two32[1]; > if (abs_start_time.tv_nsec < 0) { > abs_start_time.tv_sec--; > abs_start_time.tv_nsec += 1000000000; > } > ... > > static const char * > print_time(unsigned long long timestamp) > { > static char timebuf[128]; > struct tm *tm; > time_t sec; > unsigned long nsec; > > sec = abs_start_time.tv_sec + SECONDS(timestamp); > nsec = abs_start_time.tv_nsec + NANO_SECONDS(timestamp); > if (nsec >= 1000000000) { > nsec -= 1000000000; > sec += 1; > } > > tm = localtime(&sec); > snprintf(timebuf, sizeof(timebuf), > "%02u:%02u:%02u.%06lu", > tm->tm_hour, > tm->tm_min, > tm->tm_sec, > nsec / 1000); > return timebuf; > } I assume that abs_start_time is a timespec, implying that tv_sec is a time_t. This means it behaves differently on 32-bit and 64-bit systems, where the former will overflow in the conversion from a large unsigned 32-bit number to a signed 32-bit number, whereas the conversion to signed 64-bit will work correctly. However, this is ok, because 32-bit time_t is already broken for a number of reasons, and the code you quote will work correctly on any 32-bit system that is built with a future glibc that provides a 64-bit time_t. > > In the last case, how does it treat second values above 0x80000000? Are > > those printed as year 2038 or year 1902? > > We don't print the year. Ok, but the other numbers will be wrong in case of overflow. > > Are we sure that there is only one user space implementation that reads > > these values? > > We're never sure about that. However, I'd be very surprised if anything > outside of blktrace used this. Ok. Thanks a lot for the information. I think we can update the comment as in the incremental patch below. Jens, can you fold that into the original patch, or should I submit this as a new (or incremental) patch with an updated description? Signed-off-by: Arnd Bergmann --- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index b0816e4a61a5..4a3666779589 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -131,7 +131,8 @@ static void trace_note_time(struct blk_trace *bt) unsigned long flags; u32 words[2]; - /* need to check user space to see if this breaks in y2038 or y2106 */ + /* blktrace converts this to a time_t and will overflow in + 2106, not in 2038 */ ktime_get_real_ts64(&now); words[0] = (u32)now.tv_sec; words[1] = now.tv_nsec;