From: Arnd Bergmann <arnd@arndb.de> To: Deepa Dinamani <deepa.kernel@gmail.com>, John Stultz <john.stultz@linaro.org> Cc: y2038@lists.linaro.org, Linus Torvalds <torvalds@linux-foundation.org>, "Theodore Ts'o" <tytso@mit.edu>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Andreas Dilger <adilger.kernel@dilger.ca>, Al Viro <viro@zeniv.linux.org.uk>, linux-fsdevel <linux-fsdevel@vger.kernel.org>, Thomas Gleixner <tglx@linutronix.de>, "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org> Subject: Re: [Y2038] [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps Date: Tue, 14 Jun 2016 22:59:09 +0200 Message-ID: <67050321.8oB9X0ocRn@wuerfel> (raw) In-Reply-To: <CABeXuvp6ob-KC=y3FoEFQGEGXbDf2hCD07o_3ZW7Mxta4_x70Q@mail.gmail.com> On Tuesday, June 14, 2016 10:55:39 AM CEST Deepa Dinamani wrote: > On Fri, Jun 10, 2016 at 3:19 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Thursday, June 9, 2016 11:45:01 AM CEST Linus Torvalds wrote: > >> On Wed, Jun 8, 2016 at 10:04 PM, Deepa Dinamani <deepa.kernel@gmail.com> wrote: > >> > CURRENT_TIME_SEC and CURRENT_TIME are not y2038 safe. > >> > current_fs_time() will be transitioned to be y2038 safe > >> > along with vfs. > >> > > >> > current_fs_time() returns timestamps according to the > >> > granularities set in the super_block. > >> > >> All existing users and all the ones in this patch (and the others too, > >> although I didn't go through them very carefully) really would prefer > >> just passing in the inode directly, rather than the superblock. > >> > >> So I don't want to add more users of this broken interface. It was a > >> mistake to use the superblock. The fact that the time granularity > >> exists there is pretty much irrelevant. If every single user wants to > >> use an inode pointer, then that is what the function should get. > > > > I guess it would help to give the function a new name in the process, > > if only to avoid possible conflicts. That new name of course needs to > > be at least as intuitive as the old one. How about > > > > struct timespec fs_timestamp(struct inode *); > > Would moving the function to fs/ directory (filesystems.c/ super.c / > inode.c) and calling it current_time() or fs_current_time() make > sense? > The declaration is already part of fs.h. > > This is actually a vfs function. > And, the time functions it uses are already exported. > Leaving it in the time.c by renaming to current_time() would be > confusing in spite of > the struct inode* argument. I've looked up the original patch that introduced current_fs_time at http://marc.info/?l=linux-kernel&m=110134111125012&w=3 >From the patch, it's clear that current_fs_time was intentionally added to the same file as current_kernel_time() so it could be inlined there, but both functions have since been moved to different files. I agree moving both timespec_trunc and current_fs_time into fs/inode.c or fs/attr.c seems appropriate then, or we could move current_fs_time() into kernel/time/timekeeping.c and mark current_kernel_time64() inline again. When John Stultz moved this function in 2c6b47de17c7 ("Cleanup non-arch xtime uses, use get_seconds() or current_kernel_time()."), he evidently did not consider the "inline" behavior important there, no idea if this is even measurable. Arnd
next prev parent reply index Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-06-09 5:04 [PATCH 00/21] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Deepa Dinamani 2016-06-09 5:04 ` [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() Deepa Dinamani 2016-06-09 7:35 ` Jan Kara 2016-06-09 19:15 ` Linus Torvalds 2016-06-09 20:41 ` Deepa Dinamani 2016-06-09 12:31 ` Bob Copeland 2016-06-10 22:21 ` Arnd Bergmann 2016-06-11 5:03 ` Deepa Dinamani 2016-06-11 20:55 ` Arnd Bergmann 2016-06-09 5:04 ` [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps Deepa Dinamani 2016-06-09 18:45 ` Linus Torvalds 2016-06-09 18:55 ` Linus Torvalds 2016-06-10 22:19 ` [Y2038] " Arnd Bergmann 2016-06-14 17:55 ` Deepa Dinamani 2016-06-14 20:59 ` Arnd Bergmann [this message] 2016-06-09 5:04 ` [PATCH 03/21] fs: ubifs: " Deepa Dinamani 2016-06-09 5:04 ` [PATCH 05/21] fs: jfs: Replace CURRENT_TIME_SEC by current_fs_time() Deepa Dinamani 2016-06-09 5:04 ` [PATCH 06/21] fs: udf: Replace CURRENT_TIME with current_fs_time() Deepa Dinamani 2016-06-09 7:41 ` Jan Kara 2016-06-10 0:53 ` Deepa Dinamani 2016-06-09 5:04 ` [PATCH 07/21] fs: cifs: Replace CURRENT_TIME by current_fs_time() Deepa Dinamani 2016-06-09 5:04 ` [PATCH 08/21] fs: cifs: Replace CURRENT_TIME with ktime_get_real_ts() Deepa Dinamani 2016-06-09 5:04 ` [PATCH 09/21] fs: cifs: Replace CURRENT_TIME by get_seconds Deepa Dinamani 2016-06-09 5:04 ` [PATCH 10/21] fs: f2fs: Use ktime_get_real_seconds for sit_info times Deepa Dinamani 2016-06-09 5:04 ` [PATCH 11/21] drivers: staging: lustre: Replace CURRENT_TIME with current_fs_time() Deepa Dinamani 2016-06-11 0:36 ` [lustre-devel] " James Simmons 2016-06-11 1:53 ` Andreas Dilger 2016-06-09 5:04 ` [PATCH 12/21] block: rbd: Replace non inode " Deepa Dinamani 2016-06-09 5:04 ` [PATCH 13/21] fs: ocfs2: Use time64_t to represent orphan scan times Deepa Dinamani 2016-06-09 5:04 ` [PATCH 14/21] fs: ocfs2: Replace CURRENT_TIME with ktime_get_real_seconds() Deepa Dinamani 2016-06-09 5:04 ` [PATCH 15/21] time: Add time64_to_tm() Deepa Dinamani 2016-06-14 21:18 ` John Stultz 2016-06-15 17:44 ` Deepa Dinamani 2016-06-17 20:52 ` John Stultz 2016-06-17 20:59 ` Deepa Dinamani 2016-06-17 21:06 ` Arnd Bergmann 2016-06-09 5:05 ` [PATCH 16/21] fnic: Use time64_t to represent trace timestamps Deepa Dinamani 2016-06-09 5:05 ` [PATCH 17/21] audit: Use timespec64 to represent audit timestamps Deepa Dinamani 2016-06-09 14:31 ` Steve Grubb 2016-06-09 23:59 ` Richard Guy Briggs 2016-06-10 0:19 ` Steve Grubb 2016-06-10 1:44 ` Richard Guy Briggs 2016-06-15 21:23 ` Paul Moore 2016-06-10 0:45 ` Deepa Dinamani 2016-06-09 5:05 ` [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe Deepa Dinamani 2016-06-09 19:23 ` Trond Myklebust 2016-06-09 21:10 ` Deepa Dinamani 2016-06-10 13:12 ` Anna Schumaker 2016-06-10 14:02 ` Trond Myklebust 2016-06-09 5:05 ` [PATCH 19/21] libceph: Remove CURRENT_TIME references Deepa Dinamani 2016-06-09 5:05 ` [PATCH 20/21] libceph: Replace CURRENT_TIME with ktime_get_real_ts Deepa Dinamani 2016-06-09 5:05 ` [PATCH 21/21] time: Delete CURRENT_TIME_SEC and CURRENT_TIME macro Deepa Dinamani 2016-06-14 21:20 ` John Stultz 2016-06-09 7:51 ` [PATCH 00/21] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Felipe Balbi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=67050321.8oB9X0ocRn@wuerfel \ --to=arnd@arndb.de \ --cc=adilger.kernel@dilger.ca \ --cc=deepa.kernel@gmail.com \ --cc=john.stultz@linaro.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=torvalds@linux-foundation.org \ --cc=tytso@mit.edu \ --cc=viro@zeniv.linux.org.uk \ --cc=y2038@lists.linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
LKML Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \ linux-kernel@vger.kernel.org public-inbox-index lkml Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git