CEPH filesystem development
 help / color / mirror / Atom feed
* [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
@ 2016-06-20  0:26 Deepa Dinamani
  2016-06-20  0:27 ` [PATCH v2 20/24] block: Replace CURRENT_TIME with ktime_get_real_ts Deepa Dinamani
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Deepa Dinamani @ 2016-06-20  0:26 UTC (permalink / raw)
  To: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: shaggy-DgEjT+Ai2ygdnm+yROfE0A,
	jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA, clm-b10kYP2dOMg,
	adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q,
	buchino-FYB4Gu1CFyUAvxtiuMwx3w, tglx-hfZtesqFncYOwBW4kG4KsQ,
	zyan-H+wXaHxf7aLQT0dZR+AlfA,
	jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	paul-r2n+y4ga6xFZroRs9YW3xA, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	y2038-cunTk1MwBs8s++Sfvej+rw, idryomov-Re5JQEeQqe8AvxtiuMwx3w,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA,
	cm224.lee-Sze3O3UU22JBDgjK7y7TUQ, arnd-r2nGTMty4D4,
	mfasheh-IBi9RG/b67k, sramars-FYB4Gu1CFyUAvxtiuMwx3w,
	john.stultz-QSEj5FYQhm4dnm+yROfE0A,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, dsterba-IBi9RG/b67k,
	jaegeuk-DgEjT+Ai2ygdnm+yROfE0A, ceph-devel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, elder-DgEjT+Ai2ygdnm+yROfE0A,
	tytso-3s7WtUTddSA, sage-H+wXaHxf7aLQT0dZR+AlfA,
	martin.petersen-QHcLZuEGTsvQT0dZR+AlfA,
	dedekind1-Re5JQEeQqe8AvxtiuMwx3w, jbacik-b10kYP2dOMg,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	hiralpat-FYB4Gu1CFyUAvxtiuMwx3w,
	adrian.hunter-ral2JQCrhuEAvxtiuMwx3w,
	eparis-H+wXaHxf7aLQT0dZR+AlfA,
	linux-f2fs-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	sfrench-eUNUBHrolfbYtjvyW6yDsg,
	linux-audit-H+wXaHxf7aLQT0dZR+AlfA,
	ocfs2-devel-N0ozoZBvEnrZJqsBc5GL+g, jack-IBi9RG/b67k

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.

Thanks to Arnd Bergmann for all the guidance and discussions.

Patches 2-4 were mostly generated using coccinelle scripts.

All filesystem timestamps use current_fs_time() for right granularity as
mentioned in the respective commit texts of patches. This has a changed
signature, renamed to current_time() and moved to the fs/inode.c.

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_time() now is used as a single generic vfs filesystem timestamp api.
It also takes struct inode* as argument instead of struct super_block*.
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.

Changes from v1:
* Change current_fs_time(struct super_block *) to current_time(struct inode *)

* Note that change to add time64_to_tm() is already part of John's
  kernel tree: https://lkml.org/lkml/2016/6/17/875 .

Deepa Dinamani (24):
  vfs: Add current_time() api
  fs: Replace CURRENT_TIME with current_time() for inode timestamps
  fs: Replace CURRENT_TIME_SEC with current_time() for inode timestamps
  fs: Replace current_fs_time() with current_time()
  fs: jfs: Replace CURRENT_TIME_SEC by current_time()
  fs: ext4: Use current_time() for inode timestamps
  fs: ubifs: Replace CURRENT_TIME_SEC with current_time
  fs: btrfs: Use ktime_get_real_ts for root ctime
  fs: udf: Replace CURRENT_TIME with current_time()
  fs: cifs: Replace CURRENT_TIME by current_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_time()
  fs: ocfs2: Use time64_t to represent orphan scan times
  fs: ocfs2: Replace CURRENT_TIME with ktime_get_real_seconds()
  audit: Use timespec64 to represent audit timestamps
  fs: nfs: Make nfs boot time y2038 safe
  fnic: Use time64_t to represent trace timestamps
  block: Replace CURRENT_TIME with ktime_get_real_ts
  libceph: Replace CURRENT_TIME with ktime_get_real_ts
  fs: ceph: Replace current_fs_time for request stamp
  time: Delete CURRENT_TIME_SEC and CURRENT_TIME macro
  time: Delete current_fs_time() function

 arch/powerpc/platforms/cell/spufs/inode.c          |  2 +-
 arch/s390/hypfs/inode.c                            |  4 ++--
 drivers/block/rbd.c                                |  2 +-
 drivers/char/sonypi.c                              |  2 +-
 drivers/infiniband/hw/qib/qib_fs.c                 |  2 +-
 drivers/misc/ibmasm/ibmasmfs.c                     |  2 +-
 drivers/oprofile/oprofilefs.c                      |  2 +-
 drivers/platform/x86/sony-laptop.c                 |  2 +-
 drivers/scsi/fnic/fnic_trace.c                     |  4 ++--
 drivers/scsi/fnic/fnic_trace.h                     |  2 +-
 drivers/staging/lustre/lustre/llite/llite_lib.c    | 16 ++++++-------
 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                           | 18 +++++++-------
 drivers/usb/gadget/function/f_fs.c                 |  8 +++----
 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/attr.c                                          |  2 +-
 fs/autofs4/inode.c                                 |  2 +-
 fs/autofs4/root.c                                  |  6 ++---
 fs/bad_inode.c                                     |  2 +-
 fs/bfs/dir.c                                       | 14 +++++------
 fs/binfmt_misc.c                                   |  2 +-
 fs/btrfs/file.c                                    |  6 ++---
 fs/btrfs/inode.c                                   | 22 ++++++++---------
 fs/btrfs/ioctl.c                                   |  8 +++----
 fs/btrfs/root-tree.c                               |  3 ++-
 fs/btrfs/transaction.c                             |  4 ++--
 fs/btrfs/xattr.c                                   |  2 +-
 fs/ceph/file.c                                     |  4 ++--
 fs/ceph/inode.c                                    |  2 +-
 fs/ceph/mds_client.c                               |  4 +++-
 fs/ceph/xattr.c                                    |  2 +-
 fs/cifs/cifsencrypt.c                              |  4 +++-
 fs/cifs/cifssmb.c                                  | 10 ++++----
 fs/cifs/file.c                                     |  4 ++--
 fs/cifs/inode.c                                    | 28 ++++++++++++----------
 fs/coda/dir.c                                      |  2 +-
 fs/coda/file.c                                     |  2 +-
 fs/coda/inode.c                                    |  2 +-
 fs/configfs/inode.c                                |  6 ++---
 fs/debugfs/inode.c                                 |  2 +-
 fs/devpts/inode.c                                  |  6 ++---
 fs/efivarfs/inode.c                                |  2 +-
 fs/exofs/dir.c                                     |  6 ++---
 fs/exofs/inode.c                                   |  4 ++--
 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                                    |  4 ++--
 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                                      |  6 ++---
 fs/fat/inode.c                                     |  2 +-
 fs/fat/namei_msdos.c                               | 12 +++++-----
 fs/fat/namei_vfat.c                                | 10 ++++----
 fs/fuse/control.c                                  |  2 +-
 fs/fuse/dir.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/inode.c                                         | 21 +++++++++++++---
 fs/jffs2/acl.c                                     |  2 +-
 fs/jffs2/fs.c                                      |  2 +-
 fs/jfs/acl.c                                       |  2 +-
 fs/jfs/inode.c                                     |  2 +-
 fs/jfs/ioctl.c                                     |  2 +-
 fs/jfs/jfs_inode.c                                 |  2 +-
 fs/jfs/namei.c                                     | 24 +++++++++----------
 fs/jfs/super.c                                     |  2 +-
 fs/jfs/xattr.c                                     |  2 +-
 fs/kernfs/inode.c                                  |  2 +-
 fs/libfs.c                                         | 14 +++++------
 fs/locks.c                                         |  2 +-
 fs/logfs/dir.c                                     |  6 ++---
 fs/logfs/file.c                                    |  2 +-
 fs/logfs/inode.c                                   |  4 ++--
 fs/logfs/readwrite.c                               |  4 ++--
 fs/minix/bitmap.c                                  |  2 +-
 fs/minix/dir.c                                     |  6 ++---
 fs/minix/itree_common.c                            |  4 ++--
 fs/minix/namei.c                                   |  4 ++--
 fs/nfs/client.c                                    |  2 +-
 fs/nfs/netns.h                                     |  2 +-
 fs/nfs/nfs4proc.c                                  | 10 ++++----
 fs/nfs/nfs4xdr.c                                   |  2 +-
 fs/nfsd/blocklayout.c                              |  2 +-
 fs/nilfs2/dir.c                                    |  6 ++---
 fs/nilfs2/inode.c                                  |  4 ++--
 fs/nilfs2/ioctl.c                                  |  2 +-
 fs/nilfs2/namei.c                                  |  6 ++---
 fs/nsfs.c                                          |  2 +-
 fs/ntfs/inode.c                                    |  2 +-
 fs/ntfs/mft.c                                      |  2 +-
 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                                   | 16 +++++++------
 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                                | 10 ++++----
 fs/pipe.c                                          |  2 +-
 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                                   |  6 ++---
 fs/reiserfs/inode.c                                |  2 +-
 fs/reiserfs/ioctl.c                                |  4 ++--
 fs/reiserfs/namei.c                                | 12 +++++-----
 fs/reiserfs/stree.c                                |  8 +++----
 fs/reiserfs/super.c                                |  2 +-
 fs/reiserfs/xattr.c                                |  6 ++---
 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                                      | 14 +++++++----
 fs/ubifs/xattr.c                                   |  6 ++---
 fs/udf/ialloc.c                                    |  2 +-
 fs/udf/inode.c                                     |  4 ++--
 fs/udf/namei.c                                     | 20 ++++++++--------
 fs/udf/super.c                                     |  9 +++++--
 fs/ufs/dir.c                                       |  6 ++---
 fs/ufs/ialloc.c                                    |  8 ++++---
 fs/ufs/inode.c                                     |  6 ++---
 fs/ufs/namei.c                                     |  6 ++---
 fs/xfs/xfs_acl.c                                   |  2 +-
 fs/xfs/xfs_inode.c                                 |  2 +-
 fs/xfs/xfs_iops.c                                  |  2 +-
 fs/xfs/xfs_trans_inode.c                           |  2 +-
 include/linux/audit.h                              |  4 ++--
 include/linux/fs.h                                 |  2 +-
 include/linux/time.h                               |  3 ---
 ipc/mqueue.c                                       | 18 +++++++-------
 kernel/audit.c                                     | 10 ++++----
 kernel/audit.h                                     |  2 +-
 kernel/auditsc.c                                   |  6 ++---
 kernel/bpf/inode.c                                 |  2 +-
 kernel/time/time.c                                 | 14 -----------
 mm/shmem.c                                         | 20 ++++++++--------
 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 +-
 206 files changed, 533 insertions(+), 522 deletions(-)

-- 
1.9.1

Cc: adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org
Cc: adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Cc: anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
Cc: buchino-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org
Cc: ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: clm-b10kYP2dOMg@public.gmane.org
Cc: cm224.lee-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org
Cc: dedekind1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: dsterba-IBi9RG/b67k@public.gmane.org
Cc: elder-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org
Cc: hiralpat-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org
Cc: idryomov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: jack-IBi9RG/b67k@public.gmane.org
Cc: jaegeuk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: jbacik-b10kYP2dOMg@public.gmane.org
Cc: jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Cc: jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc: jlbec-aKy9MeLSZ9dg9hUCZPvPmw@public.gmane.org
Cc: john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
Cc: linux-audit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: linux-btrfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-f2fs-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org
Cc: martin.petersen-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org
Cc: mfasheh-IBi9RG/b67k@public.gmane.org
Cc: ocfs2-devel-N0ozoZBvEnrZJqsBc5GL+g@public.gmane.org
Cc: paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org
Cc: sage-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org
Cc: shaggy-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: sramars-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org
Cc: trond.myklebust-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org
Cc: zyan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 20/24] block: Replace CURRENT_TIME with ktime_get_real_ts
  2016-06-20  0:26 [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Deepa Dinamani
@ 2016-06-20  0:27 ` Deepa Dinamani
  2016-06-20  0:27 ` [PATCH v2 21/24] libceph: " Deepa Dinamani
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Deepa Dinamani @ 2016-06-20  0:27 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel
  Cc: Alex Elder, tytso, Sage Weil, arnd, y2038, ceph-devel, viro, tglx,
	torvalds, Ilya Dryomov

CURRENT_TIME is not y2038 safe.
The macro will be deleted and all the references to it
will be replaced by ktime_get_* apis.

struct timespec is also not y2038 safe.
Retain timespec for timestamp representation here as ceph
uses it internally everywhere.
These references will be changed to use struct timespec64
in a separate patch.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: Sage Weil <sage@redhat.com>
Cc: Alex Elder <elder@kernel.org>
Cc: ceph-devel@vger.kernel.org
---
 drivers/block/rbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 4506620..14867ab 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1888,7 +1888,7 @@ static void rbd_osd_req_format_write(struct rbd_obj_request *obj_request)
 {
 	struct ceph_osd_request *osd_req = obj_request->osd_req;
 
-	osd_req->r_mtime = CURRENT_TIME;
+	ktime_get_real_ts(&osd_req->r_mtime);
 	osd_req->r_data_offset = obj_request->offset;
 }
 
-- 
1.9.1

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 21/24] libceph: Replace CURRENT_TIME with ktime_get_real_ts
  2016-06-20  0:26 [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Deepa Dinamani
  2016-06-20  0:27 ` [PATCH v2 20/24] block: Replace CURRENT_TIME with ktime_get_real_ts Deepa Dinamani
@ 2016-06-20  0:27 ` Deepa Dinamani
  2016-06-20  8:33   ` Willem Jan Withagen
  2016-06-20  0:27 ` [PATCH v2 22/24] fs: ceph: Replace current_fs_time for request stamp Deepa Dinamani
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Deepa Dinamani @ 2016-06-20  0:27 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel
  Cc: tytso, Sage Weil, arnd, y2038, ceph-devel, Yan, Zheng, viro, tglx,
	torvalds, Ilya Dryomov

CURRENT_TIME is not y2038 safe.
The macro will be deleted and all the references to it
will be replaced by ktime_get_* apis.

struct timespec is also not y2038 safe.
Retain timespec for timestamp representation here as ceph
uses it internally everywhere.
These references will be changed to use struct timespec64
in a separate patch.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: "Yan, Zheng" <zyan@redhat.com>
Cc: Sage Weil <sage@redhat.com>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
---
 net/ceph/messenger.c  | 6 ++++--
 net/ceph/osd_client.c | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index a550289..1825eed 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -1366,8 +1366,9 @@ static void prepare_write_keepalive(struct ceph_connection *con)
 	dout("prepare_write_keepalive %p\n", con);
 	con_out_kvec_reset(con);
 	if (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2) {
-		struct timespec now = CURRENT_TIME;
+		struct timespec now;
 
+		ktime_get_real_ts(&now);
 		con_out_kvec_add(con, sizeof(tag_keepalive2), &tag_keepalive2);
 		ceph_encode_timespec(&con->out_temp_keepalive2, &now);
 		con_out_kvec_add(con, sizeof(con->out_temp_keepalive2),
@@ -3149,8 +3150,9 @@ bool ceph_con_keepalive_expired(struct ceph_connection *con,
 {
 	if (interval > 0 &&
 	    (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2)) {
-		struct timespec now = CURRENT_TIME;
+		struct timespec now;
 		struct timespec ts;
+		ktime_get_real_ts(&now);
 		jiffies_to_timespec(interval, &ts);
 		ts = timespec_add(con->last_keepalive_ack, ts);
 		return timespec_compare(&now, &ts) >= 0;
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index 8946959..44eb2d0 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -3567,7 +3567,7 @@ ceph_osdc_watch(struct ceph_osd_client *osdc,
 	ceph_oid_copy(&lreq->t.base_oid, oid);
 	ceph_oloc_copy(&lreq->t.base_oloc, oloc);
 	lreq->t.flags = CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK;
-	lreq->mtime = CURRENT_TIME;
+	ktime_get_real_ts(&lreq->mtime);
 
 	lreq->reg_req = alloc_linger_request(lreq);
 	if (!lreq->reg_req) {
@@ -3625,7 +3625,7 @@ int ceph_osdc_unwatch(struct ceph_osd_client *osdc,
 	ceph_oid_copy(&req->r_base_oid, &lreq->t.base_oid);
 	ceph_oloc_copy(&req->r_base_oloc, &lreq->t.base_oloc);
 	req->r_flags = CEPH_OSD_FLAG_WRITE | CEPH_OSD_FLAG_ONDISK;
-	req->r_mtime = CURRENT_TIME;
+	ktime_get_real_ts(&req->r_mtime);
 	osd_req_op_watch_init(req, 0, lreq->linger_id,
 			      CEPH_OSD_WATCH_OP_UNWATCH);
 
-- 
1.9.1

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 22/24] fs: ceph: Replace current_fs_time for request stamp
  2016-06-20  0:26 [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Deepa Dinamani
  2016-06-20  0:27 ` [PATCH v2 20/24] block: Replace CURRENT_TIME with ktime_get_real_ts Deepa Dinamani
  2016-06-20  0:27 ` [PATCH v2 21/24] libceph: " Deepa Dinamani
@ 2016-06-20  0:27 ` Deepa Dinamani
       [not found] ` <1466382443-11063-1-git-send-email-deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2016-06-22 15:49 ` Arnd Bergmann
  4 siblings, 0 replies; 13+ messages in thread
From: Deepa Dinamani @ 2016-06-20  0:27 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel
  Cc: tytso, Sage Weil, arnd, y2038, ceph-devel, Yan, Zheng, viro, tglx,
	torvalds, Ilya Dryomov

The current_fs_time() api is being changed to use vfs
struct inode* as an argument instead of struct super_block*.

Set the new mds client request r_stamp field using
ktime_get_real_ts() instead of using current_fs_time().

Also, since r_stamp is used as mtime on the server, use
timespec_trunc() to truncate the timestamp, using the right
granularity from the superblock.

This api will be transitioned to be y2038 safe along
with vfs.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: "Yan, Zheng" <zyan@redhat.com>
Cc: Sage Weil <sage@redhat.com>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: ceph-devel@vger.kernel.org
---
 fs/ceph/mds_client.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 2103b82..c6cae7a 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1697,6 +1697,7 @@ struct ceph_mds_request *
 ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
 {
 	struct ceph_mds_request *req = kzalloc(sizeof(*req), GFP_NOFS);
+	struct timespec ts;
 
 	if (!req)
 		return ERR_PTR(-ENOMEM);
@@ -1715,7 +1716,8 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
 	init_completion(&req->r_safe_completion);
 	INIT_LIST_HEAD(&req->r_unsafe_item);
 
-	req->r_stamp = current_fs_time(mdsc->fsc->sb);
+	ktime_get_real_ts(&ts);
+	req->r_stamp = timespec_trunc(ts, mdsc->fsc->sb->s_time_gran);
 
 	req->r_op = op;
 	req->r_direct_mode = mode;
-- 
1.9.1

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 21/24] libceph: Replace CURRENT_TIME with ktime_get_real_ts
  2016-06-20  0:27 ` [PATCH v2 21/24] libceph: " Deepa Dinamani
@ 2016-06-20  8:33   ` Willem Jan Withagen
  2016-06-22  0:56     ` Deepa Dinamani
  0 siblings, 1 reply; 13+ messages in thread
From: Willem Jan Withagen @ 2016-06-20  8:33 UTC (permalink / raw)
  To: Deepa Dinamani; +Cc: ceph-devel

On 20-6-2016 02:27, Deepa Dinamani wrote:
> CURRENT_TIME is not y2038 safe.
> The macro will be deleted and all the references to it
> will be replaced by ktime_get_* apis.
> 
> struct timespec is also not y2038 safe.
> Retain timespec for timestamp representation here as ceph
> uses it internally everywhere.
> These references will be changed to use struct timespec64
> in a separate patch.
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: "Yan, Zheng" <zyan@redhat.com>
> Cc: Sage Weil <sage@redhat.com>
> Cc: Ilya Dryomov <idryomov@gmail.com>
> Cc: ceph-devel@vger.kernel.org
> ---
>  net/ceph/messenger.c  | 6 ++++--
>  net/ceph/osd_client.c | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)

Hi Deepa,

Although I applaud 64-bit timestamps,
the ktime_get_* api seems to be a Linux only API.

So IF ever things need to be the least bit portable, I suggest wrapping
these routines into a class of itself. So that porting confines itself
to only that class. (plus/minus hairy semantic details)

For what is now committed the change is not really that big.

On the other hand, I 'll have to start figuring out if FreeBSD really
does do 64bit timestamps...

So I would welcome a more elaborate description of the problem (it seems
that Linux is going 64bit in its inodes??) and what is being fixed and
how. So that in the porting case not only the code will be the source to
figure out what is going on.

Thanx,
--WjW



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
       [not found] ` <1466382443-11063-1-git-send-email-deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-06-20 18:03   ` Linus Torvalds
       [not found]     ` <CA+55aFzK2K-7UeQRsWR7ooNHMMbtMFRAF60byR1Gvmbf9XWhbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2016-06-20 18:03 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Dave Kleikamp, jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Trond Myklebust, Adrian Hunter, Chris Mason,
	adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org,
	buchino-FYB4Gu1CFyUAvxtiuMwx3w, Thomas Gleixner, Yan, Zheng,
	jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Paul Moore,
	Linux SCSI List, y2038-cunTk1MwBs8s++Sfvej+rw, Ilya Dryomov,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Changman Lee,
	Arnd Bergmann, Mark Fasheh, sramars-FYB4Gu1CFyUAvxtiuMwx3w,
	John Stultz, Al Viro, David Sterba, Jaegeuk Kim, ceph-devel

On Sun, Jun 19, 2016 at 5:26 PM, Deepa Dinamani <deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC macros.

This version now looks ok to me.

I do have a comment (or maybe just a RFD) for future work.

It does strike me that once we actually change over the inode times to
use timespec64, the calling conventions are going to be fairly
horrendous on most 32-bit architectures.

Gcc handles 8-byte structure returns (on most architectures) by
returning them as two 32-bit registers (%edx:%eax on x86). But once it
is timespec64, that will no longer be the case, and the calling
convention will end up using a pointer to the local stack instead.

So for 32-bit code generation, we *may* want to introduce a new model of doing

    set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);

