Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1418084 > unrolled thread
| Started by | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| First post | 2016-06-09 07:20 +0200 |
| Last post | 2016-06-11 23:10 +0200 |
| Articles | 20 on this page of 21 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH 00/21] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-09 07:20 +0200
[PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-09 07:20 +0200
Re: [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe Trond Myklebust <trondmy@primarydata.com> - 2016-06-09 21:30 +0200
Re: [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-09 23:20 +0200
Re: [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe Anna Schumaker <Anna.Schumaker@netapp.com> - 2016-06-10 15:20 +0200
Re: [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe Trond Myklebust <trondmy@primarydata.com> - 2016-06-10 16:10 +0200
[PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-09 07:20 +0200
Re: [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-09 20:50 +0200
Re: [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-09 21:00 +0200
Re: [Y2038] [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps Arnd Bergmann <arnd@arndb.de> - 2016-06-11 00:20 +0200
Re: [Y2038] [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-14 20:00 +0200
Re: [Y2038] [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps Arnd Bergmann <arnd@arndb.de> - 2016-06-14 23:00 +0200
[PATCH 07/21] fs: cifs: Replace CURRENT_TIME by current_fs_time() Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-09 07:20 +0200
Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() Jan Kara <jack@suse.cz> - 2016-06-09 09:40 +0200
Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-09 21:20 +0200
Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-09 22:50 +0200
Re: [PATCH 00/21] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Felipe Balbi <balbi@kernel.org> - 2016-06-09 10:00 +0200
Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() Bob Copeland <me@bobcopeland.com> - 2016-06-09 14:40 +0200
Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() Arnd Bergmann <arnd@arndb.de> - 2016-06-11 00:30 +0200
Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-11 07:10 +0200
Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() Arnd Bergmann <arnd@arndb.de> - 2016-06-11 23:10 +0200
Page 1 of 2 [1] 2 Next page →
| From | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| Date | 2016-06-09 07:20 +0200 |
| Subject | [PATCH 00/21] Delete CURRENT_TIME and CURRENT_TIME_SEC macros |
| Message-ID | <rI0o9-1PH-9@gated-at.bofh.it> |
The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC macros. The macros are not y2038 safe. There is no plan to transition them into being y2038 safe. ktime_get_* api's can be used in their place. And, these are y2038 safe. All filesystem timestamps use current_fs_time() for the right granularity as mentioned in the respective commit texts of patches. This series also serves as a preparatory series to transition vfs to 64 bit timestamps as outlined here: https://lkml.org/lkml/2016/2/12/104 . As per Linus's suggestion in https://lkml.org/lkml/2016/5/24/663 , all the inode timestamp changes have been squashed into a single patch. Also, current_fs_time() now is used as a single generic filesystem timestamp api. Posting all patches together in a bigger series so that the big picture is clear. As per the suggestion in https://lwn.net/Articles/672598/ , CURRENT_TIME macro bug fixes are being handled in a series separate from transitioning vfs to use 64 bit timestamps. Some reviewers have requested not to change line wrapping only for the longer function call names, so checkpatch warnings for such cases are ignored in the patch series. Deepa Dinamani (21): fs: Replace CURRENT_TIME_SEC with current_fs_time() fs: ext4: Use current_fs_time() for inode timestamps fs: ubifs: Use current_fs_time() for inode timestamps fs: Replace CURRENT_TIME with current_fs_time() for inode timestamps fs: jfs: Replace CURRENT_TIME_SEC by current_fs_time() fs: udf: Replace CURRENT_TIME with current_fs_time() fs: cifs: Replace CURRENT_TIME by current_fs_time() fs: cifs: Replace CURRENT_TIME with ktime_get_real_ts() fs: cifs: Replace CURRENT_TIME by get_seconds fs: f2fs: Use ktime_get_real_seconds for sit_info times drivers: staging: lustre: Replace CURRENT_TIME with current_fs_time() block: rbd: Replace non inode CURRENT_TIME with current_fs_time() fs: ocfs2: Use time64_t to represent orphan scan times fs: ocfs2: Replace CURRENT_TIME with ktime_get_real_seconds() time: Add time64_to_tm() fnic: Use time64_t to represent trace timestamps audit: Use timespec64 to represent audit timestamps fs: nfs: Make nfs boot time y2038 safe libceph: Remove CURRENT_TIME references libceph: Replace CURRENT_TIME with ktime_get_real_ts time: Delete CURRENT_TIME_SEC and CURRENT_TIME macro arch/powerpc/platforms/cell/spufs/inode.c | 2 +- arch/s390/hypfs/inode.c | 4 +-- drivers/block/rbd.c | 2 +- drivers/infiniband/hw/qib/qib_fs.c | 2 +- drivers/misc/ibmasm/ibmasmfs.c | 2 +- drivers/oprofile/oprofilefs.c | 2 +- drivers/scsi/fnic/fnic_trace.c | 4 +-- drivers/scsi/fnic/fnic_trace.h | 2 +- drivers/staging/lustre/lustre/llite/llite_lib.c | 17 ++++++----- drivers/staging/lustre/lustre/llite/namei.c | 4 +-- drivers/staging/lustre/lustre/mdc/mdc_reint.c | 6 ++-- .../lustre/lustre/obdclass/linux/linux-obdo.c | 6 ++-- drivers/staging/lustre/lustre/obdclass/obdo.c | 6 ++-- drivers/staging/lustre/lustre/osc/osc_io.c | 2 +- drivers/usb/core/devio.c | 19 ++++++------ drivers/usb/gadget/function/f_fs.c | 2 +- drivers/usb/gadget/legacy/inode.c | 2 +- fs/9p/vfs_inode.c | 2 +- fs/adfs/inode.c | 2 +- fs/affs/amigaffs.c | 6 ++-- fs/affs/inode.c | 2 +- fs/afs/inode.c | 3 +- fs/autofs4/inode.c | 2 +- fs/autofs4/root.c | 19 +++++++----- fs/bfs/dir.c | 18 ++++++----- fs/btrfs/inode.c | 2 +- fs/cifs/cifsencrypt.c | 4 ++- fs/cifs/cifssmb.c | 10 +++---- fs/cifs/inode.c | 15 +++++----- fs/coda/dir.c | 2 +- fs/coda/file.c | 2 +- fs/coda/inode.c | 2 +- fs/devpts/inode.c | 6 ++-- fs/efivarfs/inode.c | 2 +- fs/exofs/dir.c | 9 +++--- fs/exofs/inode.c | 7 +++-- fs/exofs/namei.c | 6 ++-- fs/ext2/acl.c | 2 +- fs/ext2/dir.c | 6 ++-- fs/ext2/ialloc.c | 2 +- fs/ext2/inode.c | 4 +-- fs/ext2/ioctl.c | 5 ++-- fs/ext2/namei.c | 6 ++-- fs/ext2/super.c | 2 +- fs/ext2/xattr.c | 2 +- fs/ext4/acl.c | 2 +- fs/ext4/ext4.h | 6 ---- fs/ext4/extents.c | 10 +++---- fs/ext4/ialloc.c | 2 +- fs/ext4/inline.c | 4 +-- fs/ext4/inode.c | 6 ++-- fs/ext4/ioctl.c | 8 ++--- fs/ext4/namei.c | 24 ++++++++------- fs/ext4/super.c | 2 +- fs/ext4/xattr.c | 2 +- fs/f2fs/dir.c | 8 ++--- fs/f2fs/file.c | 8 ++--- fs/f2fs/inline.c | 2 +- fs/f2fs/namei.c | 12 ++++---- fs/f2fs/segment.c | 2 +- fs/f2fs/segment.h | 5 ++-- fs/f2fs/xattr.c | 2 +- fs/fat/dir.c | 2 +- fs/fat/file.c | 4 +-- fs/fat/inode.c | 2 +- fs/fat/namei_msdos.c | 13 ++++---- fs/fat/namei_vfat.c | 10 +++---- fs/fuse/control.c | 2 +- fs/gfs2/bmap.c | 8 ++--- fs/gfs2/dir.c | 12 ++++---- fs/gfs2/inode.c | 8 ++--- fs/gfs2/quota.c | 2 +- fs/gfs2/xattr.c | 8 ++--- fs/hfs/catalog.c | 8 ++--- fs/hfs/dir.c | 2 +- fs/hfs/inode.c | 2 +- fs/hfsplus/catalog.c | 8 ++--- fs/hfsplus/dir.c | 6 ++-- fs/hfsplus/inode.c | 2 +- fs/hfsplus/ioctl.c | 2 +- fs/hugetlbfs/inode.c | 10 +++---- fs/jffs2/acl.c | 2 +- fs/jffs2/fs.c | 2 +- fs/jfs/acl.c | 2 +- fs/jfs/inode.c | 5 ++-- fs/jfs/ioctl.c | 4 +-- fs/jfs/jfs_inode.c | 2 +- fs/jfs/namei.c | 35 ++++++++++++---------- fs/jfs/super.c | 2 +- fs/jfs/xattr.c | 2 +- fs/libfs.c | 14 ++++----- fs/logfs/dir.c | 6 ++-- fs/logfs/file.c | 2 +- fs/logfs/inode.c | 3 +- fs/logfs/readwrite.c | 4 +-- fs/minix/bitmap.c | 2 +- fs/minix/dir.c | 12 ++++---- fs/minix/itree_common.c | 4 +-- fs/minix/namei.c | 4 +-- fs/nfs/client.c | 2 +- fs/nfs/netns.h | 2 +- fs/nilfs2/dir.c | 6 ++-- fs/nilfs2/inode.c | 4 +-- fs/nilfs2/ioctl.c | 2 +- fs/nilfs2/namei.c | 6 ++-- fs/nsfs.c | 5 ++-- fs/ocfs2/acl.c | 2 +- fs/ocfs2/alloc.c | 2 +- fs/ocfs2/aops.c | 2 +- fs/ocfs2/cluster/heartbeat.c | 2 +- fs/ocfs2/dir.c | 4 +-- fs/ocfs2/dlmfs/dlmfs.c | 4 +-- fs/ocfs2/file.c | 12 ++++---- fs/ocfs2/inode.c | 2 +- fs/ocfs2/journal.c | 4 +-- fs/ocfs2/move_extents.c | 2 +- fs/ocfs2/namei.c | 17 ++++++----- fs/ocfs2/ocfs2.h | 2 +- fs/ocfs2/refcounttree.c | 4 +-- fs/ocfs2/super.c | 2 +- fs/ocfs2/xattr.c | 2 +- fs/omfs/dir.c | 4 +-- fs/omfs/inode.c | 2 +- fs/openpromfs/inode.c | 2 +- fs/orangefs/file.c | 2 +- fs/orangefs/inode.c | 2 +- fs/orangefs/namei.c | 6 ++-- fs/pipe.c | 5 ++-- fs/posix_acl.c | 2 +- fs/proc/base.c | 2 +- fs/proc/inode.c | 4 +-- fs/proc/proc_sysctl.c | 2 +- fs/proc/self.c | 2 +- fs/proc/thread_self.c | 2 +- fs/pstore/inode.c | 2 +- fs/ramfs/inode.c | 12 ++++---- fs/reiserfs/inode.c | 2 +- fs/reiserfs/ioctl.c | 4 +-- fs/reiserfs/namei.c | 14 ++++----- fs/reiserfs/stree.c | 6 ++-- fs/reiserfs/super.c | 2 +- fs/reiserfs/xattr.c | 2 +- fs/reiserfs/xattr_acl.c | 2 +- fs/sysv/dir.c | 6 ++-- fs/sysv/ialloc.c | 2 +- fs/sysv/itree.c | 4 +-- fs/sysv/namei.c | 4 +-- fs/tracefs/inode.c | 2 +- fs/ubifs/dir.c | 10 +++---- fs/ubifs/file.c | 12 ++++---- fs/ubifs/ioctl.c | 2 +- fs/ubifs/misc.h | 10 ------- fs/ubifs/sb.c | 18 ++++++++--- fs/ubifs/xattr.c | 6 ++-- fs/udf/super.c | 4 +-- fs/ufs/dir.c | 6 ++-- fs/ufs/ialloc.c | 8 +++-- fs/ufs/inode.c | 6 ++-- fs/ufs/namei.c | 6 ++-- include/linux/audit.h | 4 +-- include/linux/time.h | 18 ++++++++--- ipc/mqueue.c | 21 ++++++------- kernel/audit.c | 10 +++---- kernel/audit.h | 2 +- kernel/auditsc.c | 6 ++-- kernel/bpf/inode.c | 2 +- kernel/time/timeconv.c | 11 +++---- mm/shmem.c | 26 ++++++++-------- net/ceph/messenger.c | 6 ++-- net/ceph/osd_client.c | 4 +-- net/sunrpc/rpc_pipe.c | 2 +- security/inode.c | 2 +- security/selinux/selinuxfs.c | 2 +- 173 files changed, 494 insertions(+), 458 deletions(-) -- 1.9.1 Cc: Anna Schumaker <anna.schumaker@netapp.com> Cc: Anton Vorontsov <anton@enomsg.org> Cc: Benny Halevy <bhalevy@primarydata.com> Cc: Boaz Harrosh <ooo@electrozaur.com> Cc: Changman Lee <cm224.lee@samsung.com> Cc: Chris Mason <clm@fb.com> Cc: Colin Cross <ccross@android.com> Cc: Dave Kleikamp <shaggy@kernel.org> Cc: "David S. Miller" <davem@davemloft.net> Cc: David Sterba <dsterba@suse.com> Cc: Eric Van Hensbergen <ericvh@gmail.com> Cc: Felipe Balbi <balbi@kernel.org> Cc: Hugh Dickins <hughd@google.com> Cc: Ian Kent <raven@themaw.net> Cc: Jaegeuk Kim <jaegeuk@kernel.org> Cc: Joern Engel <joern@logfs.org> Cc: Josef Bacik <jbacik@fb.com> Cc: Kees Cook <keescook@chromium.org> Cc: Latchesar Ionkov <lucho@ionkov.net> Cc: Matt Fleming <matt@codeblueprint.co.uk> Cc: Matthew Garrett <matthew.garrett@nebula.com> Cc: Miklos Szeredi <miklos@szeredi.hu> Cc: Nadia Yvette Chambers <nyc@holomorphy.com> Cc: Prasad Joshi <prasadjoshi.linux@gmail.com> Cc: Robert Richter <rric@kernel.org> Cc: Ron Minnich <rminnich@sandia.gov> Cc: Tony Luck <tony.luck@intel.com> Cc: Trond Myklebust <trond.myklebust@primarydata.com> Cc: autofs@vger.kernel.org Cc: cluster-devel@redhat.com Cc: jfs-discussion@lists.sourceforge.net Cc: linux-btrfs@vger.kernel.org Cc: linux-efi@vger.kernel.org Cc: linux-f2fs-devel@lists.sourceforge.net Cc: linux-fsdevel@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-mm@kvack.org Cc: linux-nfs@vger.kernel.org Cc: linux-nilfs@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-rdma@vger.kernel.org Cc: linux-s390@vger.kernel.org Cc: linux-security-module@vger.kernel.org Cc: linux-usb@vger.kernel.org Cc: logfs@logfs.org Cc: netdev@vger.kernel.org Cc: ocfs2-devel@oss.oracle.com Cc: oprofile-list@lists.sf.net Cc: osd-dev@open-osd.org Cc: selinux@tycho.nsa.gov Cc: v9fs-developer@lists.sourceforge.net
[toc] | [next] | [standalone]
| From | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| Date | 2016-06-09 07:20 +0200 |
| Subject | [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe |
| Message-ID | <rI0xQ-1Vf-19@gated-at.bofh.it> |
| In reply to | #1418084 |
boot_time is represented as a struct timespec.
struct timespec and CURRENT_TIME are not y2038 safe.
Overall, the plan is to use timespec64 for all internal
kernel representation of timestamps.
CURRENT_TIME will also be removed.
Use struct timespec64 to represent boot_time.
And, ktime_get_real_ts64() for the boot_time value.
boot_time is used to construct the nfs client boot verifier.
This will now wrap in 2106 instead of 2038 on 32-bit systems.
The server only relies on the value being persistent until
reboot so the wrapping should be fine.
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Anna Schumaker <anna.schumaker@netapp.com>
Cc: linux-nfs@vger.kernel.org
---
fs/nfs/client.c | 2 +-
fs/nfs/netns.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 0c96528..406972e 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1080,7 +1080,7 @@ void nfs_clients_init(struct net *net)
idr_init(&nn->cb_ident_idr);
#endif
spin_lock_init(&nn->nfs_client_lock);
- nn->boot_time = CURRENT_TIME;
+ ktime_get_real_ts64(&nn->boot_time);
}
#ifdef CONFIG_PROC_FS
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index f0e06e4..48d6b95 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -29,7 +29,7 @@ struct nfs_net {
int cb_users[NFS4_MAX_MINOR_VERSION + 1];
#endif
spinlock_t nfs_client_lock;
- struct timespec boot_time;
+ struct timespec64 boot_time;
#ifdef CONFIG_PROC_FS
struct proc_dir_entry *proc_nfsfs;
#endif
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Trond Myklebust <trondmy@primarydata.com> |
|---|---|
| Date | 2016-06-09 21:30 +0200 |
| Subject | Re: [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe |
| Message-ID | <rIdOq-2fC-17@gated-at.bofh.it> |
| In reply to | #1418085 |
On 6/9/16, 01:05, "Deepa Dinamani" <deepa.kernel@gmail.com> wrote: >boot_time is represented as a struct timespec. >struct timespec and CURRENT_TIME are not y2038 safe. >Overall, the plan is to use timespec64 for all internal >kernel representation of timestamps. >CURRENT_TIME will also be removed. >Use struct timespec64 to represent boot_time. >And, ktime_get_real_ts64() for the boot_time value. > >boot_time is used to construct the nfs client boot verifier. >This will now wrap in 2106 instead of 2038 on 32-bit systems. >The server only relies on the value being persistent until >reboot so the wrapping should be fine. We really do not give a damn about wraparound here, since the boot time is only ever compared for an exact match, and the odds of two reboots occurring exactly 2^32 * 10^9 nanoseconds apart are cosmically small... If struct timespec is going away, can we just convert this into a ktime_t? Trond
[toc] | [prev] | [next] | [standalone]
| From | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| Date | 2016-06-09 23:20 +0200 |
| Subject | Re: [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe |
| Message-ID | <rIfwT-3u0-57@gated-at.bofh.it> |
| In reply to | #1418602 |
>>boot_time is represented as a struct timespec. >>struct timespec and CURRENT_TIME are not y2038 safe. >>Overall, the plan is to use timespec64 for all internal >>kernel representation of timestamps. >>CURRENT_TIME will also be removed. >>Use struct timespec64 to represent boot_time. >>And, ktime_get_real_ts64() for the boot_time value. >> >>boot_time is used to construct the nfs client boot verifier. >>This will now wrap in 2106 instead of 2038 on 32-bit systems. >>The server only relies on the value being persistent until >>reboot so the wrapping should be fine. > > We really do not give a damn about wraparound here, since the boot time is > only ever compared for an exact match, and the odds of two reboots occurring > exactly 2^32 * 10^9 nanoseconds apart are cosmically small... > If struct timespec is going away, can we just convert this into a ktime_t? timespec64 is the same as timespec already on 64 bit machines. But, yes, we can use ktime_t here. Did you mean the internal storage value or the wire boo_time used for verifier? In case you don't want to change the wire value, then we will have a division operation, every time the verifier needs to be sent. -Deepa -Deepa
[toc] | [prev] | [next] | [standalone]
| From | Anna Schumaker <Anna.Schumaker@netapp.com> |
|---|---|
| Date | 2016-06-10 15:20 +0200 |
| Subject | Re: [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe |
| Message-ID | <rIuvU-527-19@gated-at.bofh.it> |
| In reply to | #1418696 |
On 06/09/2016 05:10 PM, Deepa Dinamani wrote: >>> boot_time is represented as a struct timespec. >>> struct timespec and CURRENT_TIME are not y2038 safe. >>> Overall, the plan is to use timespec64 for all internal >>> kernel representation of timestamps. >>> CURRENT_TIME will also be removed. >>> Use struct timespec64 to represent boot_time. >>> And, ktime_get_real_ts64() for the boot_time value. >>> >>> boot_time is used to construct the nfs client boot verifier. >>> This will now wrap in 2106 instead of 2038 on 32-bit systems. >>> The server only relies on the value being persistent until >>> reboot so the wrapping should be fine. >> >> We really do not give a damn about wraparound here, since the boot time is >> only ever compared for an exact match, and the odds of two reboots occurring >> exactly 2^32 * 10^9 nanoseconds apart are cosmically small... >> If struct timespec is going away, can we just convert this into a ktime_t? > > timespec64 is the same as timespec already on 64 bit machines. > But, yes, we can use ktime_t here. > > Did you mean the internal storage value or the wire boo_time used for verifier? > In case you don't want to change the wire value, then we will have a division > operation, every time the verifier needs to be sent. The verifier is mostly used during mounting, so we don't send too many of them. I don't think we need to worry about adding an extra division operation here, they're pretty cheap compared to making RPC calls! :) Anna > > -Deepa > > -Deepa >
[toc] | [prev] | [next] | [standalone]
| From | Trond Myklebust <trondmy@primarydata.com> |
|---|---|
| Date | 2016-06-10 16:10 +0200 |
| Subject | Re: [PATCH 18/21] fs: nfs: Make nfs boot time y2038 safe |
| Message-ID | <rIvii-5yi-23@gated-at.bofh.it> |
| In reply to | #1419393 |
On 6/10/16, 09:12, "Anna Schumaker" <Anna.Schumaker@netapp.com> wrote: >On 06/09/2016 05:10 PM, Deepa Dinamani wrote: >>>> boot_time is represented as a struct timespec. >>>> struct timespec and CURRENT_TIME are not y2038 safe. >>>> Overall, the plan is to use timespec64 for all internal >>>> kernel representation of timestamps. >>>> CURRENT_TIME will also be removed. >>>> Use struct timespec64 to represent boot_time. >>>> And, ktime_get_real_ts64() for the boot_time value. >>>> >>>> boot_time is used to construct the nfs client boot verifier. >>>> This will now wrap in 2106 instead of 2038 on 32-bit systems. >>>> The server only relies on the value being persistent until >>>> reboot so the wrapping should be fine. >>> >>> We really do not give a damn about wraparound here, since the boot time is >>> only ever compared for an exact match, and the odds of two reboots occurring >>> exactly 2^32 * 10^9 nanoseconds apart are cosmically small... >>> If struct timespec is going away, can we just convert this into a ktime_t? >> >> timespec64 is the same as timespec already on 64 bit machines. >> But, yes, we can use ktime_t here. >> >> Did you mean the internal storage value or the wire boo_time used for verifier? >> In case you don't want to change the wire value, then we will have a division >> operation, every time the verifier needs to be sent. > >The verifier is mostly used during mounting, so we don't send too many of them. I don't think we need to worry about adding an extra division operation here, they're pretty cheap compared to making RPC calls! :) > The only requirement for the verifier is that it be unique, so changing the format is not a problem either. Cheers Trond
[toc] | [prev] | [next] | [standalone]
| From | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| Date | 2016-06-09 07:20 +0200 |
| Subject | [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps |
| Message-ID | <rI0xQ-1Vf-21@gated-at.bofh.it> |
| In reply to | #1418084 |
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.
The granularity check to call current_fs_time() or
CURRENT_TIME_SEC is not required.
Use current_fs_time() to obtain timestamps
unconditionally.
Quota files are assumed to be on the same filesystem.
Hence, use current_fs_time() for these files as well.
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: linux-ext4@vger.kernel.org
---
fs/ext4/acl.c | 2 +-
fs/ext4/ext4.h | 6 ------
fs/ext4/extents.c | 10 +++++-----
fs/ext4/ialloc.c | 2 +-
fs/ext4/inline.c | 4 ++--
fs/ext4/inode.c | 6 +++---
fs/ext4/ioctl.c | 8 ++++----
fs/ext4/namei.c | 24 +++++++++++++-----------
fs/ext4/super.c | 2 +-
fs/ext4/xattr.c | 2 +-
10 files changed, 31 insertions(+), 35 deletions(-)
diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
index c6601a4..f9469cc 100644
--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -197,7 +197,7 @@ __ext4_set_acl(handle_t *handle, struct inode *inode, int type,
if (error < 0)
return error;
else {
- inode->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
ext4_mark_inode_dirty(handle, inode);
if (error == 0)
acl = NULL;
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b84aa1c..14e5cf4 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1523,12 +1523,6 @@ static inline struct ext4_inode_info *EXT4_I(struct inode *inode)
return container_of(inode, struct ext4_inode_info, vfs_inode);
}
-static inline struct timespec ext4_current_time(struct inode *inode)
-{
- return (inode->i_sb->s_time_gran < NSEC_PER_SEC) ?
- current_fs_time(inode->i_sb) : CURRENT_TIME_SEC;
-}
-
static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino)
{
return ino == EXT4_ROOT_INO ||
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 2a2eef9..ac303be 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4722,7 +4722,7 @@ retry:
map.m_lblk += ret;
map.m_len = len = len - ret;
epos = (loff_t)map.m_lblk << inode->i_blkbits;
- inode->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
if (new_size) {
if (epos > new_size)
epos = new_size;
@@ -4850,7 +4850,7 @@ static long ext4_zero_range(struct file *file, loff_t offset,
}
/* Now release the pages and zero block aligned part of pages */
truncate_pagecache_range(inode, start, end - 1);
- inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb);
ret = ext4_alloc_file_blocks(file, lblk, max_blocks, new_size,
flags, mode);
@@ -4875,7 +4875,7 @@ static long ext4_zero_range(struct file *file, loff_t offset,
goto out_dio;
}
- inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb);
if (new_size) {
ext4_update_inode_size(inode, new_size);
} else {
@@ -5574,7 +5574,7 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
up_write(&EXT4_I(inode)->i_data_sem);
if (IS_SYNC(inode))
ext4_handle_sync(handle);
- inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb);
ext4_mark_inode_dirty(handle, inode);
out_stop:
@@ -5684,7 +5684,7 @@ int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len)
/* Expand file to avoid data loss if there is error while shifting */
inode->i_size += len;
EXT4_I(inode)->i_disksize += len;
- inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb);
ret = ext4_mark_inode_dirty(handle, inode);
if (ret)
goto out_stop;
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 3da4cf8..152ef38 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1039,7 +1039,7 @@ got:
/* This is the optimal IO size (for stat), not the fs block size */
inode->i_blocks = 0;
inode->i_mtime = inode->i_atime = inode->i_ctime = ei->i_crtime =
- ext4_current_time(inode);
+ current_fs_time(inode->i_sb);
memset(ei->i_data, 0, sizeof(ei->i_data));
ei->i_dir_start_lookup = 0;
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index ff7538c..67b3fe8 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1028,7 +1028,7 @@ static int ext4_add_dirent_to_inline(handle_t *handle,
* happen is that the times are slightly out of date
* and/or different from the directory change time.
*/
- dir->i_mtime = dir->i_ctime = ext4_current_time(dir);
+ dir->i_mtime = dir->i_ctime = current_fs_time(dir->i_sb);
ext4_update_dx_flag(dir);
dir->i_version++;
ext4_mark_inode_dirty(handle, dir);
@@ -1971,7 +1971,7 @@ out:
if (inode->i_nlink)
ext4_orphan_del(handle, inode);
- inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb);
ext4_mark_inode_dirty(handle, inode);
if (IS_SYNC(inode))
ext4_handle_sync(handle);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index f7140ca..1546c02 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3991,7 +3991,7 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
if (IS_SYNC(inode))
ext4_handle_sync(handle);
- inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb);
ext4_mark_inode_dirty(handle, inode);
out_stop:
ext4_journal_stop(handle);
@@ -4145,7 +4145,7 @@ out_stop:
if (inode->i_nlink)
ext4_orphan_del(handle, inode);
- inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb);
ext4_mark_inode_dirty(handle, inode);
ext4_journal_stop(handle);
@@ -5120,7 +5120,7 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
* update c/mtime in shrink case below
*/
if (!shrink) {
- inode->i_mtime = ext4_current_time(inode);
+ inode->i_mtime = current_fs_time(inode->i_sb);
inode->i_ctime = inode->i_mtime;
}
down_write(&EXT4_I(inode)->i_data_sem);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 28cc412..fb429ac 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -155,7 +155,7 @@ static long swap_inode_boot_loader(struct super_block *sb,
swap_inode_data(inode, inode_bl);
- inode->i_ctime = inode_bl->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = inode_bl->i_ctime = current_fs_time(inode->i_sb);
spin_lock(&sbi->s_next_gen_lock);
inode->i_generation = sbi->s_next_generation++;
@@ -274,7 +274,7 @@ static int ext4_ioctl_setflags(struct inode *inode,
}
ext4_set_inode_flags(inode);
- inode->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
err = ext4_mark_iloc_dirty(handle, inode, &iloc);
flags_err:
@@ -373,7 +373,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
}
}
EXT4_I(inode)->i_projid = kprojid;
- inode->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
out_dirty:
rc = ext4_mark_iloc_dirty(handle, inode, &iloc);
if (!err)
@@ -505,7 +505,7 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
}
err = ext4_reserve_inode_write(handle, inode, &iloc);
if (err == 0) {
- inode->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
inode->i_generation = generation;
err = ext4_mark_iloc_dirty(handle, inode, &iloc);
}
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index ec4c399..c4cc01d 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1939,7 +1939,7 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
* happen is that the times are slightly out of date
* and/or different from the directory change time.
*/
- dir->i_mtime = dir->i_ctime = ext4_current_time(dir);
+ dir->i_mtime = dir->i_ctime = current_fs_time(dir->i_sb);
ext4_update_dx_flag(dir);
dir->i_version++;
ext4_mark_inode_dirty(handle, dir);
@@ -2988,7 +2988,7 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
* recovery. */
inode->i_size = 0;
ext4_orphan_add(handle, inode);
- inode->i_ctime = dir->i_ctime = dir->i_mtime = ext4_current_time(inode);
+ inode->i_ctime = dir->i_ctime = dir->i_mtime = current_fs_time(inode->i_sb);
ext4_mark_inode_dirty(handle, inode);
ext4_dec_count(handle, dir);
ext4_update_dx_flag(dir);
@@ -3051,13 +3051,13 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
retval = ext4_delete_entry(handle, dir, de, bh);
if (retval)
goto end_unlink;
- dir->i_ctime = dir->i_mtime = ext4_current_time(dir);
+ dir->i_ctime = dir->i_mtime = current_fs_time(dir->i_sb);
ext4_update_dx_flag(dir);
ext4_mark_inode_dirty(handle, dir);
drop_nlink(inode);
if (!inode->i_nlink)
ext4_orphan_add(handle, inode);
- inode->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
ext4_mark_inode_dirty(handle, inode);
end_unlink:
@@ -3256,7 +3256,7 @@ retry:
if (IS_DIRSYNC(dir))
ext4_handle_sync(handle);
- inode->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
ext4_inc_count(handle, inode);
ihold(inode);
@@ -3383,7 +3383,7 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
ent->de->file_type = file_type;
ent->dir->i_version++;
ent->dir->i_ctime = ent->dir->i_mtime =
- ext4_current_time(ent->dir);
+ current_fs_time(ent->dir->i_sb);
ext4_mark_inode_dirty(handle, ent->dir);
BUFFER_TRACE(ent->bh, "call ext4_handle_dirty_metadata");
if (!ent->inlined) {
@@ -3654,7 +3654,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
* Like most other Unix systems, set the ctime for inodes on a
* rename.
*/
- old.inode->i_ctime = ext4_current_time(old.inode);
+ old.inode->i_ctime = current_fs_time(old.inode->i_sb);
ext4_mark_inode_dirty(handle, old.inode);
if (!whiteout) {
@@ -3666,9 +3666,9 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
if (new.inode) {
ext4_dec_count(handle, new.inode);
- new.inode->i_ctime = ext4_current_time(new.inode);
+ new.inode->i_ctime = current_fs_time(new.inode->i_sb);
}
- old.dir->i_ctime = old.dir->i_mtime = ext4_current_time(old.dir);
+ old.dir->i_ctime = old.dir->i_mtime = current_fs_time(old.dir->i_sb);
ext4_update_dx_flag(old.dir);
if (old.dir_bh) {
retval = ext4_rename_dir_finish(handle, &old, new.dir->i_ino);
@@ -3726,6 +3726,7 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
};
u8 new_file_type;
int retval;
+ struct timespec ctime;
if ((ext4_encrypted_inode(old_dir) ||
ext4_encrypted_inode(new_dir)) &&
@@ -3828,8 +3829,9 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
* Like most other Unix systems, set the ctime for inodes on a
* rename.
*/
- old.inode->i_ctime = ext4_current_time(old.inode);
- new.inode->i_ctime = ext4_current_time(new.inode);
+ ctime = current_fs_time(old.inode->i_sb);
+ old.inode->i_ctime = ctime;
+ new.inode->i_ctime = ctime;
ext4_mark_inode_dirty(handle, old.inode);
ext4_mark_inode_dirty(handle, new.inode);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3822a5a..c39cb7c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5165,7 +5165,7 @@ static int ext4_quota_off(struct super_block *sb, int type)
handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
if (IS_ERR(handle))
goto out;
- inode->i_mtime = inode->i_ctime = CURRENT_TIME;
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb);
ext4_mark_inode_dirty(handle, inode);
ext4_journal_stop(handle);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index e79bd32..808609c 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1253,7 +1253,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
}
if (!error) {
ext4_xattr_update_super_block(handle, inode->i_sb);
- inode->i_ctime = ext4_current_time(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
if (!value)
ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-06-09 20:50 +0200 |
| Subject | Re: [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps |
| Message-ID | <rIdbH-1L0-17@gated-at.bofh.it> |
| In reply to | #1418086 |
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.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-06-09 21:00 +0200 |
| Subject | Re: [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps |
| Message-ID | <rIdln-1OU-5@gated-at.bofh.it> |
| In reply to | #1418589 |
On Thu, Jun 9, 2016 at 11:45 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> 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.
Actually, there seems to be one exception to that "all existing
users", and that one exception (btrfs transacation time) really seems
to be broken. Exactly because it's *not* setting an inode time, it
shouldn't have used current_fs_time() to begin with, because it is
just setting an internal filesystem timestamp.
So not making the argument inode-related seems to actually encourage
people to misuse this function.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-06-11 00:20 +0200 |
| Subject | Re: [Y2038] [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps |
| Message-ID | <rICWv-2P1-71@gated-at.bofh.it> |
| In reply to | #1418589 |
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 *); ? Arnd
[toc] | [prev] | [next] | [standalone]
| From | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| Date | 2016-06-14 20:00 +0200 |
| Subject | Re: [Y2038] [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps |
| Message-ID | <rK0N3-865-21@gated-at.bofh.it> |
| In reply to | #1419813 |
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. -Deepa
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-06-14 23:00 +0200 |
| Subject | Re: [Y2038] [PATCH 02/21] fs: ext4: Use current_fs_time() for inode timestamps |
| Message-ID | <rK3Bf-1rf-9@gated-at.bofh.it> |
| In reply to | #1422151 |
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
[toc] | [prev] | [next] | [standalone]
| From | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| Date | 2016-06-09 07:20 +0200 |
| Subject | [PATCH 07/21] fs: cifs: Replace CURRENT_TIME by current_fs_time() |
| Message-ID | <rI0xQ-1Vf-5@gated-at.bofh.it> |
| In reply to | #1418084 |
CURRENT_TIME macro is not appropriate for filesystems as it
doesn't use the right granularity for filesystem timestamps.
Use current_fs_time() instead.
This is also in preparation for the patch that transitions
vfs timestamps to use 64 bit time and hence make them
y2038 safe.
CURRENT_TIME macro will be deleted before merging the
aforementioned change.
Change signature of helper cifs_all_info_to_fattr since it
now needs both super_block and cifs_sb_info.
Note: The inode timestamps read from the server are assumed
to have correct granularity and range.
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Steve French <sfrench@samba.org>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
---
fs/cifs/inode.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 514dadb..692c98b 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -320,9 +320,8 @@ cifs_create_dfs_fattr(struct cifs_fattr *fattr, struct super_block *sb)
fattr->cf_mode = S_IFDIR | S_IXUGO | S_IRWXU;
fattr->cf_uid = cifs_sb->mnt_uid;
fattr->cf_gid = cifs_sb->mnt_gid;
- fattr->cf_atime = CURRENT_TIME;
- fattr->cf_ctime = CURRENT_TIME;
- fattr->cf_mtime = CURRENT_TIME;
+ fattr->cf_atime = fattr->cf_ctime =
+ fattr->cf_mtime = current_fs_time(sb);
fattr->cf_nlink = 2;
fattr->cf_flags |= CIFS_FATTR_DFS_REFERRAL;
}
@@ -584,9 +583,10 @@ static int cifs_sfu_mode(struct cifs_fattr *fattr, const unsigned char *path,
/* Fill a cifs_fattr struct with info from FILE_ALL_INFO */
static void
cifs_all_info_to_fattr(struct cifs_fattr *fattr, FILE_ALL_INFO *info,
- struct cifs_sb_info *cifs_sb, bool adjust_tz,
+ struct super_block *sb, bool adjust_tz,
bool symlink)
{
+ struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
memset(fattr, 0, sizeof(*fattr));
@@ -597,7 +597,7 @@ cifs_all_info_to_fattr(struct cifs_fattr *fattr, FILE_ALL_INFO *info,
if (info->LastAccessTime)
fattr->cf_atime = cifs_NTtimeToUnix(info->LastAccessTime);
else
- fattr->cf_atime = CURRENT_TIME;
+ fattr->cf_atime = current_fs_time(sb);
fattr->cf_ctime = cifs_NTtimeToUnix(info->ChangeTime);
fattr->cf_mtime = cifs_NTtimeToUnix(info->LastWriteTime);
@@ -657,7 +657,6 @@ cifs_get_file_info(struct file *filp)
FILE_ALL_INFO find_data;
struct cifs_fattr fattr;
struct inode *inode = file_inode(filp);
- struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
struct cifsFileInfo *cfile = filp->private_data;
struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
struct TCP_Server_Info *server = tcon->ses->server;
@@ -669,7 +668,7 @@ cifs_get_file_info(struct file *filp)
rc = server->ops->query_file_info(xid, tcon, &cfile->fid, &find_data);
switch (rc) {
case 0:
- cifs_all_info_to_fattr(&fattr, &find_data, cifs_sb, false,
+ cifs_all_info_to_fattr(&fattr, &find_data, inode->i_sb, false,
false);
break;
case -EREMOTE:
@@ -751,7 +750,7 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
}
if (!rc) {
- cifs_all_info_to_fattr(&fattr, data, cifs_sb, adjust_tz,
+ cifs_all_info_to_fattr(&fattr, data, sb, adjust_tz,
symlink);
} else if (rc == -EREMOTE) {
cifs_create_dfs_fattr(&fattr, sb);
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2016-06-09 09:40 +0200 |
| Subject | Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() |
| Message-ID | <rI2Jk-3h2-11@gated-at.bofh.it> |
| In reply to | #1418084 |
On Wed 08-06-16 22:04:45, Deepa Dinamani wrote: > CURRENT_TIME_SEC is not y2038 safe. current_fs_time() will > be transitioned to use 64 bit time along with vfs in a > separate patch. > There is no plan to transistion CURRENT_TIME_SEC to use > y2038 safe time interfaces. > > current_fs_time() will also be extended to use superblock > range checking parameters when range checking is introduced. > > This works because alloc_super() fills in the the s_time_gran > in super block to NSEC_PER_SEC. > > Also note that filesystem specific times like the birthtime, > creation time that were using same interfaces to obtain time > retain same logistics. You create line longer than 80 characters for affs and reiserfs. Please wrap those lines properly. Other than that feel free to add: Acked-by: Jan Kara <jack@suse.cz> Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-06-09 21:20 +0200 |
| Subject | Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() |
| Message-ID | <rIdEJ-2bK-11@gated-at.bofh.it> |
| In reply to | #1418130 |
On Thu, Jun 9, 2016 at 12:35 AM, Jan Kara <jack@suse.cz> wrote:
>
> You create line longer than 80 characters for affs and reiserfs. Please
> wrap those lines properly.
No, please do *NOT* do things like that.
These kind of mechanical patches should
(a) be as mechanical as possible (and see elsewhere about why I think
'sb' should be 'inode' and the patch should have been 95% automated
with a trivial script thanks to that change)
(b) be made as easy to verify visually as possible.
That (b) means that a conversion should *not* add whitespace fixups or
add other non-mechanical cleanups, because it's a *lot* easier to see
that a conversion like
- inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
+ inode->i_mtime = inode->i_ctime = current_fs_time(inode);
makes no other changes, but if you start doing line-splitting or other
transformations (add new variables etc to get at 'sb'), suddenly you
have to verify the patch at a completely different level.
In other words, it's actually really important to make these kinds of
bulk changes be very very obvious. Including to the point of making
them visually easier to scan as a patch by not making any other
changes.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| Date | 2016-06-09 22:50 +0200 |
| Subject | Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() |
| Message-ID | <rIf3P-31O-7@gated-at.bofh.it> |
| In reply to | #1418597 |
On Thu, Jun 9, 2016 at 12:15 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Thu, Jun 9, 2016 at 12:35 AM, Jan Kara <jack@suse.cz> wrote: >> >> You create line longer than 80 characters for affs and reiserfs. Please >> wrap those lines properly. > > No, please do *NOT* do things like that. > > These kind of mechanical patches should > > (a) be as mechanical as possible (and see elsewhere about why I think > 'sb' should be 'inode' and the patch should have been 95% automated > with a trivial script thanks to that change) > > (b) be made as easy to verify visually as possible. > > That (b) means that a conversion should *not* add whitespace fixups or > add other non-mechanical cleanups, because it's a *lot* easier to see > that a conversion like > > - inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC; > + inode->i_mtime = inode->i_ctime = current_fs_time(inode); > > makes no other changes, but if you start doing line-splitting or other > transformations (add new variables etc to get at 'sb'), suddenly you > have to verify the patch at a completely different level. > > In other words, it's actually really important to make these kinds of > bulk changes be very very obvious. Including to the point of making > them visually easier to scan as a patch by not making any other > changes. Thanks for the guidelines. Only patches 1 and 4 are mechanical. All others need some kind of inspection/ verification. I will keep these in mind for updating 1 and 4. -Deepa
[toc] | [prev] | [next] | [standalone]
| From | Felipe Balbi <balbi@kernel.org> |
|---|---|
| Date | 2016-06-09 10:00 +0200 |
| Message-ID | <rI32G-3oT-7@gated-at.bofh.it> |
| In reply to | #1418084 |
[Multipart message — attachments visible in raw view] — view raw
Hi, Deepa Dinamani <deepa.kernel@gmail.com> writes: > drivers/usb/gadget/function/f_fs.c | 2 +- > drivers/usb/gadget/legacy/inode.c | 2 +- for drivers/usb/gadget: Acked-by: Felipe Balbi <balbi@kernel.org> -- balbi
[toc] | [prev] | [next] | [standalone]
| From | Bob Copeland <me@bobcopeland.com> |
|---|---|
| Date | 2016-06-09 14:40 +0200 |
| Subject | Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() |
| Message-ID | <rI7pE-6lh-3@gated-at.bofh.it> |
| In reply to | #1418084 |
On Wed, Jun 08, 2016 at 10:04:45PM -0700, Deepa Dinamani wrote: > CURRENT_TIME_SEC is not y2038 safe. current_fs_time() will > be transitioned to use 64 bit time along with vfs in a > separate patch. > There is no plan to transistion CURRENT_TIME_SEC to use > y2038 safe time interfaces. [...] > Cc: Bob Copeland <me@bobcopeland.com> OMFS parts look sane, thanks. -- Bob Copeland %% http://bobcopeland.com/
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-06-11 00:30 +0200 |
| Subject | Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() |
| Message-ID | <rID6a-2Ul-11@gated-at.bofh.it> |
| In reply to | #1418084 |
On Wednesday, June 8, 2016 10:04:45 PM CEST Deepa Dinamani wrote: > CURRENT_TIME_SEC is not y2038 safe. current_fs_time() will > be transitioned to use 64 bit time along with vfs in a > separate patch. > There is no plan to transistion CURRENT_TIME_SEC to use > y2038 safe time interfaces. > > current_fs_time() will also be extended to use superblock > range checking parameters when range checking is introduced. > > This works because alloc_super() fills in the the s_time_gran > in super block to NSEC_PER_SEC. > > Also note that filesystem specific times like the birthtime, > creation time that were using same interfaces to obtain time > retain same logistics. > > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com> one question: In an earlier version, you had a small optimization to use ktime_get_real_seconds() instead of current_kernel_time() when the granularity is seconds. Do you still plan to send that one, or did you decide we don't need it? Arnd
[toc] | [prev] | [next] | [standalone]
| From | Deepa Dinamani <deepa.kernel@gmail.com> |
|---|---|
| Date | 2016-06-11 07:10 +0200 |
| Subject | Re: [PATCH 01/21] fs: Replace CURRENT_TIME_SEC with current_fs_time() |
| Message-ID | <rIJlf-7fN-1@gated-at.bofh.it> |
| In reply to | #1419817 |
On Fri, Jun 10, 2016 at 3:21 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday, June 8, 2016 10:04:45 PM CEST Deepa Dinamani wrote: >> CURRENT_TIME_SEC is not y2038 safe. current_fs_time() will >> be transitioned to use 64 bit time along with vfs in a >> separate patch. >> There is no plan to transistion CURRENT_TIME_SEC to use >> y2038 safe time interfaces. >> >> current_fs_time() will also be extended to use superblock >> range checking parameters when range checking is introduced. >> >> This works because alloc_super() fills in the the s_time_gran >> in super block to NSEC_PER_SEC. >> >> Also note that filesystem specific times like the birthtime, >> creation time that were using same interfaces to obtain time >> retain same logistics. >> >> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com> > > one question: > > In an earlier version, you had a small optimization to > use ktime_get_real_seconds() instead of current_kernel_time() > when the granularity is seconds. > > Do you still plan to send that one, or did you decide we don't > need it? I was actually planning to use get_seconds() instead of current_kernel_time(). And, transition both along with vfs to y2038 safe apis. Difference between ktime_get_real_seconds() and current_kernel_time64() is not much because they both require sequence counter. It didn't make sense to me to optimize current_fs_time() for seconds only, and not optimize for 1ns granularity also. I plan to make changes to the function depending on how we end up using timespec_trunc() after the addition of range checking. Thanks for the guidance on inclusion of reviewers. I'll follow this approach when I post v2 of the series. -Deepa
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web