| Message ID | 1456154274-30487-9-git-send-email-deepa.kernel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 22, 2016 at 10:17 AM, Deepa Dinamani <deepa.kernel@gmail.com> wrote: > > CURRENT_TIME macro is not appropriate for filesystems as it > doesn't use the right granularity for filesystem timestamps. > Use current_fs_time() instead. > > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com> > Cc: "J. Bruce Fields" <bfields@fieldses.org> > Cc: Jeff Layton <jlayton@poochiereds.net> > Cc: Trond Myklebust <trond.myklebust@primarydata.com> > Cc: Anna Schumaker <anna.schumaker@netapp.com> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: linux-nfs@vger.kernel.org > Cc: netdev@vger.kernel.org > --- > net/sunrpc/rpc_pipe.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c > index 31789ef..bab3187 100644 > --- a/net/sunrpc/rpc_pipe.c > +++ b/net/sunrpc/rpc_pipe.c > @@ -477,7 +477,9 @@ rpc_get_inode(struct super_block *sb, umode_t mode) > return NULL; > inode->i_ino = get_next_ino(); > inode->i_mode = mode; > - inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; > + inode->i_atime = current_fs_time(sb); > + inode->i_mtime = inode->i_atime; > + inode->i_ctime = inode->i_atime; > switch (mode & S_IFMT) { > case S_IFDIR: > inode->i_fop = &simple_dir_operations; Why would we care? This is a pseudo-fs. There is no expectation w.r.t. timestamp accuracy or resolution. Cheers, Trond -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 22, 2016 at 9:04 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > On Mon, Feb 22, 2016 at 10:17 AM, Deepa Dinamani <deepa.kernel@gmail.com> wrote: >> >> CURRENT_TIME macro is not appropriate for filesystems as it >> doesn't use the right granularity for filesystem timestamps. >> Use current_fs_time() instead. >> >> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com> >> Cc: "J. Bruce Fields" <bfields@fieldses.org> >> Cc: Jeff Layton <jlayton@poochiereds.net> >> Cc: Trond Myklebust <trond.myklebust@primarydata.com> >> Cc: Anna Schumaker <anna.schumaker@netapp.com> >> Cc: "David S. Miller" <davem@davemloft.net> >> Cc: linux-nfs@vger.kernel.org >> Cc: netdev@vger.kernel.org >> --- >> net/sunrpc/rpc_pipe.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c >> index 31789ef..bab3187 100644 >> --- a/net/sunrpc/rpc_pipe.c >> +++ b/net/sunrpc/rpc_pipe.c >> @@ -477,7 +477,9 @@ rpc_get_inode(struct super_block *sb, umode_t mode) >> return NULL; >> inode->i_ino = get_next_ino(); >> inode->i_mode = mode; >> - inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; >> + inode->i_atime = current_fs_time(sb); >> + inode->i_mtime = inode->i_atime; >> + inode->i_ctime = inode->i_atime; >> switch (mode & S_IFMT) { >> case S_IFDIR: >> inode->i_fop = &simple_dir_operations; > > Why would we care? This is a pseudo-fs. There is no expectation w.r.t. > timestamp accuracy or resolution. CURRENT_TIME is not y2038 safe. We are switching all vfs timestamps to use 64 bit time. And, deleting all references to CURRENT_TIME. So CURRENT_TIME users will be replaced by either ktime_get_* apis or current_fs_time_* apis. There is going to be another series (based on https://lkml.org/lkml/2016/2/12/76) which switches vfs time to 64 bit. This means [a,c,m] times in struct inode, struct attr and struct kstat will be changed to use timespec64. Even though this is a psuedo-fs, we choose to use filesystem apis rather than ktime_get_* apis as current_fs_time_* apis will switch time data types together with vfs. -Deepa -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 22 February 2016 10:34:31 Trond Myklebust wrote: > > diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c > > index 31789ef..bab3187 100644 > > --- a/net/sunrpc/rpc_pipe.c > > +++ b/net/sunrpc/rpc_pipe.c > > @@ -477,7 +477,9 @@ rpc_get_inode(struct super_block *sb, umode_t mode) > > return NULL; > > inode->i_ino = get_next_ino(); > > inode->i_mode = mode; > > - inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; > > + inode->i_atime = current_fs_time(sb); > > + inode->i_mtime = inode->i_atime; > > + inode->i_ctime = inode->i_atime; > > switch (mode & S_IFMT) { > > case S_IFDIR: > > inode->i_fop = &simple_dir_operations; > > Why would we care? This is a pseudo-fs. There is no expectation w.r.t. > timestamp accuracy or resolution. As Deepa said, the primary goal is to unify the interface, so we have only one place function that handles setting the time in an inode, and we want to avoid CURRENT_TIME, as that is currently used in a couple of places for things other than inode timestamps that we don't want to convert to 64-bit times at the same time. current_fs_time() has a very small overhead compared to CURRENT_TIME, if you think it's worth avoiding that, we could introduce a current_pseudo_fs_time() function that does not take a superblock argument but is more tied to inode timestamps than CURRENT_TIME is. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index 31789ef..bab3187 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -477,7 +477,9 @@ rpc_get_inode(struct super_block *sb, umode_t mode) return NULL; inode->i_ino = get_next_ino(); inode->i_mode = mode; - inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; + inode->i_atime = current_fs_time(sb); + inode->i_mtime = inode->i_atime; + inode->i_ctime = inode->i_atime; switch (mode & S_IFMT) { case S_IFDIR: inode->i_fop = &simple_dir_operations;
CURRENT_TIME macro is not appropriate for filesystems as it doesn't use the right granularity for filesystem timestamps. Use current_fs_time() instead. Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com> Cc: "J. Bruce Fields" <bfields@fieldses.org> Cc: Jeff Layton <jlayton@poochiereds.net> Cc: Trond Myklebust <trond.myklebust@primarydata.com> Cc: Anna Schumaker <anna.schumaker@netapp.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: linux-nfs@vger.kernel.org Cc: netdev@vger.kernel.org --- net/sunrpc/rpc_pipe.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)