which basically just does

    inode->i_atime = inode->i_mtime = current_time(inode);

but with a much easier calling convention on 32-bit architectures.

But that is entirely orthogonal to this patch-set, and should be seen
as a separate issue.

And maybe it doesn't end up helping anyway, but I do think those
"simple" direct assignments will really generate pretty disgusting
code on 32-bit architectures.

That whole

    inode->i_atime = inode->i_mtime = CURRENT_TIME;

model really made a lot more sense back in the ancient days when inode
times were just simply 32-bit seconds (not even timeval structures).

                  Linus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
       [not found]     ` <CA+55aFzK2K-7UeQRsWR7ooNHMMbtMFRAF60byR1Gvmbf9XWhbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-06-20 18:58       ` Deepa Dinamani
  2016-06-21 15:00       ` [Y2038] " Arnd Bergmann
  1 sibling, 0 replies; 13+ messages in thread
From: Deepa Dinamani @ 2016-06-20 18:58 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Dave Kleikamp, jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Trond Myklebust, Adrian Hunter, Chris Mason,
	adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org,
	Brian Uchino, Thomas Gleixner, Yan, Zheng, James E.J. Bottomley,
	Paul Moore, Linux SCSI List, y2038 Mailman List, Ilya Dryomov,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Changman Lee,
	Arnd Bergmann, Mark Fasheh, Suma Ramars, John Stultz, Al Viro,
	David Sterba, Jaegeuk Kim <jaegeuk@

> This version now looks ok to me.
>
> I do have a comment (or maybe just a RFD) for future work.
>
> It does strike me that once we actually change over the inode times to
> use timespec64, the calling conventions are going to be fairly
> horrendous on most 32-bit architectures.
>
> Gcc handles 8-byte structure returns (on most architectures) by
> returning them as two 32-bit registers (%edx:%eax on x86). But once it
> is timespec64, that will no longer be the case, and the calling
> convention will end up using a pointer to the local stack instead.
>
> So for 32-bit code generation, we *may* want to introduce a new model of doing
>
>     set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);
>
> which basically just does
>
>     inode->i_atime = inode->i_mtime = current_time(inode);
>
> but with a much easier calling convention on 32-bit architectures.

Arnd and I had discussed something like this before.
But, for entirely different reasons:

Having the set_inode_time() like you suggest will also help switching
of vfs inode times to timespec64.
We were suggesting all the accesses to inode time be abstracted
through something like inode_set_time().
Arnd also had suggested a split representation of fields in the struct
inode as well which led to space savings
as well. And, having the split representation also meant no more
direct assignments:

https://lkml.org/lkml/2016/1/7/20

This in general will be similar to setattr_copy(), but only sets times
rather than other attributes as well.

If this is what is preferred, then the patches to change vfs to use
timespec64 could make use of this and will
need to be refactored. So maybe it would be good to discuss before I
post those patches.

-Deepa

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Y2038] [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
       [not found]     ` <CA+55aFzK2K-7UeQRsWR7ooNHMMbtMFRAF60byR1Gvmbf9XWhbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2016-06-20 18:58       ` Deepa Dinamani
@ 2016-06-21 15:00       ` Arnd Bergmann
  1 sibling, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2016-06-21 15:00 UTC (permalink / raw)
  To: y2038-cunTk1MwBs8s++Sfvej+rw
  Cc: Linus Torvalds, Deepa Dinamani, Dave Kleikamp,
	jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Trond Myklebust,
	Adrian Hunter, Chris Mason,
	adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org,
	buchino-FYB4Gu1CFyUAvxtiuMwx3w, Thomas Gleixner, Yan, Zheng,
	jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Paul Moore,
	Linux SCSI List, Ilya Dryomov,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Changman Lee,
	Mark Fasheh, sramars-FYB4Gu1CFyUAvxtiuMwx3w, John Stultz, Al Viro,
	David Sterba, Jaegeuk Kim, ceph-devel

