From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1465448705-25055-1-git-send-email-deepa.kernel@gmail.com> <1465448705-25055-5-git-send-email-deepa.kernel@gmail.com> From: Linus Torvalds Date: Thu, 9 Jun 2016 14:02:03 -0700 Message-ID: Subject: Re: [PATCH 04/21] fs: Replace CURRENT_TIME with current_fs_time() for inode timestamps To: Deepa Dinamani Cc: linux-fsdevel , Linux Kernel Mailing List , Arnd Bergmann , Thomas Gleixner , Al Viro , y2038@lists.linaro.org, Steve French , "linux-cifs@vger.kernel.org" , samba-technical@lists.samba.org, Joern Engel , Prasad Joshi , logfs@logfs.org, Andrew Morton , Julia Lawall , David Howells , Firo Yang , Jaegeuk Kim , Changman Lee , Chao Yu , "Linux F2FS DEV, Mailing List" , Michal Hocko , Konstantin Khlebnikov , Naoya Horiguchi , "J. Bruce Fields" , Jeff Layton , Trond Myklebust , Anna Schumaker , "David S. Miller" , Linux NFS Mailing List , Network Development , Steven Whitehouse , Bob Peterson , cluster-devel , Mark Fasheh , Joel Becker , ocfs2-devel@oss.oracle.com, Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck , Chris Mason , Josef Bacik , David Sterba , linux-btrfs , Miklos Szeredi , "open list:FUSE: FILESYSTEM..." , Felipe Balbi , Greg Kroah-Hartman , USB list , Doug Ledford , Sean Hefty , Hal Rosenstock , "linux-rdma@vger.kernel.org" , Robert Richter , "oprofile-list@lists.sf.net" , Alexei Starovoitov , Hugh Dickins , linux-mm , Paul Moore , Stephen Smalley , Eric Paris , selinux@tycho.nsa.gov, James Morris , "Serge E. Hallyn" , LSM List , Eric Van Hensbergen , Ron Minnich , Latchesar Ionkov , V9FS Developers , Ian Kent , autofs mailing list , Matthew Garrett , Jeremy Kerr , Matt Fleming , "linux-efi@vger.kernel.org" , Peter Hurley , Josh Triplett , Boaz Harrosh , Benny Halevy , open-osd , Mike Marshall , pvfs2-developers@beowulf-underground.org, Nadia Yvette Chambers , Dave Kleikamp , jfs-discussion@lists.sourceforge.net, Ryusuke Konishi , linux-nilfs@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: owner-linux-mm@kvack.org List-ID: On Thu, Jun 9, 2016 at 1:38 PM, Deepa Dinamani wrote: > > 1. There are a few link, rename functions which assign times like this: > > - inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME; > + inode->i_ctime = dir->i_ctime = dir->i_mtime = > current_fs_time(dir->i_sb); So I think you should just pass one any of the two inodes and just add a comment. Then, if we hit a filesystem that actually wants to have different granularity for different inodes, we'll split it up, but even then we'd be better off than with the superblock, since then we *could* easily split this one case up into "get directory time" and "get inode time". > 2. Also, this means that we will make it an absolute policy that any filesystem > timestamp that is not directly connected to an inode would have to use > ktime_get_* apis. The thing is, those kinds of things are all going to be inside the filesystem itself. At that point, the *filesystem* already knows what the timekeeping rules for that filesystem is. I think we should strive to design the "current_fs_time()" not for internal filesystem use, but for actual generic use where we *don't* know a priori what the rules are, and we have to go to this helper function to figure it out. Inside a filesystem, why *shouldn't* the low-level filesystem already use the normal "get time" functions? See what I'm saying? The primary value-add to "current_fs_time()" is for layers like the VFS and security layer that don't know what the filesystem itself does. At the low-level filesystem layer, you may just know that "ok, I only have 32-bit timestamps anyway, so I should just use a 32-bit time function". > 3. Even if the filesystem inode has extra timestamps and these are not > part of vfs inode, we still use > vfs inode to get the timestamps from current_fs_time(): Eg: ext4 create time But those already have an inode. In fact, ext4 is a particularly bad example, since it uses the ext4_current_time() function to get the time. And that one gets an inode pointer. So at least one filesystem that already does this, already uses a inode-based model. Everything I see just says "times are about inodes". Anything else almost has to be filesystem-internal anyway, since the only thing that is ever visible outside the filesystem (time-wise) is the inode. And as mentioned, once it's internal to the low-level filesystem, it's not obvious at all that you'd have to use "currenf_fs_time()" anyway. The internal filesystem code might very well decide to use other timekeeping functions. Linus -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org