From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f66.google.com ([209.85.213.66]:39314 "EHLO mail-vk0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752009AbeENS25 (ORCPT ); Mon, 14 May 2018 14:28:57 -0400 Received: by mail-vk0-f66.google.com with SMTP id g83-v6so8118957vkc.6 for ; Mon, 14 May 2018 11:28:57 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20180512045921.18311-1-deepa.kernel@gmail.com> <20180512045921.18311-7-deepa.kernel@gmail.com> From: Kees Cook Date: Mon, 14 May 2018 11:28:56 -0700 Message-ID: Subject: Re: [PATCH 6/6] vfs: change inode times to use struct timespec64 To: Deepa Dinamani Cc: Al Viro , Thomas Gleixner , Arnd Bergmann , LKML , "linux-fsdevel@vger.kernel.org" , y2038 Mailman List , anton@tuxera.com, Felipe Balbi , "J. Bruce Fields" , "Darrick J. Wong" , David Howells , David Sterba , David Woodhouse , Christoph Hellwig , OGAWA Hirofumi , Mike Marshall , Jan Kara , Jaegeuk Kim , Jan Harkes , Jiri Slaby , Mark Fasheh , Miklos Szeredi , Nicolas Pitre , reiserfs-devel@vger.kernel.org, Richard Weinberger , Sage Weil , Steve French , Steven Whitehouse , Tejun Heo , Trond Myklebust , "Ted Ts'o" Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, May 14, 2018 at 11:23 AM, Deepa Dinamani wrote: > On Mon, May 14, 2018 at 10:53 AM, Kees Cook wrote: >> On Mon, May 14, 2018 at 10:25 AM, Deepa Dinamani wrote: >>> On Mon, May 14, 2018 at 9:30 AM, Kees Cook wrote: >>>> On Sun, May 13, 2018 at 9:05 PM, Deepa Dinamani wrote: >>>>> Kees mentioned that he wants to merge a patch to pstore that changes >>>>> it to use timespec64 internally for 4.17: >>>>> https://lkml.org/lkml/2018/5/13/3 >>>> >>>> I'm still working on a v2 for pstore. What is the correct >>>> cross-architecture format string for timespec64's tv_sec? In your >>>> other patches, you're using %lld and a (long long) cast. I'd really >>>> like to avoid the need for casts. >>> >>> We cannot really avoid it for now. >>> struct timespec64 is defined this way for now: >>> >>> struct timespec { >>> __kernel_time_t tv_sec; /* seconds */ >>> long tv_nsec; /* nanoseconds */ >>> }; >>> >>> #if __BITS_PER_LONG == 64 >>> /* this trick allows us to optimize out timespec64_to_timespec */ >>> # define timespec64 timespec >>> >>> #else >>> >>> struct timespec64 { >>> time64_t tv_sec; /* seconds */ >>> long tv_nsec; /* nanoseconds */ >>> }; >>> >>> #endif >>> >>> This will all lead to tv_sec being long on a 64 bit architecture and >>> long long on a 32 bit architecture. >>> So there is no way of avoiding the cast for now. >>> >>> We plan to get rid of this trick and to have a single definition for >>> timespec64. But, that cleanup is planned for later when we cleanup all >>> struct timespec uses internally. >> >> Can we do something like: >> >> #if __BITS_PER_LONG == 64 >> # define TVSEC_FMT "%ld" >> #else >> # define TVSEC_FMT "%lld" >> #endif >> >> so we can do stuff like: sprintf(buf, "seconds: " KTIME_FMT, time->tv_sec) >> >> ? It seems easier to clean up than casts. > > We have already introduced these casts in many places now. > It would be easier to do the clean up if they all follow a similar > pattern. ( I could probably write a coccinelle script that is not very > long). > But, it would be not much trouble if you wanted to follow this for pstore. > > We also contemplated adding a format specifier for time. But, I think > we deferred it until we have a uniform way of using time internally. Okay, fair enough. :) I will get my pstore v2 ready with casts. Thanks! -Kees -- Kees Cook Pixel Security