On Monday, June 20, 2016 11:03:01 AM CEST you wrote:
> On Sun, Jun 19, 2016 at 5:26 PM, Deepa Dinamani <deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC macros.

> Gcc handles 8-byte structure returns (on most architectures) by
> returning them as two 32-bit registers (%edx:%eax on x86). But once it
> is timespec64, that will no longer be the case, and the calling
> convention will end up using a pointer to the local stack instead.

I guess we already have that today, as the implementation of
current_fs_time() is

static inline struct timespec64 tk_xtime(struct timekeeper *tk)
{
        struct timespec64 ts;

        ts.tv_sec = tk->xtime_sec;
        ts.tv_nsec = (long)(tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift);
        return ts;
}
extern struct timespec64 current_kernel_time64(void);
struct timespec64 current_kernel_time64(void)
{
        struct timekeeper *tk = &tk_core.timekeeper;
        struct timespec64 now;
        unsigned long seq;

        do {
                seq = read_seqcount_begin(&tk_core.seq);

                now = tk_xtime(tk);
        } while (read_seqcount_retry(&tk_core.seq, seq));

        return now;
}
static inline struct timespec current_kernel_time(void)
{
        struct timespec64 now = current_kernel_time64();

        return timespec64_to_timespec(now);
}
extern struct timespec current_fs_time(struct super_block *sb);
struct timespec current_fs_time(struct super_block *sb)
{       
        struct timespec now = current_kernel_time();
        return timespec_trunc(now, sb->s_time_gran);
}       

We can surely do a little better than this, independent of the
conversion in Deepa's patch set.

> So for 32-bit code generation, we *may* want to introduce a new model of doing
> 
>     set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);
> 
> which basically just does
> 
>     inode->i_atime = inode->i_mtime = current_time(inode);
> 
> but with a much easier calling convention on 32-bit architectures.
> 
> But that is entirely orthogonal to this patch-set, and should be seen
> as a separate issue.

I've played around with that, but found it hard to avoid going
through memory other than going all the way to the tk_xtime()
access to copy both tk->xtime_sec and the nanoseconds into
the inode fields.

Without that, the set_inode_time() implementation ends up
being more expensive than
inode->i_atime = inode->i_ctime = inode->i_mtime = current_time(inode);
because we still copy through the stack but also have
a couple of conditional branches that we don't have at the
moment.

At the moment, the triple assignment becomes (here on ARM)

   c:   4668            mov     r0, sp
  12:   f7ff fffe       bl      0 <current_kernel_time64>
  3e:   f107 0520       add.w   r5, r7, #32
                        12: R_ARM_THM_CALL      current_kernel_time64
  16:   f106 0410       add.w   r4, r6, #16
  1a:   e89d 000f       ldmia.w sp, {r0, r1, r2, r3} # load from stack
  1e:   e885 000f       stmia.w r5, {r0, r1, r2, r3} # store into i_atime
  22:   e884 000f       stmia.w r4, {r0, r1, r2, r3} #            i_ctime
  26:   e886 000f       stmia.w r6, {r0, r1, r2, r3} #            i_mtime

and a slightly more verbose version of the same thing on x86
(storing only 12 bytes instead of 16 is cheaper there, while
ARM does a store-multiple to copy the entire structure).

        Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 21/24] libceph: Replace CURRENT_TIME with ktime_get_real_ts
  2016-06-20  8:33   ` Willem Jan Withagen
@ 2016-06-22  0:56     ` Deepa Dinamani
  2016-06-22  8:20       ` Willem Jan Withagen
  0 siblings, 1 reply; 13+ messages in thread
From: Deepa Dinamani @ 2016-06-22  0:56 UTC (permalink / raw)
  To: Willem Jan Withagen; +Cc: ceph-devel, Arnd Bergmann

Adding Arnd, as he is the overall y2038 project lead.

>> CURRENT_TIME is not y2038 safe.
>> The macro will be deleted and all the references to it
>> will be replaced by ktime_get_* apis.
>>
>> struct timespec is also not y2038 safe.
>> Retain timespec for timestamp representation here as ceph
>> uses it internally everywhere.
>> These references will be changed to use struct timespec64
>> in a separate patch.
> Hi Deepa,
>
> Although I applaud 64-bit timestamps,
> the ktime_get_* api seems to be a Linux only API.
> So IF ever things need to be the least bit portable, I suggest wrapping
> these routines into a class of itself. So that porting confines itself
> to only that class. (plus/minus hairy semantic details)

ktime_get_* apis just return 64 bit timestamps.
They are the equivalent of CURRENT_TIME.
Did you have a wrapper for CURRENT_TIME?

Are these concerns about all of ceph kernel code or just that the libceph
is expected to be portable?

> For what is now committed the change is not really that big.
>
> On the other hand, I 'll have to start figuring out if FreeBSD really
> does do 64bit timestamps...

From what I can see here:
http://fxr.watson.org/fxr/ident?im=10;i=__time_t,

FreeBSD time_t is already 64 bit long.
So it supports 64 bit timestamps already.

> So I would welcome a more elaborate description of the problem (it seems
> that Linux is going 64bit in its inodes??) and what is being fixed and
> how. So that in the porting case not only the code will be the source to
> figure out what is going on.

Yes, all inode timestamps are going to be switched over to use timespec64 i.e.,
64 bit timestamps.
We will switch over just the vfs layer: struct inode, struct attr and
struct kstat.
End filesystems such as ceph will still use timespec at this time.
So whenever they access vfs timestamps, they will go through conversion apis:
timespec64_to_timespec() / timespec_to_timespec64().

My reference tree is https://github.com/deepa-hub/vfs/tree/y2038 .

This gives you an overall idea of the changes.
Only a few names have to be updated according to review discussions.
But, you can see what the end filesystems will look like after the vfs
switch-over.

The problem with ceph is really that the timestamps sent over wire use the same
encode/ decode function calls whether the timestamp is related to files or not.
I'm not sure what the plan for wire protocol is here. As Sage
suggested on the thread:
http://www.spinics.net/lists/ceph-devel/msg28960.html , you could
interpret time values as u32.

After this, porting to BSD should be straight forward.
Then, you can substitute nanotime() for ktime_get_real_ts64()?:
http://fxr.watson.org/fxr/source/kern/kern_tc.c?im=3#L387,938

-Deepa

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 21/24] libceph: Replace CURRENT_TIME with ktime_get_real_ts
  2016-06-22  0:56     ` Deepa Dinamani
@ 2016-06-22  8:20       ` Willem Jan Withagen
  2016-06-23  0:17         ` Deepa Dinamani
  0 siblings, 1 reply; 13+ messages in thread
From: Willem Jan Withagen @ 2016-06-22  8:20 UTC (permalink / raw)
  To: Deepa Dinamani; +Cc: ceph-devel, Arnd Bergmann

On 22-6-2016 02:56, Deepa Dinamani wrote:
> Adding Arnd, as he is the overall y2038 project lead.
> 
>>> CURRENT_TIME is not y2038 safe.
>>> The macro will be deleted and all the references to it
>>> will be replaced by ktime_get_* apis.
>>>
>>> struct timespec is also not y2038 safe.
>>> Retain timespec for timestamp representation here as ceph
>>> uses it internally everywhere.
>>> These references will be changed to use struct timespec64
>>> in a separate patch.
>> Hi Deepa,
>>
>> Although I applaud 64-bit timestamps,
>> the ktime_get_* api seems to be a Linux only API.
>> So IF ever things need to be the least bit portable, I suggest wrapping
>> these routines into a class of itself. So that porting confines itself
>> to only that class. (plus/minus hairy semantic details)
> 
> ktime_get_* apis just return 64 bit timestamps.
> They are the equivalent of CURRENT_TIME.
> Did you have a wrapper for CURRENT_TIME?
> 
> Are these concerns about all of ceph kernel code or just that the libceph
> is expected to be portable?
> 
>> For what is now committed the change is not really that big.
>>
>> On the other hand, I 'll have to start figuring out if FreeBSD really
>> does do 64bit timestamps...
> 
> From what I can see here:
> http://fxr.watson.org/fxr/ident?im=10;i=__time_t,
> 
> FreeBSD time_t is already 64 bit long.
> So it supports 64 bit timestamps already.

Yup, I look a bit further into it. After your mail I started diging into
my memory and realised that 64bit times are quite some time on FreeBSD.
And wrote even a small test to be sure...

#include <stdio.h>
#include <time.h>
#include <sys/stat.h>
#include <sys/timespec.h>

struct stat s;
struct itimerspec t;

int main () {
        printf( "Size of __time_t: %lu bytes\n", sizeof(__time_t));
        printf( "Size of st_atim: %lu bytes\n", sizeof(s.st_atim));
        printf( "Size of timespec: %lu bytes\n", sizeof(t.it_interval));

}
[~/Src/c-code] wjw > print_time_t_size
Size of __time_t: 8 bytes
Size of st_atim: 16 bytes
Size of timespec: 16 bytes

So FreeBSD is good.

> 
>> So I would welcome a more elaborate description of the problem (it seems
>> that Linux is going 64bit in its inodes??) and what is being fixed and
>> how. So that in the porting case not only the code will be the source to
>> figure out what is going on.
> 
> Yes, all inode timestamps are going to be switched over to use timespec64 i.e.,
> 64 bit timestamps.
> We will switch over just the vfs layer: struct inode, struct attr and
> struct kstat.
> End filesystems such as ceph will still use timespec at this time.
> So whenever they access vfs timestamps, they will go through conversion apis:
> timespec64_to_timespec() / timespec_to_timespec64().
> 
> My reference tree is https://github.com/deepa-hub/vfs/tree/y2038 .
> 
> This gives you an overall idea of the changes.
> Only a few names have to be updated according to review discussions.
> But, you can see what the end filesystems will look like after the vfs
> switch-over.
> 
> The problem with ceph is really that the timestamps sent over wire use the same
> encode/ decode function calls whether the timestamp is related to files or not.
> I'm not sure what the plan for wire protocol is here. As Sage
> suggested on the thread:
> http://www.spinics.net/lists/ceph-devel/msg28960.html , you could
> interpret time values as u32.

I've seen the discussion here but for the time being chose to ignore
that, since whatever would come from it
> 
> After this, porting to BSD should be straight forward.
> Then, you can substitute nanotime() for ktime_get_real_ts64()?:
> http://fxr.watson.org/fxr/source/kern/kern_tc.c?im=3#L387,938

So that is more or less why I suggest to make into a (small) class.
Otherwise there are going to be wraped into ifdef's.
You could even put it just in a *.h file, and than a good compiler would
do the optimisation, and clean-out one call set, and inline.

--WjW

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Y2038] [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
  2016-06-20  0:26 [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Deepa Dinamani
                   ` (3 preceding siblings ...)
       [not found] ` <1466382443-11063-1-git-send-email-deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-06-22 15:49 ` Arnd Bergmann
  4 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2016-06-22 15:49 UTC (permalink / raw)
  To: y2038, tglx, linux-ext4, jbacik
  Cc: shaggy, jfs-discussion, trond.myklebust, adrian.hunter, clm,
	adilger.kernel, Deepa Dinamani, sfrench, jejb, paul, linux-scsi,
	cm224.lee, mfasheh, linux-nfs, john.stultz, viro, dsterba,
	jaegeuk, ceph-devel, jlbec, sramars, elder, tytso, sage,
	martin.petersen, gregkh, hiralpat, linux-kernel, eparis,
	linux-f2fs-devel, zyan, linux-audit, linux-btrfs, jack,
	linux-fsdevel, linux-mtd, torvalds

On Sunday, June 19, 2016 5:26:59 PM CEST Deepa Dinamani wrote:
> 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.
> 
> Thanks to Arnd Bergmann for all the guidance and discussions.
> 
> Patches 2-4 were mostly generated using coccinelle scripts.
> 
> All filesystem timestamps use current_fs_time() for right granularity as
> mentioned in the respective commit texts of patches. This has a changed
> signature, renamed to current_time() and moved to the fs/inode.c.
> 
> 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_time() now is used as a single generic vfs filesystem timestamp api.
> It also takes struct inode* as argument instead of struct super_block*.
> 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.

I've looked in detail at all the patches in this version now, and while
overall everything is fine, I found that two patches cannot be part of the
series because of the dependency on the patch that John already took (adding
time64_to_tm), but I think that's ok because we just need to change over
all the users of CURRENT_TIME and CURRENT_TIME_SEC that assign to inode
timestamps in order to prepare for the type change, the other ones
can be changed later.

I also found a few things that could be done differently to make the
later conversion slightly easier, but it's also possible that I missed
part of your bigger plan for those files, and none of them seem important.

	Arnd

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 21/24] libceph: Replace CURRENT_TIME with ktime_get_real_ts
  2016-06-22  8:20       ` Willem Jan Withagen
@ 2016-06-23  0:17         ` Deepa Dinamani
  2016-06-23  7:01           ` Willem Jan Withagen
  0 siblings, 1 reply; 13+ messages in thread
From: Deepa Dinamani @ 2016-06-23  0:17 UTC (permalink / raw)
  To: Willem Jan Withagen; +Cc: ceph-devel, Arnd Bergmann

>> Adding Arnd, as he is the overall y2038 project lead.
>>
>>>> CURRENT_TIME is not y2038 safe.
>>>> The macro will be deleted and all the references to it
>>>> will be replaced by ktime_get_* apis.
>>>>
>>>> struct timespec is also not y2038 safe.
>>>> Retain timespec for timestamp representation here as ceph
>>>> uses it internally everywhere.
>>>> These references will be changed to use struct timespec64
>>>> in a separate patch.
>>> Hi Deepa,
>>>
>>> Although I applaud 64-bit timestamps,
>>> the ktime_get_* api seems to be a Linux only API.
>>> So IF ever things need to be the least bit portable, I suggest wrapping
>>> these routines into a class of itself. So that porting confines itself
>>> to only that class. (plus/minus hairy semantic details)
>>
>> ktime_get_* apis just return 64 bit timestamps.
>> They are the equivalent of CURRENT_TIME.
>> Did you have a wrapper for CURRENT_TIME?
>>
>> Are these concerns about all of ceph kernel code or just that the libceph
>> is expected to be portable?
>>
>>> For what is now committed the change is not really that big.
>>>
>>> On the other hand, I 'll have to start figuring out if FreeBSD really
>>> does do 64bit timestamps...
>>
>> From what I can see here:
>> http://fxr.watson.org/fxr/ident?im=10;i=__time_t,
>>
>> FreeBSD time_t is already 64 bit long.
>> So it supports 64 bit timestamps already.
>
> Yup, I look a bit further into it. After your mail I started diging into
> my memory and realised that 64bit times are quite some time on FreeBSD.
> And wrote even a small test to be sure...
>
> #include <stdio.h>
> #include <time.h>
> #include <sys/stat.h>
> #include <sys/timespec.h>
>
> struct stat s;
> struct itimerspec t;
>
> int main () {
>         printf( "Size of __time_t: %lu bytes\n", sizeof(__time_t));
>         printf( "Size of st_atim: %lu bytes\n", sizeof(s.st_atim));
>         printf( "Size of timespec: %lu bytes\n", sizeof(t.it_interval));
>
> }
> [~/Src/c-code] wjw > print_time_t_size
> Size of __time_t: 8 bytes
> Size of st_atim: 16 bytes
> Size of timespec: 16 bytes
>
> So FreeBSD is good.
>
>>
>>> So I would welcome a more elaborate description of the problem (it seems
>>> that Linux is going 64bit in its inodes??) and what is being fixed and
>>> how. So that in the porting case not only the code will be the source to
>>> figure out what is going on.
>>
>> Yes, all inode timestamps are going to be switched over to use timespec64 i.e.,
>> 64 bit timestamps.
>> We will switch over just the vfs layer: struct inode, struct attr and
>> struct kstat.
>> End filesystems such as ceph will still use timespec at this time.
>> So whenever they access vfs timestamps, they will go through conversion apis:
>> timespec64_to_timespec() / timespec_to_timespec64().
>>
>> My reference tree is https://github.com/deepa-hub/vfs/tree/y2038 .
>>
>> This gives you an overall idea of the changes.
>> Only a few names have to be updated according to review discussions.
>> But, you can see what the end filesystems will look like after the vfs
>> switch-over.
>>
>> The problem with ceph is really that the timestamps sent over wire use the same
>> encode/ decode function calls whether the timestamp is related to files or not.
>> I'm not sure what the plan for wire protocol is here. As Sage
>> suggested on the thread:
>> http://www.spinics.net/lists/ceph-devel/msg28960.html , you could
>> interpret time values as u32.
>
> I've seen the discussion here but for the time being chose to ignore
> that, since whatever would come from it
>>
>> After this, porting to BSD should be straight forward.
>> Then, you can substitute nanotime() for ktime_get_real_ts64()?:
>> http://fxr.watson.org/fxr/source/kern/kern_tc.c?im=3#L387,938
>
> So that is more or less why I suggest to make into a (small) class.
> Otherwise there are going to be wraped into ifdef's.
> You could even put it just in a *.h file, and than a good compiler would
> do the optimisation, and clean-out one call set, and inline.

I'm not sure what you mean by class here.
Could you elaborate?

ktime apis are not available in the userspace and the changes here are
only relevant to linux kernel.

-Deepa

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 21/24] libceph: Replace CURRENT_TIME with ktime_get_real_ts
  2016-06-23  0:17         ` Deepa Dinamani
@ 2016-06-23  7:01           ` Willem Jan Withagen
  0 siblings, 0 replies; 13+ messages in thread
From: Willem Jan Withagen @ 2016-06-23  7:01 UTC (permalink / raw)
  To: Deepa Dinamani; +Cc: ceph-devel, Arnd Bergmann

On 23-6-2016 02:17, Deepa Dinamani wrote:
>>> Adding Arnd, as he is the overall y2038 project lead.
>>>
>>>>> CURRENT_TIME is not y2038 safe.
>>>>> The macro will be deleted and all the references to it
>>>>> will be replaced by ktime_get_* apis.
>>>>>
>>>>> struct timespec is also not y2038 safe.
>>>>> Retain timespec for timestamp representation here as ceph
>>>>> uses it internally everywhere.
>>>>> These references will be changed to use struct timespec64
>>>>> in a separate patch.
>>>> Hi Deepa,
>>>>
>>>> Although I applaud 64-bit timestamps,
>>>> the ktime_get_* api seems to be a Linux only API.
>>>> So IF ever things need to be the least bit portable, I suggest wrapping
>>>> these routines into a class of itself. So that porting confines itself
>>>> to only that class. (plus/minus hairy semantic details)
>>>
>>> ktime_get_* apis just return 64 bit timestamps.
>>> They are the equivalent of CURRENT_TIME.
>>> Did you have a wrapper for CURRENT_TIME?
>>>
>>> Are these concerns about all of ceph kernel code or just that the libceph
>>> is expected to be portable?
>>>
>>>> For what is now committed the change is not really that big.
>>>>
>>>> On the other hand, I 'll have to start figuring out if FreeBSD really
>>>> does do 64bit timestamps...
>>>
>>> From what I can see here:
>>> http://fxr.watson.org/fxr/ident?im=10;i=__time_t,
>>>
>>> FreeBSD time_t is already 64 bit long.
>>> So it supports 64 bit timestamps already.
>>
>> Yup, I look a bit further into it. After your mail I started diging into
>> my memory and realised that 64bit times are quite some time on FreeBSD.
>> And wrote even a small test to be sure...
>>
>> #include <stdio.h>
>> #include <time.h>
>> #include <sys/stat.h>
>> #include <sys/timespec.h>
>>
>> struct stat s;
>> struct itimerspec t;
>>
>> int main () {
>>         printf( "Size of __time_t: %lu bytes\n", sizeof(__time_t));
>>         printf( "Size of st_atim: %lu bytes\n", sizeof(s.st_atim));
>>         printf( "Size of timespec: %lu bytes\n", sizeof(t.it_interval));
>>
>> }
>> [~/Src/c-code] wjw > print_time_t_size
>> Size of __time_t: 8 bytes
>> Size of st_atim: 16 bytes
>> Size of timespec: 16 bytes
>>
>> So FreeBSD is good.
>>
>>>
>>>> So I would welcome a more elaborate description of the problem (it seems
>>>> that Linux is going 64bit in its inodes??) and what is being fixed and
>>>> how. So that in the porting case not only the code will be the source to
>>>> figure out what is going on.
>>>
>>> Yes, all inode timestamps are going to be switched over to use timespec64 i.e.,
>>> 64 bit timestamps.
>>> We will switch over just the vfs layer: struct inode, struct attr and
>>> struct kstat.
>>> End filesystems such as ceph will still use timespec at this time.
>>> So whenever they access vfs timestamps, they will go through conversion apis:
>>> timespec64_to_timespec() / timespec_to_timespec64().
>>>
>>> My reference tree is https://github.com/deepa-hub/vfs/tree/y2038 .
>>>
>>> This gives you an overall idea of the changes.
>>> Only a few names have to be updated according to review discussions.
>>> But, you can see what the end filesystems will look like after the vfs
>>> switch-over.
>>>
>>> The problem with ceph is really that the timestamps sent over wire use the same
>>> encode/ decode function calls whether the timestamp is related to files or not.
>>> I'm not sure what the plan for wire protocol is here. As Sage
>>> suggested on the thread:
>>> http://www.spinics.net/lists/ceph-devel/msg28960.html , you could
>>> interpret time values as u32.
>>
>> I've seen the discussion here but for the time being chose to ignore
>> that, since whatever would come from it
>>>
>>> After this, porting to BSD should be straight forward.
>>> Then, you can substitute nanotime() for ktime_get_real_ts64()?:
>>> http://fxr.watson.org/fxr/source/kern/kern_tc.c?im=3#L387,938
>>
>> So that is more or less why I suggest to make into a (small) class.
>> Otherwise there are going to be wraped into ifdef's.
>> You could even put it just in a *.h file, and than a good compiler would
>> do the optimisation, and clean-out one call set, and inline.
> 
> I'm not sure what you mean by class here.
> Could you elaborate?
> 
> ktime apis are not available in the userspace and the changes here are
> only relevant to linux kernel.

'mmm,

I was certain that I saw the ktime api calls in Ceph code.
ktime_get_real_ts() is used in a few of the Ceph-modules to replace
CURRENT_TIME. Perhaps that is not really part of the ktime API??

eg.:
-	osd_req->r_mtime = CURRENT_TIME;
+	ktime_get_real_ts(&osd_req->r_mtime);

Having this wrapped in a separate class or routine would make porting a
nobrainer. Other OSes just need to augment this one routine.

--WjW

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-06-23  7:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-20  0:26 [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Deepa Dinamani
2016-06-20  0:27 ` [PATCH v2 20/24] block: Replace CURRENT_TIME with ktime_get_real_ts Deepa Dinamani
2016-06-20  0:27 ` [PATCH v2 21/24] libceph: " Deepa Dinamani
2016-06-20  8:33   ` Willem Jan Withagen
2016-06-22  0:56     ` Deepa Dinamani
2016-06-22  8:20       ` Willem Jan Withagen
2016-06-23  0:17         ` Deepa Dinamani
2016-06-23  7:01           ` Willem Jan Withagen
2016-06-20  0:27 ` [PATCH v2 22/24] fs: ceph: Replace current_fs_time for request stamp Deepa Dinamani
     [not found] ` <1466382443-11063-1-git-send-email-deepa.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-06-20 18:03   ` [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros Linus Torvalds
     [not found]     ` <CA+55aFzK2K-7UeQRsWR7ooNHMMbtMFRAF60byR1Gvmbf9XWhbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-20 18:58       ` Deepa Dinamani
2016-06-21 15:00       ` [Y2038] " Arnd Bergmann
2016-06-22 15:49 ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox