[PATCH v3 00/11] xfs: widen timestamps to deal with y2038

[PATCH v3 00/11] xfs: widen timestamps to deal with y2038

[Darrick J. Wong]

Hi all,

This series performs some refactoring of our timestamp and inode
encoding functions, then retrofits the timestamp union to handle
timestamps as a 64-bit nanosecond counter.  Next, it adds bit shifting
to the non-root dquot timer fields to boost their effective size to 34
bits.  These two changes enable correct time handling on XFS through the
year 2486.

On a current V5 filesystem, inodes timestamps are a signed 32-bit
seconds counter, with 0 being the Unix epoch.  Quota timers are an
unsigned 32-bit seconds counter, with 0 also being the Unix epoch.

This means that inode timestamps can range from:
-(2^31-1) (13 Dec 1901) through (2^31-1) (19 Jan 2038).

And quota timers can range from:
0 (1 Jan 1970) through (2^32-1) (7 Feb 2106).

With the bigtime encoding turned on, inode timestamps are an unsigned
64-bit nanoseconds counter, with 0 being the 1901 epoch.  Quota timers
are a 34-bit unsigned second counter right shifted two bits, with 0
being the Unix epoch, and capped at the maximum inode timestamp value.

This means that inode timestamps can range from:
0 (13 Dec 1901) through (2^64-1 / 1e9) (2 Jul 2486)

Quota timers could theoretically range from:
0 (1 Jan 1970) through (((2^34-1) + (2^31-1)) & ~3) (16 Jun 2582).

But with the capping in place, the quota timers maximum is:
max((2^64-1 / 1e9) - (2^31-1), (((2^34-1) + (2^31-1)) & ~3) (2 Jul 2486).

v2: rebase to 5.9, having landed the quota refactoring
v3: various suggestions by Amir and Dave

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=bigtime

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=bigtime

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=bigtime
---
 fs/xfs/libxfs/xfs_dquot_buf.c   |   48 +++++++++++++
 fs/xfs/libxfs/xfs_format.h      |  140 ++++++++++++++++++++++++++++++++++++---
 fs/xfs/libxfs/xfs_fs.h          |    1 
 fs/xfs/libxfs/xfs_inode_buf.c   |  132 ++++++++++++++++++-------------------
 fs/xfs/libxfs/xfs_inode_buf.h   |    7 +-
 fs/xfs/libxfs/xfs_log_format.h  |   21 ++++--
 fs/xfs/libxfs/xfs_quota_defs.h  |    9 ++-
 fs/xfs/libxfs/xfs_sb.c          |    2 +
 fs/xfs/libxfs/xfs_trans_inode.c |   11 +++
 fs/xfs/scrub/inode.c            |   31 ++++++---
 fs/xfs/xfs_dquot.c              |   57 +++++++++++++---
 fs/xfs/xfs_dquot.h              |    4 +
 fs/xfs/xfs_inode.c              |   15 ++++
 fs/xfs/xfs_inode_item.c         |   97 +++++++++++++++++++++++++--
 fs/xfs/xfs_inode_item.h         |    3 +
 fs/xfs/xfs_ioctl.c              |    3 +
 fs/xfs/xfs_ondisk.h             |   33 +++++++++
 fs/xfs/xfs_qm.c                 |    2 +
 fs/xfs/xfs_qm_syscalls.c        |   18 +++--
 fs/xfs/xfs_super.c              |   14 +++-
 fs/xfs/xfs_trans_dquot.c        |    6 ++
 21 files changed, 526 insertions(+), 128 deletions(-)

[PATCH 01/11] xfs: explicitly define inode timestamp range

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Formally define the inode timestamp ranges that existing filesystems
support, and switch the vfs timetamp ranges to use it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_format.h |   19 +++++++++++++++++++
 fs/xfs/xfs_ondisk.h        |   12 ++++++++++++
 fs/xfs/xfs_super.c         |    5 +++--
 3 files changed, 34 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index be86fa1a5556..b1b8a5c05cea 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -849,11 +849,30 @@ struct xfs_agfl {
 	    ASSERT(xfs_daddr_to_agno(mp, d) == \
 		   xfs_daddr_to_agno(mp, (d) + (len) - 1)))
 
+/*
+ * XFS Timestamps
+ * ==============
+ *
+ * Inode timestamps consist of signed 32-bit counters for seconds and
+ * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
+ */
 typedef struct xfs_timestamp {
 	__be32		t_sec;		/* timestamp seconds */
 	__be32		t_nsec;		/* timestamp nanoseconds */
 } xfs_timestamp_t;
 
+/*
+ * Smallest possible timestamp with traditional timestamps, which is
+ * Dec 13 20:45:52 UTC 1901.
+ */
+#define XFS_INO_TIME_MIN	((int64_t)S32_MIN)
+
+/*
+ * Largest possible timestamp with traditional timestamps, which is
+ * Jan 19 03:14:07 UTC 2038.
+ */
+#define XFS_INO_TIME_MAX	((int64_t)S32_MAX)
+
 /*
  * On-disk inode structure.
  *
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index acb9b737fe6b..48a64fa49f91 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -15,6 +15,18 @@
 		"XFS: offsetof(" #structname ", " #member ") is wrong, " \
 		"expected " #off)
 
+#define XFS_CHECK_VALUE(value, expected) \
+	BUILD_BUG_ON_MSG((value) != (expected), \
+		"XFS: value of " #value " is wrong, expected " #expected)
+
+static inline void __init
+xfs_check_limits(void)
+{
+	/* make sure timestamp limits are correct */
+	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
+	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
+}
+
 static inline void __init
 xfs_check_ondisk_structs(void)
 {
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index c7ffcb57b586..375f05a47ba4 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1484,8 +1484,8 @@ xfs_fc_fill_super(
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_max_links = XFS_MAXLINK;
 	sb->s_time_gran = 1;
-	sb->s_time_min = S32_MIN;
-	sb->s_time_max = S32_MAX;
+	sb->s_time_min = XFS_INO_TIME_MIN;
+	sb->s_time_max = XFS_INO_TIME_MAX;
 	sb->s_iflags |= SB_I_CGROUPWB;
 
 	set_posix_acl_flag(sb);
@@ -2077,6 +2077,7 @@ init_xfs_fs(void)
 {
 	int			error;
 
+	xfs_check_limits();
 	xfs_check_ondisk_structs();
 
 	printk(KERN_INFO XFS_VERSION_STRING " with "

[PATCH 02/11] xfs: refactor quota expiration timer modification

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Define explicit limits on the range of quota grace period expiration
timeouts and refactor the code that modifies the timeouts into helpers
that clamp the values appropriately.  Note that we'll deal with the
grace period timer separately.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_format.h |   22 ++++++++++++++++++++++
 fs/xfs/xfs_dquot.c         |   13 ++++++++++++-
 fs/xfs/xfs_dquot.h         |    2 ++
 fs/xfs/xfs_ondisk.h        |    2 ++
 fs/xfs/xfs_qm_syscalls.c   |    9 +++++++--
 5 files changed, 45 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index b1b8a5c05cea..ef36978239ac 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1197,6 +1197,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
 
 #define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK)
 
+/*
+ * XFS Quota Timers
+ * ================
+ *
+ * Quota grace period expiration timers are an unsigned 32-bit seconds counter;
+ * time zero is the Unix epoch, Jan  1 00:00:01 UTC 1970.  An expiration value
+ * of zero means that the quota limit has not been reached, and therefore no
+ * expiration has been set.
+ */
+
+/*
+ * Smallest possible quota expiration with traditional timestamps, which is
+ * Jan  1 00:00:01 UTC 1970.
+ */
+#define XFS_DQ_TIMEOUT_MIN	((int64_t)1)
+
+/*
+ * Largest possible quota expiration with traditional timestamps, which is
+ * Feb  7 06:28:15 UTC 2106.
+ */
+#define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
+
 /*
  * This is the main portion of the on-disk representation of quota information
  * for a user.  We pad this with some more expansion room to construct the on
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index bcd73b9c2994..2425b1c30d11 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -98,6 +98,16 @@ xfs_qm_adjust_dqlimits(
 		xfs_dquot_set_prealloc_limits(dq);
 }
 
+/* Set the expiration time of a quota's grace period. */
+void
+xfs_dquot_set_timeout(
+	time64_t		*timer,
+	time64_t		value)
+{
+	*timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN,
+					  XFS_DQ_TIMEOUT_MAX);
+}
+
 /*
  * Determine if this quota counter is over either limit and set the quota
  * timers as appropriate.
@@ -112,7 +122,8 @@ xfs_qm_adjust_res_timer(
 	if ((res->softlimit && res->count > res->softlimit) ||
 	    (res->hardlimit && res->count > res->hardlimit)) {
 		if (res->timer == 0)
-			res->timer = ktime_get_real_seconds() + qlim->time;
+			xfs_dquot_set_timeout(&res->timer,
+					ktime_get_real_seconds() + qlim->time);
 	} else {
 		if (res->timer == 0)
 			res->warnings = 0;
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 282a65da93c7..11bd0ee9b0fa 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -237,4 +237,6 @@ typedef int (*xfs_qm_dqiterate_fn)(struct xfs_dquot *dq,
 int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type,
 		xfs_qm_dqiterate_fn iter_fn, void *priv);
 
+void xfs_dquot_set_timeout(time64_t *timer, time64_t limit);
+
 #endif /* __XFS_DQUOT_H__ */
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 48a64fa49f91..38ccffcf3336 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -25,6 +25,8 @@ xfs_check_limits(void)
 	/* make sure timestamp limits are correct */
 	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
 	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
+	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MIN,			1LL);
+	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MAX,			4294967295LL);
 }
 
 static inline void __init
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 1c542b4a5220..b16d533a6feb 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -483,9 +483,14 @@ xfs_setqlim_timer(
 	struct xfs_quota_limits	*qlim,
 	s64			timer)
 {
-	res->timer = timer;
-	if (qlim)
+	if (qlim) {
+		/* Set the length of the default grace period. */
+		res->timer = timer;
 		qlim->time = timer;
+	} else {
+		/* Set the grace period expiration on a quota. */
+		xfs_dquot_set_timeout(&res->timer, timer);
+	}
 }
 
 /*

[PATCH 03/11] xfs: refactor default quota grace period setting code

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor the code that sets the default quota grace period into a helper
function so that we can override the ondisk behavior later.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_format.h |   13 +++++++++++++
 fs/xfs/xfs_dquot.c         |    9 +++++++++
 fs/xfs/xfs_dquot.h         |    1 +
 fs/xfs/xfs_ondisk.h        |    2 ++
 fs/xfs/xfs_qm_syscalls.c   |    4 ++--
 5 files changed, 27 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index ef36978239ac..e9e6248b35be 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1205,6 +1205,11 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  * time zero is the Unix epoch, Jan  1 00:00:01 UTC 1970.  An expiration value
  * of zero means that the quota limit has not been reached, and therefore no
  * expiration has been set.
+ *
+ * The grace period for each quota type is stored in the root dquot (id = 0)
+ * and is applied to a non-root dquot when it exceeds the soft or hard limits.
+ * The length of quota grace periods are unsigned 32-bit quantities measured in
+ * units of seconds.  A value of zero means to use the default period.
  */
 
 /*
@@ -1219,6 +1224,14 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  */
 #define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
 
+/*
+ * Default quota grace periods, ranging from zero (use the compiled defaults)
+ * to ~136 years.  These are applied to a non-root dquot that has exceeded
+ * either limit.
+ */
+#define XFS_DQ_GRACE_MIN	((int64_t)0)
+#define XFS_DQ_GRACE_MAX	((int64_t)U32_MAX)
+
 /*
  * This is the main portion of the on-disk representation of quota information
  * for a user.  We pad this with some more expansion room to construct the on
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 2425b1c30d11..ed3fa6ada0d3 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -98,6 +98,15 @@ xfs_qm_adjust_dqlimits(
 		xfs_dquot_set_prealloc_limits(dq);
 }
 
+/* Set the length of the default grace period. */
+void
+xfs_dquot_set_grace_period(
+	time64_t		*timer,
+	time64_t		value)
+{
+	*timer = clamp_t(time64_t, value, XFS_DQ_GRACE_MIN, XFS_DQ_GRACE_MAX);
+}
+
 /* Set the expiration time of a quota's grace period. */
 void
 xfs_dquot_set_timeout(
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 11bd0ee9b0fa..0ba4d91c3a11 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -237,6 +237,7 @@ typedef int (*xfs_qm_dqiterate_fn)(struct xfs_dquot *dq,
 int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type,
 		xfs_qm_dqiterate_fn iter_fn, void *priv);
 
+void xfs_dquot_set_grace_period(time64_t *timer, time64_t limit);
 void xfs_dquot_set_timeout(time64_t *timer, time64_t limit);
 
 #endif /* __XFS_DQUOT_H__ */
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 38ccffcf3336..498e9063c605 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -27,6 +27,8 @@ xfs_check_limits(void)
 	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
 	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MIN,			1LL);
 	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MAX,			4294967295LL);
+	XFS_CHECK_VALUE(XFS_DQ_GRACE_MIN,			0LL);
+	XFS_CHECK_VALUE(XFS_DQ_GRACE_MAX,			4294967295LL);
 }
 
 static inline void __init
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index b16d533a6feb..95b0c25b9969 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -485,8 +485,8 @@ xfs_setqlim_timer(
 {
 	if (qlim) {
 		/* Set the length of the default grace period. */
-		res->timer = timer;
-		qlim->time = timer;
+		xfs_dquot_set_grace_period(&res->timer, timer);
+		qlim->time = res->timer;
 	} else {
 		/* Set the grace period expiration on a quota. */
 		xfs_dquot_set_timeout(&res->timer, timer);

[PATCH 04/11] xfs: remove xfs_timestamp_t

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Kill this old typedef.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_format.h     |   12 ++++++------
 fs/xfs/libxfs/xfs_log_format.h |   12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index e9e6248b35be..1f3a2be6c396 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -856,10 +856,10 @@ struct xfs_agfl {
  * Inode timestamps consist of signed 32-bit counters for seconds and
  * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
  */
-typedef struct xfs_timestamp {
+struct xfs_timestamp {
 	__be32		t_sec;		/* timestamp seconds */
 	__be32		t_nsec;		/* timestamp nanoseconds */
-} xfs_timestamp_t;
+};
 
 /*
  * Smallest possible timestamp with traditional timestamps, which is
@@ -904,9 +904,9 @@ typedef struct xfs_dinode {
 	__be16		di_projid_hi;	/* higher part owner's project id */
 	__u8		di_pad[6];	/* unused, zeroed space */
 	__be16		di_flushiter;	/* incremented on flush */
-	xfs_timestamp_t	di_atime;	/* time last accessed */
-	xfs_timestamp_t	di_mtime;	/* time last modified */
-	xfs_timestamp_t	di_ctime;	/* time created/inode modified */
+	struct xfs_timestamp di_atime;	/* time last accessed */
+	struct xfs_timestamp di_mtime;	/* time last modified */
+	struct xfs_timestamp di_ctime;	/* time created/inode modified */
 	__be64		di_size;	/* number of bytes in file */
 	__be64		di_nblocks;	/* # of direct & btree blocks used */
 	__be32		di_extsize;	/* basic/minimum extent size for file */
@@ -931,7 +931,7 @@ typedef struct xfs_dinode {
 	__u8		di_pad2[12];	/* more padding for future expansion */
 
 	/* fields only written to during inode creation */
-	xfs_timestamp_t	di_crtime;	/* time created */
+	struct xfs_timestamp di_crtime;	/* time created */
 	__be64		di_ino;		/* inode number */
 	uuid_t		di_uuid;	/* UUID of the filesystem */
 
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index e3400c9c71cd..f2fac9bea66d 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -368,10 +368,10 @@ static inline int xfs_ilog_fdata(int w)
  * directly mirrors the xfs_dinode structure as it must contain all the same
  * information.
  */
-typedef struct xfs_ictimestamp {
+struct xfs_ictimestamp {
 	int32_t		t_sec;		/* timestamp seconds */
 	int32_t		t_nsec;		/* timestamp nanoseconds */
-} xfs_ictimestamp_t;
+};
 
 /*
  * Define the format of the inode core that is logged. This structure must be
@@ -390,9 +390,9 @@ struct xfs_log_dinode {
 	uint16_t	di_projid_hi;	/* higher part of owner's project id */
 	uint8_t		di_pad[6];	/* unused, zeroed space */
 	uint16_t	di_flushiter;	/* incremented on flush */
-	xfs_ictimestamp_t di_atime;	/* time last accessed */
-	xfs_ictimestamp_t di_mtime;	/* time last modified */
-	xfs_ictimestamp_t di_ctime;	/* time created/inode modified */
+	struct xfs_ictimestamp di_atime;	/* time last accessed */
+	struct xfs_ictimestamp di_mtime;	/* time last modified */
+	struct xfs_ictimestamp di_ctime;	/* time created/inode modified */
 	xfs_fsize_t	di_size;	/* number of bytes in file */
 	xfs_rfsblock_t	di_nblocks;	/* # of direct & btree blocks used */
 	xfs_extlen_t	di_extsize;	/* basic/minimum extent size for file */
@@ -417,7 +417,7 @@ struct xfs_log_dinode {
 	uint8_t		di_pad2[12];	/* more padding for future expansion */
 
 	/* fields only written to during inode creation */
-	xfs_ictimestamp_t di_crtime;	/* time created */
+	struct xfs_ictimestamp di_crtime;	/* time created */
 	xfs_ino_t	di_ino;		/* inode number */
 	uuid_t		di_uuid;	/* UUID of the filesystem */
 

[PATCH 05/11] xfs: move xfs_log_dinode_to_disk to the log code

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Move this function to xfs_inode_item.c to match the encoding function
that's already there.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c |   52 -----------------------------------------
 fs/xfs/libxfs/xfs_inode_buf.h |    2 --
 fs/xfs/xfs_inode_item.c       |   52 +++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_inode_item.h       |    3 ++
 4 files changed, 55 insertions(+), 54 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 8d5dd08eab75..fa83591ca89b 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -310,58 +310,6 @@ xfs_inode_to_disk(
 	}
 }
 
-void
-xfs_log_dinode_to_disk(
-	struct xfs_log_dinode	*from,
-	struct xfs_dinode	*to)
-{
-	to->di_magic = cpu_to_be16(from->di_magic);
-	to->di_mode = cpu_to_be16(from->di_mode);
-	to->di_version = from->di_version;
-	to->di_format = from->di_format;
-	to->di_onlink = 0;
-	to->di_uid = cpu_to_be32(from->di_uid);
-	to->di_gid = cpu_to_be32(from->di_gid);
-	to->di_nlink = cpu_to_be32(from->di_nlink);
-	to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
-	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
-	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
-
-	to->di_atime.t_sec = cpu_to_be32(from->di_atime.t_sec);
-	to->di_atime.t_nsec = cpu_to_be32(from->di_atime.t_nsec);
-	to->di_mtime.t_sec = cpu_to_be32(from->di_mtime.t_sec);
-	to->di_mtime.t_nsec = cpu_to_be32(from->di_mtime.t_nsec);
-	to->di_ctime.t_sec = cpu_to_be32(from->di_ctime.t_sec);
-	to->di_ctime.t_nsec = cpu_to_be32(from->di_ctime.t_nsec);
-
-	to->di_size = cpu_to_be64(from->di_size);
-	to->di_nblocks = cpu_to_be64(from->di_nblocks);
-	to->di_extsize = cpu_to_be32(from->di_extsize);
-	to->di_nextents = cpu_to_be32(from->di_nextents);
-	to->di_anextents = cpu_to_be16(from->di_anextents);
-	to->di_forkoff = from->di_forkoff;
-	to->di_aformat = from->di_aformat;
-	to->di_dmevmask = cpu_to_be32(from->di_dmevmask);
-	to->di_dmstate = cpu_to_be16(from->di_dmstate);
-	to->di_flags = cpu_to_be16(from->di_flags);
-	to->di_gen = cpu_to_be32(from->di_gen);
-
-	if (from->di_version == 3) {
-		to->di_changecount = cpu_to_be64(from->di_changecount);
-		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec);
-		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec);
-		to->di_flags2 = cpu_to_be64(from->di_flags2);
-		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
-		to->di_ino = cpu_to_be64(from->di_ino);
-		to->di_lsn = cpu_to_be64(from->di_lsn);
-		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
-		uuid_copy(&to->di_uuid, &from->di_uuid);
-		to->di_flushiter = 0;
-	} else {
-		to->di_flushiter = cpu_to_be16(from->di_flushiter);
-	}
-}
-
 static xfs_failaddr_t
 xfs_dinode_verify_fork(
 	struct xfs_dinode	*dip,
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 6b08b9d060c2..89f7bea8efd6 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -49,8 +49,6 @@ void	xfs_dinode_calc_crc(struct xfs_mount *, struct xfs_dinode *);
 void	xfs_inode_to_disk(struct xfs_inode *ip, struct xfs_dinode *to,
 			  xfs_lsn_t lsn);
 int	xfs_inode_from_disk(struct xfs_inode *ip, struct xfs_dinode *from);
-void	xfs_log_dinode_to_disk(struct xfs_log_dinode *from,
-			       struct xfs_dinode *to);
 
 xfs_failaddr_t xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
 			   struct xfs_dinode *dip);
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 6c65938cee1c..d95a00376fad 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -295,6 +295,58 @@ xfs_inode_item_format_attr_fork(
 	}
 }
 
+void
+xfs_log_dinode_to_disk(
+	struct xfs_log_dinode	*from,
+	struct xfs_dinode	*to)
+{
+	to->di_magic = cpu_to_be16(from->di_magic);
+	to->di_mode = cpu_to_be16(from->di_mode);
+	to->di_version = from->di_version;
+	to->di_format = from->di_format;
+	to->di_onlink = 0;
+	to->di_uid = cpu_to_be32(from->di_uid);
+	to->di_gid = cpu_to_be32(from->di_gid);
+	to->di_nlink = cpu_to_be32(from->di_nlink);
+	to->di_projid_lo = cpu_to_be16(from->di_projid_lo);
+	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
+	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
+
+	to->di_atime.t_sec = cpu_to_be32(from->di_atime.t_sec);
+	to->di_atime.t_nsec = cpu_to_be32(from->di_atime.t_nsec);
+	to->di_mtime.t_sec = cpu_to_be32(from->di_mtime.t_sec);
+	to->di_mtime.t_nsec = cpu_to_be32(from->di_mtime.t_nsec);
+	to->di_ctime.t_sec = cpu_to_be32(from->di_ctime.t_sec);
+	to->di_ctime.t_nsec = cpu_to_be32(from->di_ctime.t_nsec);
+
+	to->di_size = cpu_to_be64(from->di_size);
+	to->di_nblocks = cpu_to_be64(from->di_nblocks);
+	to->di_extsize = cpu_to_be32(from->di_extsize);
+	to->di_nextents = cpu_to_be32(from->di_nextents);
+	to->di_anextents = cpu_to_be16(from->di_anextents);
+	to->di_forkoff = from->di_forkoff;
+	to->di_aformat = from->di_aformat;
+	to->di_dmevmask = cpu_to_be32(from->di_dmevmask);
+	to->di_dmstate = cpu_to_be16(from->di_dmstate);
+	to->di_flags = cpu_to_be16(from->di_flags);
+	to->di_gen = cpu_to_be32(from->di_gen);
+
+	if (from->di_version == 3) {
+		to->di_changecount = cpu_to_be64(from->di_changecount);
+		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec);
+		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec);
+		to->di_flags2 = cpu_to_be64(from->di_flags2);
+		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
+		to->di_ino = cpu_to_be64(from->di_ino);
+		to->di_lsn = cpu_to_be64(from->di_lsn);
+		memcpy(to->di_pad2, from->di_pad2, sizeof(to->di_pad2));
+		uuid_copy(&to->di_uuid, &from->di_uuid);
+		to->di_flushiter = 0;
+	} else {
+		to->di_flushiter = cpu_to_be16(from->di_flushiter);
+	}
+}
+
 static void
 xfs_inode_to_log_dinode(
 	struct xfs_inode	*ip,
diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h
index 048b5e7dee90..dc924a1c94bc 100644
--- a/fs/xfs/xfs_inode_item.h
+++ b/fs/xfs/xfs_inode_item.h
@@ -50,4 +50,7 @@ extern int xfs_inode_item_format_convert(xfs_log_iovec_t *,
 
 extern struct kmem_zone	*xfs_ili_zone;
 
+void	xfs_log_dinode_to_disk(struct xfs_log_dinode *from,
+			       struct xfs_dinode *to);
+
 #endif	/* __XFS_INODE_ITEM_H__ */

[PATCH 06/11] xfs: refactor inode timestamp coding

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor inode timestamp encoding and decoding into helper functions so
that we can add extra behaviors in subsequent patches.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c |   42 ++++++++++++++++++++++++---------------
 fs/xfs/libxfs/xfs_inode_buf.h |    5 +++++
 fs/xfs/scrub/inode.c          |   25 ++++++++++++++++-------
 fs/xfs/xfs_inode_item.c       |   44 ++++++++++++++++++++++++++---------------
 4 files changed, 76 insertions(+), 40 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index fa83591ca89b..4930eabed6d8 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -157,6 +157,15 @@ xfs_imap_to_bp(
 	return 0;
 }
 
+void
+xfs_inode_from_disk_timestamp(
+	struct timespec64		*tv,
+	const struct xfs_timestamp	*ts)
+{
+	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
+	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
+}
+
 int
 xfs_inode_from_disk(
 	struct xfs_inode	*ip,
@@ -211,12 +220,9 @@ xfs_inode_from_disk(
 	 * a time before epoch is converted to a time long after epoch
 	 * on 64 bit systems.
 	 */
-	inode->i_atime.tv_sec = (int)be32_to_cpu(from->di_atime.t_sec);
-	inode->i_atime.tv_nsec = (int)be32_to_cpu(from->di_atime.t_nsec);
-	inode->i_mtime.tv_sec = (int)be32_to_cpu(from->di_mtime.t_sec);
-	inode->i_mtime.tv_nsec = (int)be32_to_cpu(from->di_mtime.t_nsec);
-	inode->i_ctime.tv_sec = (int)be32_to_cpu(from->di_ctime.t_sec);
-	inode->i_ctime.tv_nsec = (int)be32_to_cpu(from->di_ctime.t_nsec);
+	xfs_inode_from_disk_timestamp(&inode->i_atime, &from->di_atime);
+	xfs_inode_from_disk_timestamp(&inode->i_mtime, &from->di_mtime);
+	xfs_inode_from_disk_timestamp(&inode->i_ctime, &from->di_ctime);
 
 	to->di_size = be64_to_cpu(from->di_size);
 	to->di_nblocks = be64_to_cpu(from->di_nblocks);
@@ -229,8 +235,7 @@ xfs_inode_from_disk(
 	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
 		inode_set_iversion_queried(inode,
 					   be64_to_cpu(from->di_changecount));
-		to->di_crtime.tv_sec = be32_to_cpu(from->di_crtime.t_sec);
-		to->di_crtime.tv_nsec = be32_to_cpu(from->di_crtime.t_nsec);
+		xfs_inode_from_disk_timestamp(&to->di_crtime, &from->di_crtime);
 		to->di_flags2 = be64_to_cpu(from->di_flags2);
 		to->di_cowextsize = be32_to_cpu(from->di_cowextsize);
 	}
@@ -252,6 +257,15 @@ xfs_inode_from_disk(
 	return error;
 }
 
+void
+xfs_inode_to_disk_timestamp(
+	struct xfs_timestamp		*ts,
+	const struct timespec64		*tv)
+{
+	ts->t_sec = cpu_to_be32(tv->tv_sec);
+	ts->t_nsec = cpu_to_be32(tv->tv_nsec);
+}
+
 void
 xfs_inode_to_disk(
 	struct xfs_inode	*ip,
@@ -271,12 +285,9 @@ xfs_inode_to_disk(
 	to->di_projid_hi = cpu_to_be16(from->di_projid >> 16);
 
 	memset(to->di_pad, 0, sizeof(to->di_pad));
-	to->di_atime.t_sec = cpu_to_be32(inode->i_atime.tv_sec);
-	to->di_atime.t_nsec = cpu_to_be32(inode->i_atime.tv_nsec);
-	to->di_mtime.t_sec = cpu_to_be32(inode->i_mtime.tv_sec);
-	to->di_mtime.t_nsec = cpu_to_be32(inode->i_mtime.tv_nsec);
-	to->di_ctime.t_sec = cpu_to_be32(inode->i_ctime.tv_sec);
-	to->di_ctime.t_nsec = cpu_to_be32(inode->i_ctime.tv_nsec);
+	xfs_inode_to_disk_timestamp(&to->di_atime, &inode->i_atime);
+	xfs_inode_to_disk_timestamp(&to->di_mtime, &inode->i_mtime);
+	xfs_inode_to_disk_timestamp(&to->di_ctime, &inode->i_ctime);
 	to->di_nlink = cpu_to_be32(inode->i_nlink);
 	to->di_gen = cpu_to_be32(inode->i_generation);
 	to->di_mode = cpu_to_be16(inode->i_mode);
@@ -295,8 +306,7 @@ xfs_inode_to_disk(
 	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
 		to->di_version = 3;
 		to->di_changecount = cpu_to_be64(inode_peek_iversion(inode));
-		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.tv_sec);
-		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.tv_nsec);
+		xfs_inode_to_disk_timestamp(&to->di_crtime, &from->di_crtime);
 		to->di_flags2 = cpu_to_be64(from->di_flags2);
 		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
 		to->di_ino = cpu_to_be64(ip->i_ino);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 89f7bea8efd6..9c63f3f932d7 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -58,4 +58,9 @@ xfs_failaddr_t xfs_inode_validate_cowextsize(struct xfs_mount *mp,
 		uint32_t cowextsize, uint16_t mode, uint16_t flags,
 		uint64_t flags2);
 
+void xfs_inode_from_disk_timestamp(struct timespec64 *tv,
+		const struct xfs_timestamp *ts);
+void xfs_inode_to_disk_timestamp(struct xfs_timestamp *ts,
+		const struct timespec64 *tv);
+
 #endif	/* __XFS_INODE_BUF_H__ */
diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index 6d483ab29e63..ccb5c217c0ee 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -195,6 +195,19 @@ xchk_inode_flags2(
 	xchk_ino_set_corrupt(sc, ino);
 }
 
+static inline void
+xchk_dinode_nsec(
+	struct xfs_scrub		*sc,
+	xfs_ino_t			ino,
+	const struct xfs_timestamp	*ts)
+{
+	struct timespec64		tv;
+
+	xfs_inode_from_disk_timestamp(&tv, ts);
+	if (tv.tv_nsec < 0 || tv.tv_nsec >= NSEC_PER_SEC)
+		xchk_ino_set_corrupt(sc, ino);
+}
+
 /* Scrub all the ondisk inode fields. */
 STATIC void
 xchk_dinode(
@@ -293,12 +306,9 @@ xchk_dinode(
 	}
 
 	/* di_[amc]time.nsec */
-	if (be32_to_cpu(dip->di_atime.t_nsec) >= NSEC_PER_SEC)
-		xchk_ino_set_corrupt(sc, ino);
-	if (be32_to_cpu(dip->di_mtime.t_nsec) >= NSEC_PER_SEC)
-		xchk_ino_set_corrupt(sc, ino);
-	if (be32_to_cpu(dip->di_ctime.t_nsec) >= NSEC_PER_SEC)
-		xchk_ino_set_corrupt(sc, ino);
+	xchk_dinode_nsec(sc, ino, &dip->di_atime);
+	xchk_dinode_nsec(sc, ino, &dip->di_mtime);
+	xchk_dinode_nsec(sc, ino, &dip->di_ctime);
 
 	/*
 	 * di_size.  xfs_dinode_verify checks for things that screw up
@@ -403,8 +413,7 @@ xchk_dinode(
 	}
 
 	if (dip->di_version >= 3) {
-		if (be32_to_cpu(dip->di_crtime.t_nsec) >= NSEC_PER_SEC)
-			xchk_ino_set_corrupt(sc, ino);
+		xchk_dinode_nsec(sc, ino, &dip->di_crtime);
 		xchk_inode_flags2(sc, dip, ino, mode, flags, flags2);
 		xchk_inode_cowextsize(sc, dip, ino, mode, flags,
 				flags2);
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index d95a00376fad..dac0ab59e88f 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -295,6 +295,16 @@ xfs_inode_item_format_attr_fork(
 	}
 }
 
+/* Write a log_dinode timestamp into an ondisk inode timestamp. */
+static inline void
+xfs_log_dinode_to_disk_ts(
+	struct xfs_timestamp		*ts,
+	const struct xfs_ictimestamp	*its)
+{
+	ts->t_sec = cpu_to_be32(its->t_sec);
+	ts->t_nsec = cpu_to_be32(its->t_nsec);
+}
+
 void
 xfs_log_dinode_to_disk(
 	struct xfs_log_dinode	*from,
@@ -312,12 +322,9 @@ xfs_log_dinode_to_disk(
 	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
 	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
 
-	to->di_atime.t_sec = cpu_to_be32(from->di_atime.t_sec);
-	to->di_atime.t_nsec = cpu_to_be32(from->di_atime.t_nsec);
-	to->di_mtime.t_sec = cpu_to_be32(from->di_mtime.t_sec);
-	to->di_mtime.t_nsec = cpu_to_be32(from->di_mtime.t_nsec);
-	to->di_ctime.t_sec = cpu_to_be32(from->di_ctime.t_sec);
-	to->di_ctime.t_nsec = cpu_to_be32(from->di_ctime.t_nsec);
+	xfs_log_dinode_to_disk_ts(&to->di_atime, &from->di_atime);
+	xfs_log_dinode_to_disk_ts(&to->di_mtime, &from->di_mtime);
+	xfs_log_dinode_to_disk_ts(&to->di_ctime, &from->di_ctime);
 
 	to->di_size = cpu_to_be64(from->di_size);
 	to->di_nblocks = cpu_to_be64(from->di_nblocks);
@@ -333,8 +340,7 @@ xfs_log_dinode_to_disk(
 
 	if (from->di_version == 3) {
 		to->di_changecount = cpu_to_be64(from->di_changecount);
-		to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec);
-		to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec);
+		xfs_log_dinode_to_disk_ts(&to->di_crtime, &from->di_crtime);
 		to->di_flags2 = cpu_to_be64(from->di_flags2);
 		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
 		to->di_ino = cpu_to_be64(from->di_ino);
@@ -347,6 +353,16 @@ xfs_log_dinode_to_disk(
 	}
 }
 
+/* Write an incore inode timestamp into a log_dinode timestamp. */
+static inline void
+xfs_inode_to_log_dinode_ts(
+	struct xfs_ictimestamp		*its,
+	const struct timespec64		*ts)
+{
+	its->t_sec = ts->tv_sec;
+	its->t_nsec = ts->tv_nsec;
+}
+
 static void
 xfs_inode_to_log_dinode(
 	struct xfs_inode	*ip,
@@ -365,12 +381,9 @@ xfs_inode_to_log_dinode(
 
 	memset(to->di_pad, 0, sizeof(to->di_pad));
 	memset(to->di_pad3, 0, sizeof(to->di_pad3));
-	to->di_atime.t_sec = inode->i_atime.tv_sec;
-	to->di_atime.t_nsec = inode->i_atime.tv_nsec;
-	to->di_mtime.t_sec = inode->i_mtime.tv_sec;
-	to->di_mtime.t_nsec = inode->i_mtime.tv_nsec;
-	to->di_ctime.t_sec = inode->i_ctime.tv_sec;
-	to->di_ctime.t_nsec = inode->i_ctime.tv_nsec;
+	xfs_inode_to_log_dinode_ts(&to->di_atime, &inode->i_atime);
+	xfs_inode_to_log_dinode_ts(&to->di_mtime, &inode->i_mtime);
+	xfs_inode_to_log_dinode_ts(&to->di_ctime, &inode->i_ctime);
 	to->di_nlink = inode->i_nlink;
 	to->di_gen = inode->i_generation;
 	to->di_mode = inode->i_mode;
@@ -392,8 +405,7 @@ xfs_inode_to_log_dinode(
 	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
 		to->di_version = 3;
 		to->di_changecount = inode_peek_iversion(inode);
-		to->di_crtime.t_sec = from->di_crtime.tv_sec;
-		to->di_crtime.t_nsec = from->di_crtime.tv_nsec;
+		xfs_inode_to_log_dinode_ts(&to->di_crtime, &from->di_crtime);
 		to->di_flags2 = from->di_flags2;
 		to->di_cowextsize = from->di_cowextsize;
 		to->di_ino = ip->i_ino;

[PATCH 07/11] xfs: convert struct xfs_timestamp to union

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Convert the xfs_timestamp struct to a union so that we can overload it
in the next patch.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_format.h     |   16 +++++++++-------
 fs/xfs/libxfs/xfs_inode_buf.c  |    4 ++--
 fs/xfs/libxfs/xfs_inode_buf.h  |    4 ++--
 fs/xfs/libxfs/xfs_log_format.h |   16 +++++++++-------
 fs/xfs/scrub/inode.c           |    2 +-
 fs/xfs/xfs_inode_item.c        |    6 +++---
 fs/xfs/xfs_ondisk.h            |    4 ++--
 7 files changed, 28 insertions(+), 24 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 1f3a2be6c396..772113db41aa 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -856,9 +856,11 @@ struct xfs_agfl {
  * Inode timestamps consist of signed 32-bit counters for seconds and
  * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
  */
-struct xfs_timestamp {
-	__be32		t_sec;		/* timestamp seconds */
-	__be32		t_nsec;		/* timestamp nanoseconds */
+union xfs_timestamp {
+	struct {
+		__be32		t_sec;		/* timestamp seconds */
+		__be32		t_nsec;		/* timestamp nanoseconds */
+	};
 };
 
 /*
@@ -904,9 +906,9 @@ typedef struct xfs_dinode {
 	__be16		di_projid_hi;	/* higher part owner's project id */
 	__u8		di_pad[6];	/* unused, zeroed space */
 	__be16		di_flushiter;	/* incremented on flush */
-	struct xfs_timestamp di_atime;	/* time last accessed */
-	struct xfs_timestamp di_mtime;	/* time last modified */
-	struct xfs_timestamp di_ctime;	/* time created/inode modified */
+	union xfs_timestamp di_atime;	/* time last accessed */
+	union xfs_timestamp di_mtime;	/* time last modified */
+	union xfs_timestamp di_ctime;	/* time created/inode modified */
 	__be64		di_size;	/* number of bytes in file */
 	__be64		di_nblocks;	/* # of direct & btree blocks used */
 	__be32		di_extsize;	/* basic/minimum extent size for file */
@@ -931,7 +933,7 @@ typedef struct xfs_dinode {
 	__u8		di_pad2[12];	/* more padding for future expansion */
 
 	/* fields only written to during inode creation */
-	struct xfs_timestamp di_crtime;	/* time created */
+	union xfs_timestamp di_crtime;	/* time created */
 	__be64		di_ino;		/* inode number */
 	uuid_t		di_uuid;	/* UUID of the filesystem */
 
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 4930eabed6d8..cc1316a5fe0c 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -160,7 +160,7 @@ xfs_imap_to_bp(
 void
 xfs_inode_from_disk_timestamp(
 	struct timespec64		*tv,
-	const struct xfs_timestamp	*ts)
+	const union xfs_timestamp	*ts)
 {
 	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
 	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
@@ -259,7 +259,7 @@ xfs_inode_from_disk(
 
 void
 xfs_inode_to_disk_timestamp(
-	struct xfs_timestamp		*ts,
+	union xfs_timestamp		*ts,
 	const struct timespec64		*tv)
 {
 	ts->t_sec = cpu_to_be32(tv->tv_sec);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 9c63f3f932d7..f6160033fcbd 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -59,8 +59,8 @@ xfs_failaddr_t xfs_inode_validate_cowextsize(struct xfs_mount *mp,
 		uint64_t flags2);
 
 void xfs_inode_from_disk_timestamp(struct timespec64 *tv,
-		const struct xfs_timestamp *ts);
-void xfs_inode_to_disk_timestamp(struct xfs_timestamp *ts,
+		const union xfs_timestamp *ts);
+void xfs_inode_to_disk_timestamp(union xfs_timestamp *ts,
 		const struct timespec64 *tv);
 
 #endif	/* __XFS_INODE_BUF_H__ */
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index f2fac9bea66d..17c83d29998c 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -368,9 +368,11 @@ static inline int xfs_ilog_fdata(int w)
  * directly mirrors the xfs_dinode structure as it must contain all the same
  * information.
  */
-struct xfs_ictimestamp {
-	int32_t		t_sec;		/* timestamp seconds */
-	int32_t		t_nsec;		/* timestamp nanoseconds */
+union xfs_ictimestamp {
+	struct {
+		int32_t		t_sec;		/* timestamp seconds */
+		int32_t		t_nsec;		/* timestamp nanoseconds */
+	};
 };
 
 /*
@@ -390,9 +392,9 @@ struct xfs_log_dinode {
 	uint16_t	di_projid_hi;	/* higher part of owner's project id */
 	uint8_t		di_pad[6];	/* unused, zeroed space */
 	uint16_t	di_flushiter;	/* incremented on flush */
-	struct xfs_ictimestamp di_atime;	/* time last accessed */
-	struct xfs_ictimestamp di_mtime;	/* time last modified */
-	struct xfs_ictimestamp di_ctime;	/* time created/inode modified */
+	union xfs_ictimestamp di_atime;	/* time last accessed */
+	union xfs_ictimestamp di_mtime;	/* time last modified */
+	union xfs_ictimestamp di_ctime;	/* time created/inode modified */
 	xfs_fsize_t	di_size;	/* number of bytes in file */
 	xfs_rfsblock_t	di_nblocks;	/* # of direct & btree blocks used */
 	xfs_extlen_t	di_extsize;	/* basic/minimum extent size for file */
@@ -417,7 +419,7 @@ struct xfs_log_dinode {
 	uint8_t		di_pad2[12];	/* more padding for future expansion */
 
 	/* fields only written to during inode creation */
-	struct xfs_ictimestamp di_crtime;	/* time created */
+	union xfs_ictimestamp di_crtime;	/* time created */
 	xfs_ino_t	di_ino;		/* inode number */
 	uuid_t		di_uuid;	/* UUID of the filesystem */
 
diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index ccb5c217c0ee..9f036053fdb7 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -199,7 +199,7 @@ static inline void
 xchk_dinode_nsec(
 	struct xfs_scrub		*sc,
 	xfs_ino_t			ino,
-	const struct xfs_timestamp	*ts)
+	const union xfs_timestamp	*ts)
 {
 	struct timespec64		tv;
 
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index dac0ab59e88f..cec6d4d82bc4 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -298,8 +298,8 @@ xfs_inode_item_format_attr_fork(
 /* Write a log_dinode timestamp into an ondisk inode timestamp. */
 static inline void
 xfs_log_dinode_to_disk_ts(
-	struct xfs_timestamp		*ts,
-	const struct xfs_ictimestamp	*its)
+	union xfs_timestamp		*ts,
+	const union xfs_ictimestamp	*its)
 {
 	ts->t_sec = cpu_to_be32(its->t_sec);
 	ts->t_nsec = cpu_to_be32(its->t_nsec);
@@ -356,7 +356,7 @@ xfs_log_dinode_to_disk(
 /* Write an incore inode timestamp into a log_dinode timestamp. */
 static inline void
 xfs_inode_to_log_dinode_ts(
-	struct xfs_ictimestamp		*its,
+	union xfs_ictimestamp		*its,
 	const struct timespec64		*ts)
 {
 	its->t_sec = ts->tv_sec;
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 498e9063c605..7158a8de719f 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -57,7 +57,7 @@ xfs_check_ondisk_structs(void)
 	XFS_CHECK_STRUCT_SIZE(struct xfs_refcount_rec,		12);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_rmap_key,		20);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_rmap_rec,		24);
-	XFS_CHECK_STRUCT_SIZE(struct xfs_timestamp,		8);
+	XFS_CHECK_STRUCT_SIZE(union xfs_timestamp,		8);
 	XFS_CHECK_STRUCT_SIZE(xfs_alloc_key_t,			8);
 	XFS_CHECK_STRUCT_SIZE(xfs_alloc_ptr_t,			4);
 	XFS_CHECK_STRUCT_SIZE(xfs_alloc_rec_t,			8);
@@ -137,7 +137,7 @@ xfs_check_ondisk_structs(void)
 	XFS_CHECK_STRUCT_SIZE(struct xfs_extent_64,		16);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_log_dinode,		176);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_icreate_log,		28);
-	XFS_CHECK_STRUCT_SIZE(struct xfs_ictimestamp,		8);
+	XFS_CHECK_STRUCT_SIZE(union xfs_ictimestamp,		8);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format_32,	52);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format,	56);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_qoff_logformat,	20);

[PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Redesign the ondisk timestamps to be a simple unsigned 64-bit counter of
nanoseconds since 14 Dec 1901 (i.e. the minimum time in the 32-bit unix
time epoch).  This enables us to handle dates up to 2486, which solves
the y2038 problem.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_format.h      |   38 +++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_fs.h          |    1 +
 fs/xfs/libxfs/xfs_inode_buf.c   |   54 +++++++++++++++++++++++++++++++++------
 fs/xfs/libxfs/xfs_inode_buf.h   |    8 +++---
 fs/xfs/libxfs/xfs_log_format.h  |    3 ++
 fs/xfs/libxfs/xfs_sb.c          |    2 +
 fs/xfs/libxfs/xfs_trans_inode.c |   11 ++++++++
 fs/xfs/scrub/inode.c            |   16 ++++++++----
 fs/xfs/xfs_inode.c              |   15 ++++++++++-
 fs/xfs/xfs_inode_item.c         |   33 ++++++++++++++++++------
 fs/xfs/xfs_ioctl.c              |    3 +-
 fs/xfs/xfs_ondisk.h             |    6 ++++
 fs/xfs/xfs_super.c              |   13 ++++++++-
 13 files changed, 173 insertions(+), 30 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 772113db41aa..57343973e5e5 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -467,6 +467,7 @@ xfs_sb_has_ro_compat_feature(
 #define XFS_SB_FEAT_INCOMPAT_FTYPE	(1 << 0)	/* filetype in dirent */
 #define XFS_SB_FEAT_INCOMPAT_SPINODES	(1 << 1)	/* sparse inode chunks */
 #define XFS_SB_FEAT_INCOMPAT_META_UUID	(1 << 2)	/* metadata UUID */
+#define XFS_SB_FEAT_INCOMPAT_BIGTIME	(1 << 3)	/* large timestamps */
 #define XFS_SB_FEAT_INCOMPAT_ALL \
 		(XFS_SB_FEAT_INCOMPAT_FTYPE|	\
 		 XFS_SB_FEAT_INCOMPAT_SPINODES|	\
@@ -565,6 +566,12 @@ static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
 		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_REFLINK);
 }
 
+static inline bool xfs_sb_version_hasbigtime(struct xfs_sb *sbp)
+{
+	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+		(sbp->sb_features_incompat & XFS_SB_FEAT_INCOMPAT_BIGTIME);
+}
+
 /*
  * Inode btree block counter.  We record the number of inobt and finobt blocks
  * in the AGI header so that we can skip the finobt walk at mount time when
@@ -855,12 +862,18 @@ struct xfs_agfl {
  *
  * Inode timestamps consist of signed 32-bit counters for seconds and
  * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
+ *
+ * When bigtime is enabled, timestamps become an unsigned 64-bit nanoseconds
+ * counter.  Time zero is the start of the classic timestamp range.
  */
 union xfs_timestamp {
 	struct {
 		__be32		t_sec;		/* timestamp seconds */
 		__be32		t_nsec;		/* timestamp nanoseconds */
 	};
+
+	/* Nanoseconds since the bigtime epoch. */
+	__be64			t_bigtime;
 };
 
 /*
@@ -875,6 +888,25 @@ union xfs_timestamp {
  */
 #define XFS_INO_TIME_MAX	((int64_t)S32_MAX)
 
+/*
+ * Number of seconds between the start of the bigtime timestamp range and the
+ * start of the Unix epoch.
+ */
+#define XFS_INO_BIGTIME_EPOCH	(-XFS_INO_TIME_MIN)
+
+/*
+ * Smallest possible timestamp with big timestamps, which is
+ * Dec 13 20:45:52 UTC 1901.
+ */
+#define XFS_INO_BIGTIME_MIN	(XFS_INO_TIME_MIN)
+
+/*
+ * Largest possible timestamp with big timestamps, which is
+ * Jul  2 20:20:25 UTC 2486.
+ */
+#define XFS_INO_BIGTIME_MAX	((int64_t)((-1ULL / NSEC_PER_SEC) - \
+					   XFS_INO_BIGTIME_EPOCH))
+
 /*
  * On-disk inode structure.
  *
@@ -1100,12 +1132,16 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
 #define XFS_DIFLAG2_DAX_BIT	0	/* use DAX for this inode */
 #define XFS_DIFLAG2_REFLINK_BIT	1	/* file's blocks may be shared */
 #define XFS_DIFLAG2_COWEXTSIZE_BIT   2  /* copy on write extent size hint */
+#define XFS_DIFLAG2_BIGTIME_BIT	3	/* big timestamps */
+
 #define XFS_DIFLAG2_DAX		(1 << XFS_DIFLAG2_DAX_BIT)
 #define XFS_DIFLAG2_REFLINK     (1 << XFS_DIFLAG2_REFLINK_BIT)
 #define XFS_DIFLAG2_COWEXTSIZE  (1 << XFS_DIFLAG2_COWEXTSIZE_BIT)
+#define XFS_DIFLAG2_BIGTIME	(1 << XFS_DIFLAG2_BIGTIME_BIT)
 
 #define XFS_DIFLAG2_ANY \
-	(XFS_DIFLAG2_DAX | XFS_DIFLAG2_REFLINK | XFS_DIFLAG2_COWEXTSIZE)
+	(XFS_DIFLAG2_DAX | XFS_DIFLAG2_REFLINK | XFS_DIFLAG2_COWEXTSIZE | \
+	 XFS_DIFLAG2_BIGTIME)
 
 /*
  * Inode number format:
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 84bcffa87753..2a2e3cfd94f0 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -249,6 +249,7 @@ typedef struct xfs_fsop_resblks {
 #define XFS_FSOP_GEOM_FLAGS_SPINODES	(1 << 18) /* sparse inode chunks   */
 #define XFS_FSOP_GEOM_FLAGS_RMAPBT	(1 << 19) /* reverse mapping btree */
 #define XFS_FSOP_GEOM_FLAGS_REFLINK	(1 << 20) /* files can share blocks */
+#define XFS_FSOP_GEOM_FLAGS_BIGTIME	(1 << 21) /* 64-bit nsec timestamps */
 
 /*
  * Minimum and maximum sizes need for growth checks.
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index cc1316a5fe0c..c59ddb56bb90 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -157,11 +157,25 @@ xfs_imap_to_bp(
 	return 0;
 }
 
+/* Convert an ondisk timestamp into the 64-bit safe incore format. */
 void
 xfs_inode_from_disk_timestamp(
+	struct xfs_dinode		*dip,
 	struct timespec64		*tv,
 	const union xfs_timestamp	*ts)
 {
+	if (dip->di_version >= 3 &&
+	    (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) {
+		uint64_t		t = be64_to_cpu(ts->t_bigtime);
+		uint64_t		s;
+		uint32_t		n;
+
+		s = div_u64_rem(t, NSEC_PER_SEC, &n);
+		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
+		tv->tv_nsec = n;
+		return;
+	}
+
 	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
 	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
 }
@@ -220,9 +234,9 @@ xfs_inode_from_disk(
 	 * a time before epoch is converted to a time long after epoch
 	 * on 64 bit systems.
 	 */
-	xfs_inode_from_disk_timestamp(&inode->i_atime, &from->di_atime);
-	xfs_inode_from_disk_timestamp(&inode->i_mtime, &from->di_mtime);
-	xfs_inode_from_disk_timestamp(&inode->i_ctime, &from->di_ctime);
+	xfs_inode_from_disk_timestamp(from, &inode->i_atime, &from->di_atime);
+	xfs_inode_from_disk_timestamp(from, &inode->i_mtime, &from->di_mtime);
+	xfs_inode_from_disk_timestamp(from, &inode->i_ctime, &from->di_ctime);
 
 	to->di_size = be64_to_cpu(from->di_size);
 	to->di_nblocks = be64_to_cpu(from->di_nblocks);
@@ -235,9 +249,17 @@ xfs_inode_from_disk(
 	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
 		inode_set_iversion_queried(inode,
 					   be64_to_cpu(from->di_changecount));
-		xfs_inode_from_disk_timestamp(&to->di_crtime, &from->di_crtime);
+		xfs_inode_from_disk_timestamp(from, &to->di_crtime,
+				&from->di_crtime);
 		to->di_flags2 = be64_to_cpu(from->di_flags2);
 		to->di_cowextsize = be32_to_cpu(from->di_cowextsize);
+		/*
+		 * Set the bigtime flag incore so that we automatically convert
+		 * this inode's ondisk timestamps to bigtime format the next
+		 * time we write the inode core to disk.
+		 */
+		if (xfs_sb_version_hasbigtime(&ip->i_mount->m_sb))
+			to->di_flags2 |= XFS_DIFLAG2_BIGTIME;
 	}
 
 	error = xfs_iformat_data_fork(ip, from);
@@ -259,9 +281,19 @@ xfs_inode_from_disk(
 
 void
 xfs_inode_to_disk_timestamp(
+	struct xfs_icdinode		*from,
 	union xfs_timestamp		*ts,
 	const struct timespec64		*tv)
 {
+	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {
+		uint64_t		t;
+
+		t = (uint64_t)(tv->tv_sec + XFS_INO_BIGTIME_EPOCH);
+		t *= NSEC_PER_SEC;
+		ts->t_bigtime = cpu_to_be64(t + tv->tv_nsec);
+		return;
+	}
+
 	ts->t_sec = cpu_to_be32(tv->tv_sec);
 	ts->t_nsec = cpu_to_be32(tv->tv_nsec);
 }
@@ -285,9 +317,9 @@ xfs_inode_to_disk(
 	to->di_projid_hi = cpu_to_be16(from->di_projid >> 16);
 
 	memset(to->di_pad, 0, sizeof(to->di_pad));
-	xfs_inode_to_disk_timestamp(&to->di_atime, &inode->i_atime);
-	xfs_inode_to_disk_timestamp(&to->di_mtime, &inode->i_mtime);
-	xfs_inode_to_disk_timestamp(&to->di_ctime, &inode->i_ctime);
+	xfs_inode_to_disk_timestamp(from, &to->di_atime, &inode->i_atime);
+	xfs_inode_to_disk_timestamp(from, &to->di_mtime, &inode->i_mtime);
+	xfs_inode_to_disk_timestamp(from, &to->di_ctime, &inode->i_ctime);
 	to->di_nlink = cpu_to_be32(inode->i_nlink);
 	to->di_gen = cpu_to_be32(inode->i_generation);
 	to->di_mode = cpu_to_be16(inode->i_mode);
@@ -306,7 +338,8 @@ xfs_inode_to_disk(
 	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
 		to->di_version = 3;
 		to->di_changecount = cpu_to_be64(inode_peek_iversion(inode));
-		xfs_inode_to_disk_timestamp(&to->di_crtime, &from->di_crtime);
+		xfs_inode_to_disk_timestamp(from, &to->di_crtime,
+				&from->di_crtime);
 		to->di_flags2 = cpu_to_be64(from->di_flags2);
 		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
 		to->di_ino = cpu_to_be64(ip->i_ino);
@@ -526,6 +559,11 @@ xfs_dinode_verify(
 	if (fa)
 		return fa;
 
+	/* bigtime iflag can only happen on bigtime filesystems */
+	if ((flags2 & (XFS_DIFLAG2_BIGTIME)) &&
+	    !xfs_sb_version_hasbigtime(&mp->m_sb))
+		return __this_address;
+
 	return NULL;
 }
 
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index f6160033fcbd..3b25e43eafa1 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -58,9 +58,9 @@ xfs_failaddr_t xfs_inode_validate_cowextsize(struct xfs_mount *mp,
 		uint32_t cowextsize, uint16_t mode, uint16_t flags,
 		uint64_t flags2);
 
-void xfs_inode_from_disk_timestamp(struct timespec64 *tv,
-		const union xfs_timestamp *ts);
-void xfs_inode_to_disk_timestamp(union xfs_timestamp *ts,
-		const struct timespec64 *tv);
+void xfs_inode_from_disk_timestamp(struct xfs_dinode *dip,
+		struct timespec64 *tv, const union xfs_timestamp *ts);
+void xfs_inode_to_disk_timestamp(struct xfs_icdinode *from,
+		union xfs_timestamp *ts, const struct timespec64 *tv);
 
 #endif	/* __XFS_INODE_BUF_H__ */
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index 17c83d29998c..569721f7f9e5 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -373,6 +373,9 @@ union xfs_ictimestamp {
 		int32_t		t_sec;		/* timestamp seconds */
 		int32_t		t_nsec;		/* timestamp nanoseconds */
 	};
+
+	/* Nanoseconds since the bigtime epoch. */
+	uint64_t		t_bigtime;
 };
 
 /*
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index ae9aaf1f34bf..d5d60cd1c2ea 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -1166,6 +1166,8 @@ xfs_fs_geometry(
 		geo->flags |= XFS_FSOP_GEOM_FLAGS_RMAPBT;
 	if (xfs_sb_version_hasreflink(sbp))
 		geo->flags |= XFS_FSOP_GEOM_FLAGS_REFLINK;
+	if (xfs_sb_version_hasbigtime(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_BIGTIME;
 	if (xfs_sb_version_hassector(sbp))
 		geo->logsectsize = sbp->sb_logsectsize;
 	else
diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
index e15129647e00..71aca4587715 100644
--- a/fs/xfs/libxfs/xfs_trans_inode.c
+++ b/fs/xfs/libxfs/xfs_trans_inode.c
@@ -131,6 +131,17 @@ xfs_trans_log_inode(
 			iversion_flags = XFS_ILOG_CORE;
 	}
 
+	/*
+	 * If we're updating the inode core or the timestamps and it's possible
+	 * to upgrade this inode to bigtime format, do so now.
+	 */
+	if ((flags & (XFS_ILOG_CORE | XFS_ILOG_TIMESTAMP)) &&
+	    xfs_sb_version_hasbigtime(&tp->t_mountp->m_sb) &&
+	    !(ip->i_d.di_flags2 & XFS_DIFLAG2_BIGTIME)) {
+		ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
+		flags |= XFS_ILOG_CORE;
+	}
+
 	/*
 	 * Record the specific change for fdatasync optimisation. This allows
 	 * fdatasync to skip log forces for inodes that are only timestamp
diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index 9f036053fdb7..6f00309de2d4 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -190,6 +190,11 @@ xchk_inode_flags2(
 	if ((flags2 & XFS_DIFLAG2_DAX) && (flags2 & XFS_DIFLAG2_REFLINK))
 		goto bad;
 
+	/* no bigtime iflag without the bigtime feature */
+	if (!xfs_sb_version_hasbigtime(&mp->m_sb) &&
+	    (flags2 & XFS_DIFLAG2_BIGTIME))
+		goto bad;
+
 	return;
 bad:
 	xchk_ino_set_corrupt(sc, ino);
@@ -199,11 +204,12 @@ static inline void
 xchk_dinode_nsec(
 	struct xfs_scrub		*sc,
 	xfs_ino_t			ino,
+	struct xfs_dinode		*dip,
 	const union xfs_timestamp	*ts)
 {
 	struct timespec64		tv;
 
-	xfs_inode_from_disk_timestamp(&tv, ts);
+	xfs_inode_from_disk_timestamp(dip, &tv, ts);
 	if (tv.tv_nsec < 0 || tv.tv_nsec >= NSEC_PER_SEC)
 		xchk_ino_set_corrupt(sc, ino);
 }
@@ -306,9 +312,9 @@ xchk_dinode(
 	}
 
 	/* di_[amc]time.nsec */
-	xchk_dinode_nsec(sc, ino, &dip->di_atime);
-	xchk_dinode_nsec(sc, ino, &dip->di_mtime);
-	xchk_dinode_nsec(sc, ino, &dip->di_ctime);
+	xchk_dinode_nsec(sc, ino, dip, &dip->di_atime);
+	xchk_dinode_nsec(sc, ino, dip, &dip->di_mtime);
+	xchk_dinode_nsec(sc, ino, dip, &dip->di_ctime);
 
 	/*
 	 * di_size.  xfs_dinode_verify checks for things that screw up
@@ -413,7 +419,7 @@ xchk_dinode(
 	}
 
 	if (dip->di_version >= 3) {
-		xchk_dinode_nsec(sc, ino, &dip->di_crtime);
+		xchk_dinode_nsec(sc, ino, dip, &dip->di_crtime);
 		xchk_inode_flags2(sc, dip, ino, mode, flags, flags2);
 		xchk_inode_cowextsize(sc, dip, ino, mode, flags,
 				flags2);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c06129cffba9..bbc309bc70c4 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -841,6 +841,8 @@ xfs_ialloc(
 	if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
 		inode_set_iversion(inode, 1);
 		ip->i_d.di_flags2 = 0;
+		if (xfs_sb_version_hasbigtime(&mp->m_sb))
+			ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
 		ip->i_d.di_cowextsize = 0;
 		ip->i_d.di_crtime = tv;
 	}
@@ -2717,7 +2719,11 @@ xfs_ifree(
 
 	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
 	ip->i_d.di_flags = 0;
-	ip->i_d.di_flags2 = 0;
+	/*
+	 * Preserve the bigtime flag so that di_ctime accurately stores the
+	 * deletion time.
+	 */
+	ip->i_d.di_flags2 &= XFS_DIFLAG2_BIGTIME;
 	ip->i_d.di_dmevmask = 0;
 	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
 	ip->i_df.if_format = XFS_DINODE_FMT_EXTENTS;
@@ -3503,6 +3509,13 @@ xfs_iflush(
 			__func__, ip->i_ino, ip->i_d.di_forkoff, ip);
 		goto flush_out;
 	}
+	if (xfs_sb_version_hasbigtime(&mp->m_sb) &&
+	    !(ip->i_d.di_flags2 & XFS_DIFLAG2_BIGTIME)) {
+		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
+			"%s: bad inode %Lu, bigtime unset, ptr "PTR_FMT,
+			__func__, ip->i_ino, ip);
+		goto flush_out;
+	}
 
 	/*
 	 * Inode item log recovery for v2 inodes are dependent on the
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index cec6d4d82bc4..c38af3eea48f 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -298,9 +298,16 @@ xfs_inode_item_format_attr_fork(
 /* Write a log_dinode timestamp into an ondisk inode timestamp. */
 static inline void
 xfs_log_dinode_to_disk_ts(
+	struct xfs_log_dinode		*from,
 	union xfs_timestamp		*ts,
 	const union xfs_ictimestamp	*its)
 {
+	if (from->di_version >= 3 &&
+	    (from->di_flags2 & XFS_DIFLAG2_BIGTIME)) {
+		ts->t_bigtime = cpu_to_be64(its->t_bigtime);
+		return;
+	}
+
 	ts->t_sec = cpu_to_be32(its->t_sec);
 	ts->t_nsec = cpu_to_be32(its->t_nsec);
 }
@@ -322,9 +329,9 @@ xfs_log_dinode_to_disk(
 	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
 	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
 
-	xfs_log_dinode_to_disk_ts(&to->di_atime, &from->di_atime);
-	xfs_log_dinode_to_disk_ts(&to->di_mtime, &from->di_mtime);
-	xfs_log_dinode_to_disk_ts(&to->di_ctime, &from->di_ctime);
+	xfs_log_dinode_to_disk_ts(from, &to->di_atime, &from->di_atime);
+	xfs_log_dinode_to_disk_ts(from, &to->di_mtime, &from->di_mtime);
+	xfs_log_dinode_to_disk_ts(from, &to->di_ctime, &from->di_ctime);
 
 	to->di_size = cpu_to_be64(from->di_size);
 	to->di_nblocks = cpu_to_be64(from->di_nblocks);
@@ -340,7 +347,7 @@ xfs_log_dinode_to_disk(
 
 	if (from->di_version == 3) {
 		to->di_changecount = cpu_to_be64(from->di_changecount);
-		xfs_log_dinode_to_disk_ts(&to->di_crtime, &from->di_crtime);
+		xfs_log_dinode_to_disk_ts(from, &to->di_crtime, &from->di_crtime);
 		to->di_flags2 = cpu_to_be64(from->di_flags2);
 		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
 		to->di_ino = cpu_to_be64(from->di_ino);
@@ -356,9 +363,19 @@ xfs_log_dinode_to_disk(
 /* Write an incore inode timestamp into a log_dinode timestamp. */
 static inline void
 xfs_inode_to_log_dinode_ts(
+	struct xfs_icdinode		*from,
 	union xfs_ictimestamp		*its,
 	const struct timespec64		*ts)
 {
+	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {
+		uint64_t		t;
+
+		t = (uint64_t)(ts->tv_sec + XFS_INO_BIGTIME_EPOCH);
+		t *= NSEC_PER_SEC;
+		its->t_bigtime = t + ts->tv_nsec;
+		return;
+	}
+
 	its->t_sec = ts->tv_sec;
 	its->t_nsec = ts->tv_nsec;
 }
@@ -381,9 +398,9 @@ xfs_inode_to_log_dinode(
 
 	memset(to->di_pad, 0, sizeof(to->di_pad));
 	memset(to->di_pad3, 0, sizeof(to->di_pad3));
-	xfs_inode_to_log_dinode_ts(&to->di_atime, &inode->i_atime);
-	xfs_inode_to_log_dinode_ts(&to->di_mtime, &inode->i_mtime);
-	xfs_inode_to_log_dinode_ts(&to->di_ctime, &inode->i_ctime);
+	xfs_inode_to_log_dinode_ts(from, &to->di_atime, &inode->i_atime);
+	xfs_inode_to_log_dinode_ts(from, &to->di_mtime, &inode->i_mtime);
+	xfs_inode_to_log_dinode_ts(from, &to->di_ctime, &inode->i_ctime);
 	to->di_nlink = inode->i_nlink;
 	to->di_gen = inode->i_generation;
 	to->di_mode = inode->i_mode;
@@ -405,7 +422,7 @@ xfs_inode_to_log_dinode(
 	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
 		to->di_version = 3;
 		to->di_changecount = inode_peek_iversion(inode);
-		xfs_inode_to_log_dinode_ts(&to->di_crtime, &from->di_crtime);
+		xfs_inode_to_log_dinode_ts(from, &to->di_crtime, &from->di_crtime);
 		to->di_flags2 = from->di_flags2;
 		to->di_cowextsize = from->di_cowextsize;
 		to->di_ino = ip->i_ino;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 6f22a66777cd..13396c3665d1 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1190,7 +1190,8 @@ xfs_flags2diflags2(
 	unsigned int		xflags)
 {
 	uint64_t		di_flags2 =
-		(ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK);
+		(ip->i_d.di_flags2 & (XFS_DIFLAG2_REFLINK |
+				      XFS_DIFLAG2_BIGTIME));
 
 	if (xflags & FS_XFLAG_DAX)
 		di_flags2 |= XFS_DIFLAG2_DAX;
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 7158a8de719f..3e0c677cff15 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -25,6 +25,9 @@ xfs_check_limits(void)
 	/* make sure timestamp limits are correct */
 	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
 	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
+	XFS_CHECK_VALUE(XFS_INO_BIGTIME_EPOCH,			2147483648LL);
+	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MIN,			-2147483648LL);
+	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MAX,			16299260425LL);
 	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MIN,			1LL);
 	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MAX,			4294967295LL);
 	XFS_CHECK_VALUE(XFS_DQ_GRACE_MIN,			0LL);
@@ -58,6 +61,9 @@ xfs_check_ondisk_structs(void)
 	XFS_CHECK_STRUCT_SIZE(struct xfs_rmap_key,		20);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_rmap_rec,		24);
 	XFS_CHECK_STRUCT_SIZE(union xfs_timestamp,		8);
+	XFS_CHECK_OFFSET(union xfs_timestamp, t_bigtime,	0);
+	XFS_CHECK_OFFSET(union xfs_timestamp, t_sec,		0);
+	XFS_CHECK_OFFSET(union xfs_timestamp, t_nsec,		4);
 	XFS_CHECK_STRUCT_SIZE(xfs_alloc_key_t,			8);
 	XFS_CHECK_STRUCT_SIZE(xfs_alloc_ptr_t,			4);
 	XFS_CHECK_STRUCT_SIZE(xfs_alloc_rec_t,			8);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 375f05a47ba4..b19ae052f7a3 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1484,8 +1484,13 @@ xfs_fc_fill_super(
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_max_links = XFS_MAXLINK;
 	sb->s_time_gran = 1;
-	sb->s_time_min = XFS_INO_TIME_MIN;
-	sb->s_time_max = XFS_INO_TIME_MAX;
+	if (xfs_sb_version_hasbigtime(&mp->m_sb)) {
+		sb->s_time_min = XFS_INO_BIGTIME_MIN;
+		sb->s_time_max = XFS_INO_BIGTIME_MAX;
+	} else {
+		sb->s_time_min = XFS_INO_TIME_MIN;
+		sb->s_time_max = XFS_INO_TIME_MAX;
+	}
 	sb->s_iflags |= SB_I_CGROUPWB;
 
 	set_posix_acl_flag(sb);
@@ -1494,6 +1499,10 @@ xfs_fc_fill_super(
 	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
 		sb->s_flags |= SB_I_VERSION;
 
+	if (xfs_sb_version_hasbigtime(&mp->m_sb))
+		xfs_warn(mp,
+ "EXPERIMENTAL big timestamp feature in use. Use at your own risk!");
+
 	if (mp->m_flags & XFS_MOUNT_DAX_ALWAYS) {
 		bool rtdev_is_dax = false, datadev_is_dax;
 

[PATCH 09/11] xfs: refactor quota timestamp coding

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor quota timestamp encoding and decoding into helper functions so
that we can add extra behavior in the next patch.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c  |   20 ++++++++++++++++++++
 fs/xfs/libxfs/xfs_quota_defs.h |    6 ++++++
 fs/xfs/xfs_dquot.c             |   12 ++++++------
 3 files changed, 32 insertions(+), 6 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 5a2db00b9d5f..7f5291022b11 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -288,3 +288,23 @@ const struct xfs_buf_ops xfs_dquot_buf_ra_ops = {
 	.verify_read = xfs_dquot_buf_readahead_verify,
 	.verify_write = xfs_dquot_buf_write_verify,
 };
+
+/* Convert an on-disk timer value into an incore timer value. */
+void
+xfs_dquot_from_disk_timestamp(
+	struct xfs_disk_dquot	*ddq,
+	time64_t		*timer,
+	__be32			dtimer)
+{
+	*timer = be32_to_cpu(dtimer);
+}
+
+/* Convert an incore timer value into an on-disk timer value. */
+void
+xfs_dquot_to_disk_timestamp(
+	struct xfs_dquot	*dqp,
+	__be32			*dtimer,
+	time64_t		timer)
+{
+	*dtimer = cpu_to_be32(timer);
+}
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index 076bdc7037ee..b524059faab5 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -143,4 +143,10 @@ extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
 extern void xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
 		xfs_dqid_t id, xfs_dqtype_t type);
 
+struct xfs_dquot;
+void xfs_dquot_from_disk_timestamp(struct xfs_disk_dquot *ddq,
+		time64_t *timer, __be32 dtimer);
+void xfs_dquot_to_disk_timestamp(struct xfs_dquot *ddq,
+		__be32 *dtimer, time64_t timer);
+
 #endif	/* __XFS_QUOTA_H__ */
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index ed3fa6ada0d3..08d497d413b9 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -534,9 +534,9 @@ xfs_dquot_from_disk(
 	dqp->q_ino.warnings = be16_to_cpu(ddqp->d_iwarns);
 	dqp->q_rtb.warnings = be16_to_cpu(ddqp->d_rtbwarns);
 
-	dqp->q_blk.timer = be32_to_cpu(ddqp->d_btimer);
-	dqp->q_ino.timer = be32_to_cpu(ddqp->d_itimer);
-	dqp->q_rtb.timer = be32_to_cpu(ddqp->d_rtbtimer);
+	xfs_dquot_from_disk_timestamp(ddqp, &dqp->q_blk.timer, ddqp->d_btimer);
+	xfs_dquot_from_disk_timestamp(ddqp, &dqp->q_ino.timer, ddqp->d_itimer);
+	xfs_dquot_from_disk_timestamp(ddqp, &dqp->q_rtb.timer, ddqp->d_rtbtimer);
 
 	/*
 	 * Reservation counters are defined as reservation plus current usage
@@ -579,9 +579,9 @@ xfs_dquot_to_disk(
 	ddqp->d_iwarns = cpu_to_be16(dqp->q_ino.warnings);
 	ddqp->d_rtbwarns = cpu_to_be16(dqp->q_rtb.warnings);
 
-	ddqp->d_btimer = cpu_to_be32(dqp->q_blk.timer);
-	ddqp->d_itimer = cpu_to_be32(dqp->q_ino.timer);
-	ddqp->d_rtbtimer = cpu_to_be32(dqp->q_rtb.timer);
+	xfs_dquot_to_disk_timestamp(dqp, &ddqp->d_btimer, dqp->q_blk.timer);
+	xfs_dquot_to_disk_timestamp(dqp, &ddqp->d_itimer, dqp->q_ino.timer);
+	xfs_dquot_to_disk_timestamp(dqp, &ddqp->d_rtbtimer, dqp->q_rtb.timer);
 }
 
 /* Allocate and initialize the dquot buffer for this in-core dquot. */

[PATCH 10/11] xfs: enable bigtime for quota timers

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Enable the bigtime feature for quota timers.  We decrease the accuracy
of the timers to ~4s in exchange for being able to set timers up to the
bigtime maximum.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c  |   32 ++++++++++++++++++++++++++++++--
 fs/xfs/libxfs/xfs_format.h     |   27 ++++++++++++++++++++++++++-
 fs/xfs/libxfs/xfs_quota_defs.h |    3 ++-
 fs/xfs/xfs_dquot.c             |   25 +++++++++++++++++++++----
 fs/xfs/xfs_dquot.h             |    3 ++-
 fs/xfs/xfs_ondisk.h            |    7 +++++++
 fs/xfs/xfs_qm.c                |    2 ++
 fs/xfs/xfs_qm_syscalls.c       |    9 +++++----
 fs/xfs/xfs_trans_dquot.c       |    6 ++++++
 9 files changed, 101 insertions(+), 13 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 7f5291022b11..f5997fbdd308 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -69,6 +69,13 @@ xfs_dquot_verify(
 	    ddq_type != XFS_DQTYPE_GROUP)
 		return __this_address;
 
+	if ((ddq->d_type & XFS_DQTYPE_BIGTIME) &&
+	    !xfs_sb_version_hasbigtime(&mp->m_sb))
+		return __this_address;
+
+	if ((ddq->d_type & XFS_DQTYPE_BIGTIME) && !ddq->d_id)
+		return __this_address;
+
 	if (id != -1 && id != be32_to_cpu(ddq->d_id))
 		return __this_address;
 
@@ -296,7 +303,15 @@ xfs_dquot_from_disk_timestamp(
 	time64_t		*timer,
 	__be32			dtimer)
 {
-	*timer = be32_to_cpu(dtimer);
+	uint64_t		t;
+
+	if (!timer || !(ddq->d_type & XFS_DQTYPE_BIGTIME)) {
+		*timer = be32_to_cpu(dtimer);
+		return;
+	}
+
+	t = be32_to_cpu(dtimer);
+	*timer = t << XFS_DQ_BIGTIME_SHIFT;
 }
 
 /* Convert an incore timer value into an on-disk timer value. */
@@ -306,5 +321,18 @@ xfs_dquot_to_disk_timestamp(
 	__be32			*dtimer,
 	time64_t		timer)
 {
-	*dtimer = cpu_to_be32(timer);
+	uint64_t		t;
+
+	if (!timer || !(dqp->q_type & XFS_DQTYPE_BIGTIME)) {
+		*dtimer = cpu_to_be32(timer);
+		return;
+	}
+
+	/*
+	 * Round the end of the grace period up to the nearest bigtime
+	 * interval that we support, to give users the most time to fix
+	 * the problems.
+	 */
+	t = timer + XFS_DQ_BIGTIME_SLACK;
+	*dtimer = cpu_to_be32(t >> XFS_DQ_BIGTIME_SHIFT);
 }
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 57343973e5e5..7ce0fb3bd4d1 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1227,13 +1227,15 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
 #define XFS_DQTYPE_USER		0x01		/* user dquot record */
 #define XFS_DQTYPE_PROJ		0x02		/* project dquot record */
 #define XFS_DQTYPE_GROUP	0x04		/* group dquot record */
+#define XFS_DQTYPE_BIGTIME	0x08		/* large expiry timestamps */
 
 /* bitmask to determine if this is a user/group/project dquot */
 #define XFS_DQTYPE_REC_MASK	(XFS_DQTYPE_USER | \
 				 XFS_DQTYPE_PROJ | \
 				 XFS_DQTYPE_GROUP)
 
-#define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK)
+#define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK | \
+				 XFS_DQTYPE_BIGTIME)
 
 /*
  * XFS Quota Timers
@@ -1270,6 +1272,29 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
 #define XFS_DQ_GRACE_MIN	((int64_t)0)
 #define XFS_DQ_GRACE_MAX	((int64_t)U32_MAX)
 
+/*
+ * When bigtime is enabled, we trade a few bits of precision to expand the
+ * expiration timeout range to match that of big inode timestamps.  The grace
+ * periods stored in dquot 0 are not shifted, since they record an interval,
+ * not a timestamp.
+ */
+#define XFS_DQ_BIGTIME_SHIFT	(2)
+#define XFS_DQ_BIGTIME_SLACK	((int64_t)(1ULL << XFS_DQ_BIGTIME_SHIFT) - 1)
+
+/*
+ * Smallest possible quota expiration with big timestamps, which is
+ * Jan  1 00:00:01 UTC 1970.
+ */
+#define XFS_DQ_BIGTIMEOUT_MIN		(XFS_DQ_TIMEOUT_MIN)
+
+/*
+ * Largest supported quota expiration with traditional timestamps, which is
+ * the largest bigtime inode timestamp, or Jul  2 20:20:25 UTC 2486.  The field
+ * is large enough that it's possible to fit expirations up to 2514, but we
+ * want to keep the maximum timestamp in sync.
+ */
+#define XFS_DQ_BIGTIMEOUT_MAX		(XFS_INO_BIGTIME_MAX)
+
 /*
  * This is the main portion of the on-disk representation of quota information
  * for a user.  We pad this with some more expansion room to construct the on
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index b524059faab5..b9a47eae684b 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -23,7 +23,8 @@ typedef uint8_t		xfs_dqtype_t;
 #define XFS_DQTYPE_STRINGS \
 	{ XFS_DQTYPE_USER,	"USER" }, \
 	{ XFS_DQTYPE_PROJ,	"PROJ" }, \
-	{ XFS_DQTYPE_GROUP,	"GROUP" }
+	{ XFS_DQTYPE_GROUP,	"GROUP" }, \
+	{ XFS_DQTYPE_BIGTIME,	"BIGTIME" }
 
 /*
  * flags for q_flags field in the dquot.
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 08d497d413b9..6bc1f6e90c3e 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -110,9 +110,15 @@ xfs_dquot_set_grace_period(
 /* Set the expiration time of a quota's grace period. */
 void
 xfs_dquot_set_timeout(
+	struct xfs_mount	*mp,
 	time64_t		*timer,
 	time64_t		value)
 {
+	if (xfs_sb_version_hasbigtime(&mp->m_sb)) {
+		*timer = clamp_t(time64_t, value, XFS_DQ_BIGTIMEOUT_MIN,
+						  XFS_DQ_BIGTIMEOUT_MAX);
+		return;
+	}
 	*timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN,
 					  XFS_DQ_TIMEOUT_MAX);
 }
@@ -123,6 +129,7 @@ xfs_dquot_set_timeout(
  */
 static inline void
 xfs_qm_adjust_res_timer(
+	struct xfs_dquot	*dqp,
 	struct xfs_dquot_res	*res,
 	struct xfs_quota_limits	*qlim)
 {
@@ -131,7 +138,7 @@ xfs_qm_adjust_res_timer(
 	if ((res->softlimit && res->count > res->softlimit) ||
 	    (res->hardlimit && res->count > res->hardlimit)) {
 		if (res->timer == 0)
-			xfs_dquot_set_timeout(&res->timer,
+			xfs_dquot_set_timeout(dqp->q_mount, &res->timer,
 					ktime_get_real_seconds() + qlim->time);
 	} else {
 		if (res->timer == 0)
@@ -165,9 +172,9 @@ xfs_qm_adjust_dqtimers(
 	ASSERT(dq->q_id);
 	defq = xfs_get_defquota(qi, xfs_dquot_type(dq));
 
-	xfs_qm_adjust_res_timer(&dq->q_blk, &defq->blk);
-	xfs_qm_adjust_res_timer(&dq->q_ino, &defq->ino);
-	xfs_qm_adjust_res_timer(&dq->q_rtb, &defq->rtb);
+	xfs_qm_adjust_res_timer(dq, &dq->q_blk, &defq->blk);
+	xfs_qm_adjust_res_timer(dq, &dq->q_ino, &defq->ino);
+	xfs_qm_adjust_res_timer(dq, &dq->q_rtb, &defq->rtb);
 }
 
 /*
@@ -221,6 +228,8 @@ xfs_qm_init_dquot_blk(
 		d->dd_diskdq.d_version = XFS_DQUOT_VERSION;
 		d->dd_diskdq.d_id = cpu_to_be32(curid);
 		d->dd_diskdq.d_type = type;
+		if (curid > 0 && xfs_sb_version_hasbigtime(&mp->m_sb))
+			d->dd_diskdq.d_type |= XFS_DQTYPE_BIGTIME;
 		if (xfs_sb_version_hascrc(&mp->m_sb)) {
 			uuid_copy(&d->dd_uuid, &mp->m_sb.sb_meta_uuid);
 			xfs_update_cksum((char *)d, sizeof(struct xfs_dqblk),
@@ -1165,6 +1174,14 @@ xfs_qm_dqflush_check(
 	    !dqp->q_rtb.timer)
 		return __this_address;
 
+	/* bigtime flag should never be set on root dquots */
+	if (dqp->q_type & XFS_DQTYPE_BIGTIME) {
+		if (!xfs_sb_version_hasbigtime(&dqp->q_mount->m_sb))
+			return __this_address;
+		if (dqp->q_id == 0)
+			return __this_address;
+	}
+
 	return NULL;
 }
 
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 0ba4d91c3a11..74ca87e02a14 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -238,6 +238,7 @@ int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type,
 		xfs_qm_dqiterate_fn iter_fn, void *priv);
 
 void xfs_dquot_set_grace_period(time64_t *timer, time64_t limit);
-void xfs_dquot_set_timeout(time64_t *timer, time64_t limit);
+void xfs_dquot_set_timeout(struct xfs_mount *mp, time64_t *timer,
+		time64_t limit);
 
 #endif /* __XFS_DQUOT_H__ */
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 3e0c677cff15..cc12d6e1c89a 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -32,6 +32,13 @@ xfs_check_limits(void)
 	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MAX,			4294967295LL);
 	XFS_CHECK_VALUE(XFS_DQ_GRACE_MIN,			0LL);
 	XFS_CHECK_VALUE(XFS_DQ_GRACE_MAX,			4294967295LL);
+	XFS_CHECK_VALUE(XFS_DQ_BIGTIMEOUT_MIN,			1LL);
+	XFS_CHECK_VALUE(XFS_DQ_BIGTIMEOUT_MAX,			16299260425LL);
+
+	BUILD_BUG_ON_MSG((XFS_DQ_TIMEOUT_MAX << XFS_DQ_BIGTIME_SHIFT) <
+			 XFS_DQ_BIGTIMEOUT_MAX,
+			 "XFS: quota timeout field is not large enough to fit "
+			 "XFS_DQ_BIGTIMEOUT_MAX");
 }
 
 static inline void __init
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index be67570badf8..a5136e40e118 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -879,6 +879,8 @@ xfs_qm_reset_dqcounts(
 			ddq->d_bwarns = 0;
 			ddq->d_iwarns = 0;
 			ddq->d_rtbwarns = 0;
+			if (xfs_sb_version_hasbigtime(&mp->m_sb))
+				ddq->d_type |= XFS_DQTYPE_BIGTIME;
 		}
 
 		if (xfs_sb_version_hascrc(&mp->m_sb)) {
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 95b0c25b9969..4d9c245d65c2 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -479,6 +479,7 @@ xfs_setqlim_warns(
 
 static inline void
 xfs_setqlim_timer(
+	struct xfs_dquot	*dqp,
 	struct xfs_dquot_res	*res,
 	struct xfs_quota_limits	*qlim,
 	s64			timer)
@@ -489,7 +490,7 @@ xfs_setqlim_timer(
 		qlim->time = res->timer;
 	} else {
 		/* Set the grace period expiration on a quota. */
-		xfs_dquot_set_timeout(&res->timer, timer);
+		xfs_dquot_set_timeout(dqp->q_mount, &res->timer, timer);
 	}
 }
 
@@ -579,7 +580,7 @@ xfs_qm_scall_setqlim(
 	if (newlim->d_fieldmask & QC_SPC_WARNS)
 		xfs_setqlim_warns(res, qlim, newlim->d_spc_warns);
 	if (newlim->d_fieldmask & QC_SPC_TIMER)
-		xfs_setqlim_timer(res, qlim, newlim->d_spc_timer);
+		xfs_setqlim_timer(dqp, res, qlim, newlim->d_spc_timer);
 
 	/* Blocks on the realtime device. */
 	hard = (newlim->d_fieldmask & QC_RT_SPC_HARD) ?
@@ -595,7 +596,7 @@ xfs_qm_scall_setqlim(
 	if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
 		xfs_setqlim_warns(res, qlim, newlim->d_rt_spc_warns);
 	if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
-		xfs_setqlim_timer(res, qlim, newlim->d_rt_spc_timer);
+		xfs_setqlim_timer(dqp, res, qlim, newlim->d_rt_spc_timer);
 
 	/* Inodes */
 	hard = (newlim->d_fieldmask & QC_INO_HARD) ?
@@ -611,7 +612,7 @@ xfs_qm_scall_setqlim(
 	if (newlim->d_fieldmask & QC_INO_WARNS)
 		xfs_setqlim_warns(res, qlim, newlim->d_ino_warns);
 	if (newlim->d_fieldmask & QC_INO_TIMER)
-		xfs_setqlim_timer(res, qlim, newlim->d_ino_timer);
+		xfs_setqlim_timer(dqp, res, qlim, newlim->d_ino_timer);
 
 	if (id != 0) {
 		/*
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index c6ba7ef18e06..133fc6fc3edd 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -55,6 +55,12 @@ xfs_trans_log_dquot(
 {
 	ASSERT(XFS_DQ_IS_LOCKED(dqp));
 
+	/* Upgrade the dquot to bigtime format if possible. */
+	if (dqp->q_id != 0 &&
+	    xfs_sb_version_hasbigtime(&tp->t_mountp->m_sb) &&
+	    !(dqp->q_type & XFS_DQTYPE_BIGTIME))
+		dqp->q_type |= XFS_DQTYPE_BIGTIME;
+
 	tp->t_flags |= XFS_TRANS_DIRTY;
 	set_bit(XFS_LI_DIRTY, &dqp->q_logitem.qli_item.li_flags);
 }

[PATCH 11/11] xfs: enable big timestamps

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Enable the big timestamp feature.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_format.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 7ce0fb3bd4d1..3a8b9c12541a 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -471,7 +471,8 @@ xfs_sb_has_ro_compat_feature(
 #define XFS_SB_FEAT_INCOMPAT_ALL \
 		(XFS_SB_FEAT_INCOMPAT_FTYPE|	\
 		 XFS_SB_FEAT_INCOMPAT_SPINODES|	\
-		 XFS_SB_FEAT_INCOMPAT_META_UUID)
+		 XFS_SB_FEAT_INCOMPAT_META_UUID| \
+		 XFS_SB_FEAT_INCOMPAT_BIGTIME)
 
 #define XFS_SB_FEAT_INCOMPAT_UNKNOWN	~XFS_SB_FEAT_INCOMPAT_ALL
 static inline bool

Re: [PATCH 01/11] xfs: explicitly define inode timestamp range

[Christoph Hellwig]

> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index acb9b737fe6b..48a64fa49f91 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -15,6 +15,18 @@
>  		"XFS: offsetof(" #structname ", " #member ") is wrong, " \
>  		"expected " #off)
>  
> +#define XFS_CHECK_VALUE(value, expected) \
> +	BUILD_BUG_ON_MSG((value) != (expected), \
> +		"XFS: value of " #value " is wrong, expected " #expected)
> +
> +static inline void __init
> +xfs_check_limits(void)
> +{
> +	/* make sure timestamp limits are correct */
> +	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
> +	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);

I have to admit I don't get why you'd define a constant and then
check that it has the value you defined it to.  Seems a little cargo
cult.

Re: [PATCH 02/11] xfs: refactor quota expiration timer modification

[Christoph Hellwig]

On Thu, Aug 20, 2020 at 07:11:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Define explicit limits on the range of quota grace period expiration
> timeouts and refactor the code that modifies the timeouts into helpers
> that clamp the values appropriately.  Note that we'll deal with the
> grace period timer separately.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_format.h |   22 ++++++++++++++++++++++
>  fs/xfs/xfs_dquot.c         |   13 ++++++++++++-
>  fs/xfs/xfs_dquot.h         |    2 ++
>  fs/xfs/xfs_ondisk.h        |    2 ++
>  fs/xfs/xfs_qm_syscalls.c   |    9 +++++++--
>  5 files changed, 45 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index b1b8a5c05cea..ef36978239ac 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1197,6 +1197,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>  
>  #define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK)
>  
> +/*
> + * XFS Quota Timers
> + * ================
> + *
> + * Quota grace period expiration timers are an unsigned 32-bit seconds counter;
> + * time zero is the Unix epoch, Jan  1 00:00:01 UTC 1970.  An expiration value
> + * of zero means that the quota limit has not been reached, and therefore no
> + * expiration has been set.
> + */
> +
> +/*
> + * Smallest possible quota expiration with traditional timestamps, which is
> + * Jan  1 00:00:01 UTC 1970.
> + */
> +#define XFS_DQ_TIMEOUT_MIN	((int64_t)1)
> +
> +/*
> + * Largest possible quota expiration with traditional timestamps, which is
> + * Feb  7 06:28:15 UTC 2106.
> + */
> +#define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
> +
>  /*
>   * This is the main portion of the on-disk representation of quota information
>   * for a user.  We pad this with some more expansion room to construct the on
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index bcd73b9c2994..2425b1c30d11 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -98,6 +98,16 @@ xfs_qm_adjust_dqlimits(
>  		xfs_dquot_set_prealloc_limits(dq);
>  }
>  
> +/* Set the expiration time of a quota's grace period. */
> +void
> +xfs_dquot_set_timeout(
> +	time64_t		*timer,
> +	time64_t		value)
> +{
> +	*timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN,
> +					  XFS_DQ_TIMEOUT_MAX);
> +}

Why doesn't this just return the value?  That would seem like
a much more natural calling convention to me.

Re: [PATCH 03/11] xfs: refactor default quota grace period setting code

[Christoph Hellwig]

On Thu, Aug 20, 2020 at 07:11:47PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor the code that sets the default quota grace period into a helper
> function so that we can override the ondisk behavior later.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_format.h |   13 +++++++++++++
>  fs/xfs/xfs_dquot.c         |    9 +++++++++
>  fs/xfs/xfs_dquot.h         |    1 +
>  fs/xfs/xfs_ondisk.h        |    2 ++
>  fs/xfs/xfs_qm_syscalls.c   |    4 ++--
>  5 files changed, 27 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index ef36978239ac..e9e6248b35be 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1205,6 +1205,11 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>   * time zero is the Unix epoch, Jan  1 00:00:01 UTC 1970.  An expiration value
>   * of zero means that the quota limit has not been reached, and therefore no
>   * expiration has been set.
> + *
> + * The grace period for each quota type is stored in the root dquot (id = 0)
> + * and is applied to a non-root dquot when it exceeds the soft or hard limits.
> + * The length of quota grace periods are unsigned 32-bit quantities measured in
> + * units of seconds.  A value of zero means to use the default period.
>   */
>  
>  /*
> @@ -1219,6 +1224,14 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>   */
>  #define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
>  
> +/*
> + * Default quota grace periods, ranging from zero (use the compiled defaults)
> + * to ~136 years.  These are applied to a non-root dquot that has exceeded
> + * either limit.
> + */
> +#define XFS_DQ_GRACE_MIN	((int64_t)0)
> +#define XFS_DQ_GRACE_MAX	((int64_t)U32_MAX)
> +
>  /*
>   * This is the main portion of the on-disk representation of quota information
>   * for a user.  We pad this with some more expansion room to construct the on
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 2425b1c30d11..ed3fa6ada0d3 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -98,6 +98,15 @@ xfs_qm_adjust_dqlimits(
>  		xfs_dquot_set_prealloc_limits(dq);
>  }
>  
> +/* Set the length of the default grace period. */
> +void
> +xfs_dquot_set_grace_period(
> +	time64_t		*timer,
> +	time64_t		value)
> +{
> +	*timer = clamp_t(time64_t, value, XFS_DQ_GRACE_MIN, XFS_DQ_GRACE_MAX);
> +}

Why not return the value?

Re: [PATCH 04/11] xfs: remove xfs_timestamp_t

[Christoph Hellwig]

On Thu, Aug 20, 2020 at 07:11:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Kill this old typedef.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 05/11] xfs: move xfs_log_dinode_to_disk to the log code

[Christoph Hellwig]

On Thu, Aug 20, 2020 at 07:12:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move this function to xfs_inode_item.c to match the encoding function
> that's already there.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

An good reason to not move it towards its only caller in
fs/xfs/xfs_inode_item_recover.c?

Re: [PATCH 06/11] xfs: refactor inode timestamp coding

[Christoph Hellwig]

On Thu, Aug 20, 2020 at 07:12:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor inode timestamp encoding and decoding into helper functions so
> that we can add extra behaviors in subsequent patches.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 07/11] xfs: convert struct xfs_timestamp to union

[Christoph Hellwig]

On Thu, Aug 20, 2020 at 07:12:15PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Convert the xfs_timestamp struct to a union so that we can overload it
> in the next patch.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_format.h     |   16 +++++++++-------
>  fs/xfs/libxfs/xfs_inode_buf.c  |    4 ++--
>  fs/xfs/libxfs/xfs_inode_buf.h  |    4 ++--
>  fs/xfs/libxfs/xfs_log_format.h |   16 +++++++++-------
>  fs/xfs/scrub/inode.c           |    2 +-
>  fs/xfs/xfs_inode_item.c        |    6 +++---
>  fs/xfs/xfs_ondisk.h            |    4 ++--
>  7 files changed, 28 insertions(+), 24 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 1f3a2be6c396..772113db41aa 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -856,9 +856,11 @@ struct xfs_agfl {
>   * Inode timestamps consist of signed 32-bit counters for seconds and
>   * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
>   */
> -struct xfs_timestamp {
> -	__be32		t_sec;		/* timestamp seconds */
> -	__be32		t_nsec;		/* timestamp nanoseconds */
> +union xfs_timestamp {
> +	struct {
> +		__be32		t_sec;		/* timestamp seconds */
> +		__be32		t_nsec;		/* timestamp nanoseconds */
> +	};
>  };

Wouldn't it make sense to merge the typedef removal patch into this
one to avoid touching all the users twice?

Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Christoph Hellwig]

>   * in the AGI header so that we can skip the finobt walk at mount time when
> @@ -855,12 +862,18 @@ struct xfs_agfl {
>   *
>   * Inode timestamps consist of signed 32-bit counters for seconds and
>   * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
> + *
> + * When bigtime is enabled, timestamps become an unsigned 64-bit nanoseconds
> + * counter.  Time zero is the start of the classic timestamp range.
>   */
>  union xfs_timestamp {
>  	struct {
>  		__be32		t_sec;		/* timestamp seconds */
>  		__be32		t_nsec;		/* timestamp nanoseconds */
>  	};
> +
> +	/* Nanoseconds since the bigtime epoch. */
> +	__be64			t_bigtime;
>  };

So do we really need the union here?  What about:

 (1) keep the typedef instead of removing it
 (2) switch the typedef to be just a __be64, and use trivial helpers
     to extract the two separate legacy sec/nsec field
 (3) PROFIT!!!

> +/* Convert an ondisk timestamp into the 64-bit safe incore format. */
>  void
>  xfs_inode_from_disk_timestamp(
> +	struct xfs_dinode		*dip,
>  	struct timespec64		*tv,
>  	const union xfs_timestamp	*ts)

I think passing ts by value might lead to somewhat better code
generation on modern ABIs (and older ABIs just fall back to pass
by reference transparently).

>  {
> +	if (dip->di_version >= 3 &&
> +	    (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) {

Do we want a helper for this condition?

> +		uint64_t		t = be64_to_cpu(ts->t_bigtime);
> +		uint64_t		s;
> +		uint32_t		n;
> +
> +		s = div_u64_rem(t, NSEC_PER_SEC, &n);
> +		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
> +		tv->tv_nsec = n;
> +		return;
> +	}
> +
>  	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
>  	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);

Nit: for these kinds of symmetric conditions and if/else feels a little
more natural.

> +		xfs_log_dinode_to_disk_ts(from, &to->di_crtime, &from->di_crtime);

This adds a > 80 char line.

> +	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {
> +		uint64_t		t;
> +
> +		t = (uint64_t)(ts->tv_sec + XFS_INO_BIGTIME_EPOCH);
> +		t *= NSEC_PER_SEC;
> +		its->t_bigtime = t + ts->tv_nsec;

This calculation is dupliated in two places, might be worth
adding a little helper (which will need to get the sec/nsec values
passed separately due to the different structures).

> +		xfs_inode_to_log_dinode_ts(from, &to->di_crtime, &from->di_crtime);

Another line over 8 characters here.

> +	if (xfs_sb_version_hasbigtime(&mp->m_sb)) {
> +		sb->s_time_min = XFS_INO_BIGTIME_MIN;
> +		sb->s_time_max = XFS_INO_BIGTIME_MAX;
> +	} else {
> +		sb->s_time_min = XFS_INO_TIME_MIN;
> +		sb->s_time_max = XFS_INO_TIME_MAX;
> +	}

This is really a comment on the earlier patch, but maybe we should
name the old constants with "OLD" or "LEGACY" or "SMALL" in the name?

> @@ -1494,6 +1499,10 @@ xfs_fc_fill_super(
>  	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
>  		sb->s_flags |= SB_I_VERSION;
>  
> +	if (xfs_sb_version_hasbigtime(&mp->m_sb))
> +		xfs_warn(mp,
> + "EXPERIMENTAL big timestamp feature in use. Use at your own risk!");
> +

Is there any good reason to mark this experimental?

Re: [PATCH 09/11] xfs: refactor quota timestamp coding

[Christoph Hellwig]

> +/* Convert an on-disk timer value into an incore timer value. */
> +void
> +xfs_dquot_from_disk_timestamp(
> +	struct xfs_disk_dquot	*ddq,
> +	time64_t		*timer,
> +	__be32			dtimer)
> +{
> +	*timer = be32_to_cpu(dtimer);
> +}
> +
> +/* Convert an incore timer value into an on-disk timer value. */
> +void
> +xfs_dquot_to_disk_timestamp(
> +	struct xfs_dquot	*dqp,
> +	__be32			*dtimer,
> +	time64_t		timer)
> +{
> +	*dtimer = cpu_to_be32(timer);

Why not return the values?

Re: [PATCH 10/11] xfs: enable bigtime for quota timers

[Christoph Hellwig]

On Thu, Aug 20, 2020 at 07:12:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Enable the bigtime feature for quota timers.  We decrease the accuracy
> of the timers to ~4s in exchange for being able to set timers up to the
> bigtime maximum.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_dquot_buf.c  |   32 ++++++++++++++++++++++++++++++--
>  fs/xfs/libxfs/xfs_format.h     |   27 ++++++++++++++++++++++++++-
>  fs/xfs/libxfs/xfs_quota_defs.h |    3 ++-
>  fs/xfs/xfs_dquot.c             |   25 +++++++++++++++++++++----
>  fs/xfs/xfs_dquot.h             |    3 ++-
>  fs/xfs/xfs_ondisk.h            |    7 +++++++
>  fs/xfs/xfs_qm.c                |    2 ++
>  fs/xfs/xfs_qm_syscalls.c       |    9 +++++----
>  fs/xfs/xfs_trans_dquot.c       |    6 ++++++
>  9 files changed, 101 insertions(+), 13 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index 7f5291022b11..f5997fbdd308 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -69,6 +69,13 @@ xfs_dquot_verify(
>  	    ddq_type != XFS_DQTYPE_GROUP)
>  		return __this_address;
>  
> +	if ((ddq->d_type & XFS_DQTYPE_BIGTIME) &&
> +	    !xfs_sb_version_hasbigtime(&mp->m_sb))
> +		return __this_address;
> +
> +	if ((ddq->d_type & XFS_DQTYPE_BIGTIME) && !ddq->d_id)
> +		return __this_address;
> +
>  	if (id != -1 && id != be32_to_cpu(ddq->d_id))
>  		return __this_address;
>  
> @@ -296,7 +303,15 @@ xfs_dquot_from_disk_timestamp(
>  	time64_t		*timer,
>  	__be32			dtimer)
>  {
> -	*timer = be32_to_cpu(dtimer);
> +	uint64_t		t;
> +
> +	if (!timer || !(ddq->d_type & XFS_DQTYPE_BIGTIME)) {
> +		*timer = be32_to_cpu(dtimer);

I don't think setting *time makes any sense if time is NULL..

> +		return;
> +	}
> +
> +	t = be32_to_cpu(dtimer);
> +	*timer = t << XFS_DQ_BIGTIME_SHIFT;

Why not:

	*timer = (time64_t)be32_to_cpu(dtimer) << XFS_DQ_BIGTIME_SHIFT;

or (with my previous suggestion):

	return (time64_t)be32_to_cpu(dtimer) << XFS_DQ_BIGTIME_SHIFT;

?

> @@ -1227,13 +1227,15 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>  #define XFS_DQTYPE_USER		0x01		/* user dquot record */
>  #define XFS_DQTYPE_PROJ		0x02		/* project dquot record */
>  #define XFS_DQTYPE_GROUP	0x04		/* group dquot record */
> +#define XFS_DQTYPE_BIGTIME	0x08		/* large expiry timestamps */
>  
>  /* bitmask to determine if this is a user/group/project dquot */
>  #define XFS_DQTYPE_REC_MASK	(XFS_DQTYPE_USER | \
>  				 XFS_DQTYPE_PROJ | \
>  				 XFS_DQTYPE_GROUP)
>  
> -#define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK)
> +#define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK | \
> +				 XFS_DQTYPE_BIGTIME)
>  
>  /*
>   * XFS Quota Timers
> @@ -1270,6 +1272,29 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>  #define XFS_DQ_GRACE_MIN	((int64_t)0)
>  #define XFS_DQ_GRACE_MAX	((int64_t)U32_MAX)
>  
> +/*
> + * When bigtime is enabled, we trade a few bits of precision to expand the
> + * expiration timeout range to match that of big inode timestamps.  The grace
> + * periods stored in dquot 0 are not shifted, since they record an interval,
> + * not a timestamp.
> + */
> +#define XFS_DQ_BIGTIME_SHIFT	(2)
> +#define XFS_DQ_BIGTIME_SLACK	((int64_t)(1ULL << XFS_DQ_BIGTIME_SHIFT) - 1)
> +
> +/*
> + * Smallest possible quota expiration with big timestamps, which is
> + * Jan  1 00:00:01 UTC 1970.
> + */
> +#define XFS_DQ_BIGTIMEOUT_MIN		(XFS_DQ_TIMEOUT_MIN)
> +
> +/*
> + * Largest supported quota expiration with traditional timestamps, which is
> + * the largest bigtime inode timestamp, or Jul  2 20:20:25 UTC 2486.  The field

This adds and > 80 char line.

Re: [PATCH 01/11] xfs: explicitly define inode timestamp range

[Dave Chinner]

On Thu, Aug 20, 2020 at 07:11:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Formally define the inode timestamp ranges that existing filesystems
> support, and switch the vfs timetamp ranges to use it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_format.h |   19 +++++++++++++++++++
>  fs/xfs/xfs_ondisk.h        |   12 ++++++++++++
>  fs/xfs/xfs_super.c         |    5 +++--
>  3 files changed, 34 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index be86fa1a5556..b1b8a5c05cea 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -849,11 +849,30 @@ struct xfs_agfl {
>  	    ASSERT(xfs_daddr_to_agno(mp, d) == \
>  		   xfs_daddr_to_agno(mp, (d) + (len) - 1)))
>  
> +/*
> + * XFS Timestamps
> + * ==============
> + *
> + * Inode timestamps consist of signed 32-bit counters for seconds and
> + * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
> + */
>  typedef struct xfs_timestamp {
>  	__be32		t_sec;		/* timestamp seconds */
>  	__be32		t_nsec;		/* timestamp nanoseconds */
>  } xfs_timestamp_t;
>  
> +/*
> + * Smallest possible timestamp with traditional timestamps, which is
> + * Dec 13 20:45:52 UTC 1901.
> + */
> +#define XFS_INO_TIME_MIN	((int64_t)S32_MIN)
> +
> +/*
> + * Largest possible timestamp with traditional timestamps, which is
> + * Jan 19 03:14:07 UTC 2038.
> + */
> +#define XFS_INO_TIME_MAX	((int64_t)S32_MAX)

These are based on the Unix epoch. Can we call them something like
XFS_INO_UNIX_TIME_{MIN,MAX} to indicate what epoch they reference?

>  /*
>   * On-disk inode structure.
>   *
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index acb9b737fe6b..48a64fa49f91 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -15,6 +15,18 @@
>  		"XFS: offsetof(" #structname ", " #member ") is wrong, " \
>  		"expected " #off)
>  
> +#define XFS_CHECK_VALUE(value, expected) \
> +	BUILD_BUG_ON_MSG((value) != (expected), \
> +		"XFS: value of " #value " is wrong, expected " #expected)
> +
> +static inline void __init
> +xfs_check_limits(void)
> +{
> +	/* make sure timestamp limits are correct */
> +	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
> +	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> +}

Not sure this really gains us anything? All it does is check that
S32_MIN/S32_MAX haven't changed value....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 02/11] xfs: refactor quota expiration timer modification

[Dave Chinner]

On Thu, Aug 20, 2020 at 07:11:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Define explicit limits on the range of quota grace period expiration
> timeouts and refactor the code that modifies the timeouts into helpers
> that clamp the values appropriately.  Note that we'll deal with the
> grace period timer separately.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_format.h |   22 ++++++++++++++++++++++
>  fs/xfs/xfs_dquot.c         |   13 ++++++++++++-
>  fs/xfs/xfs_dquot.h         |    2 ++
>  fs/xfs/xfs_ondisk.h        |    2 ++
>  fs/xfs/xfs_qm_syscalls.c   |    9 +++++++--
>  5 files changed, 45 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index b1b8a5c05cea..ef36978239ac 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1197,6 +1197,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>  
>  #define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK)
>  
> +/*
> + * XFS Quota Timers
> + * ================
> + *
> + * Quota grace period expiration timers are an unsigned 32-bit seconds counter;
> + * time zero is the Unix epoch, Jan  1 00:00:01 UTC 1970.  An expiration value
> + * of zero means that the quota limit has not been reached, and therefore no
> + * expiration has been set.
> + */
> +
> +/*
> + * Smallest possible quota expiration with traditional timestamps, which is
> + * Jan  1 00:00:01 UTC 1970.
> + */
> +#define XFS_DQ_TIMEOUT_MIN	((int64_t)1)
> +
> +/*
> + * Largest possible quota expiration with traditional timestamps, which is
> + * Feb  7 06:28:15 UTC 2106.
> + */
> +#define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)

Same as last patch - these reference the unix epoch, so perhaps
they should be named that way....

>   * This is the main portion of the on-disk representation of quota information
>   * for a user.  We pad this with some more expansion room to construct the on
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index bcd73b9c2994..2425b1c30d11 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -98,6 +98,16 @@ xfs_qm_adjust_dqlimits(
>  		xfs_dquot_set_prealloc_limits(dq);
>  }
>  
> +/* Set the expiration time of a quota's grace period. */
> +void
> +xfs_dquot_set_timeout(
> +	time64_t		*timer,
> +	time64_t		value)
> +{
> +	*timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN,
> +					  XFS_DQ_TIMEOUT_MAX);
> +}
> +

Not sure I see any benefit in passing *timer as a parameter over
just returning a time64_t value...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 03/11] xfs: refactor default quota grace period setting code

[Dave Chinner]

On Thu, Aug 20, 2020 at 07:11:47PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor the code that sets the default quota grace period into a helper
> function so that we can override the ondisk behavior later.

Same comments as last patch.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 04/11] xfs: remove xfs_timestamp_t

[Dave Chinner]

On Thu, Aug 20, 2020 at 07:11:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Kill this old typedef.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_format.h     |   12 ++++++------
>  fs/xfs/libxfs/xfs_log_format.h |   12 ++++++------
>  2 files changed, 12 insertions(+), 12 deletions(-)

wouldn't it be less churn to remove this later when struct
xfs_timestamp is converted to a union?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 05/11] xfs: move xfs_log_dinode_to_disk to the log code

[Dave Chinner]

On Thu, Aug 20, 2020 at 07:12:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move this function to xfs_inode_item.c to match the encoding function
> that's already there.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_inode_buf.c |   52 -----------------------------------------
>  fs/xfs/libxfs/xfs_inode_buf.h |    2 --
>  fs/xfs/xfs_inode_item.c       |   52 +++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_inode_item.h       |    3 ++
>  4 files changed, 55 insertions(+), 54 deletions(-)

Looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 06/11] xfs: refactor inode timestamp coding

[Dave Chinner]

On Thu, Aug 20, 2020 at 07:12:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor inode timestamp encoding and decoding into helper functions so
> that we can add extra behaviors in subsequent patches.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_inode_buf.c |   42 ++++++++++++++++++++++++---------------
>  fs/xfs/libxfs/xfs_inode_buf.h |    5 +++++
>  fs/xfs/scrub/inode.c          |   25 ++++++++++++++++-------
>  fs/xfs/xfs_inode_item.c       |   44 ++++++++++++++++++++++++++---------------
>  4 files changed, 76 insertions(+), 40 deletions(-)

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Dave Chinner]

On Thu, Aug 20, 2020 at 07:12:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Redesign the ondisk timestamps to be a simple unsigned 64-bit counter of
> nanoseconds since 14 Dec 1901 (i.e. the minimum time in the 32-bit unix
> time epoch).  This enables us to handle dates up to 2486, which solves
> the y2038 problem.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

....

> @@ -875,6 +888,25 @@ union xfs_timestamp {
>   */
>  #define XFS_INO_TIME_MAX	((int64_t)S32_MAX)
>  
> +/*
> + * Number of seconds between the start of the bigtime timestamp range and the
> + * start of the Unix epoch.
> + */
> +#define XFS_INO_BIGTIME_EPOCH	(-XFS_INO_TIME_MIN)

This is confusing. It's taken me 15 minutes so far to get my head
around this because the reference frame for all these definitions is
not clear. I though these had something to do with nanosecond
timestamp limits because that's what BIGTIME records, but.....

The start of the epoch is a negative number based on the definition
of the on-disk format for the minimum number of seconds that the
"Unix" timestamp format can store?  Why is this not defined in
nanoseconds given that is what is stored on disk?

XFS_INO_BIGTIME_EPOCH = (-XFS_INO_TIME_MIN)
			= (-((int64_t)S32_MIN))
			= (-((int64_t)-2^31))
			= 2^31?

So the bigtime epoch is considered to be 2^31 *seconds* into the
range of the on-disk nanosecond timestamp? Huh?

> +
> +/*
> + * Smallest possible timestamp with big timestamps, which is
> + * Dec 13 20:45:52 UTC 1901.
> + */
> +#define XFS_INO_BIGTIME_MIN	(XFS_INO_TIME_MIN)

Same here. The reference here is "seconds before the Unix epoch",
not the minimum valid on disk nanosecond value.

Oh, these are defining the post-disk-to-in-memory-format conversion
limits? They have nothing to do with on-disk limits nor that on-disk
format is stored in nanosecond? i.e. the reference frame for these
limits is actually still the in-kernel Unix epoch definition?

> +/*
> + * Largest possible timestamp with big timestamps, which is
> + * Jul  2 20:20:25 UTC 2486.
> + */
> +#define XFS_INO_BIGTIME_MAX	((int64_t)((-1ULL / NSEC_PER_SEC) - \
> +					   XFS_INO_BIGTIME_EPOCH))

Urk. This makes my head -hurt-.

It's converting the maximum on-disk format nanosecond count to a
maximum second count then taking away the second count for the epoch
and then casting it to a signed int because the in-memory format for
seconds is signed? Oh, and the 64 bit division is via constants, so
the compiler does it and doesn't need to use something like
div_u64(), right?

Mind you, I'm just guessing here that the "-1ULL" is the
representation of the maximum on-disk nanosecond timestamp here,
because that doesn't seem to be defined anywhere....


> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index cc1316a5fe0c..c59ddb56bb90 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -157,11 +157,25 @@ xfs_imap_to_bp(
>  	return 0;
>  }
>  
> +/* Convert an ondisk timestamp into the 64-bit safe incore format. */
>  void
>  xfs_inode_from_disk_timestamp(
> +	struct xfs_dinode		*dip,
>  	struct timespec64		*tv,
>  	const union xfs_timestamp	*ts)
>  {
> +	if (dip->di_version >= 3 &&
> +	    (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) {

Helper. xfs_dinode_has_bigtime() was what I suggested previously.

> +		uint64_t		t = be64_to_cpu(ts->t_bigtime);
> +		uint64_t		s;
> +		uint32_t		n;
> +
> +		s = div_u64_rem(t, NSEC_PER_SEC, &n);
> +		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
> +		tv->tv_nsec = n;
> +		return;
> +	}
> +
>  	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
>  	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
>  }

I still don't really like the way this turned out :(

> @@ -220,9 +234,9 @@ xfs_inode_from_disk(
>  	 * a time before epoch is converted to a time long after epoch
>  	 * on 64 bit systems.
>  	 */
> -	xfs_inode_from_disk_timestamp(&inode->i_atime, &from->di_atime);
> -	xfs_inode_from_disk_timestamp(&inode->i_mtime, &from->di_mtime);
> -	xfs_inode_from_disk_timestamp(&inode->i_ctime, &from->di_ctime);
> +	xfs_inode_from_disk_timestamp(from, &inode->i_atime, &from->di_atime);
> +	xfs_inode_from_disk_timestamp(from, &inode->i_mtime, &from->di_mtime);
> +	xfs_inode_from_disk_timestamp(from, &inode->i_ctime, &from->di_ctime);
>  
>  	to->di_size = be64_to_cpu(from->di_size);
>  	to->di_nblocks = be64_to_cpu(from->di_nblocks);
> @@ -235,9 +249,17 @@ xfs_inode_from_disk(
>  	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
>  		inode_set_iversion_queried(inode,
>  					   be64_to_cpu(from->di_changecount));
> -		xfs_inode_from_disk_timestamp(&to->di_crtime, &from->di_crtime);
> +		xfs_inode_from_disk_timestamp(from, &to->di_crtime,
> +				&from->di_crtime);
>  		to->di_flags2 = be64_to_cpu(from->di_flags2);
>  		to->di_cowextsize = be32_to_cpu(from->di_cowextsize);
> +		/*
> +		 * Set the bigtime flag incore so that we automatically convert
> +		 * this inode's ondisk timestamps to bigtime format the next
> +		 * time we write the inode core to disk.
> +		 */
> +		if (xfs_sb_version_hasbigtime(&ip->i_mount->m_sb))
> +			to->di_flags2 |= XFS_DIFLAG2_BIGTIME;
>  	}

iAs I said before, you can't set this flag here - it needs to be
done transactionally when the timestamp is next logged.


> @@ -259,9 +281,19 @@ xfs_inode_from_disk(
>  
>  void
>  xfs_inode_to_disk_timestamp(
> +	struct xfs_icdinode		*from,
>  	union xfs_timestamp		*ts,
>  	const struct timespec64		*tv)
>  {
> +	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {

Shouldn't this also check the inode version number like the dinode
decoder? Perhaps a helper like xfs_inode_has_bigtime(ip)?

> +		uint64_t		t;
> +
> +		t = (uint64_t)(tv->tv_sec + XFS_INO_BIGTIME_EPOCH);

tv_sec can still overflow, right?

		t = (uint64_t)tv->tv_sec + XFS_INO_BIGTIME_EPOCH;

> +		t *= NSEC_PER_SEC;
> +		ts->t_bigtime = cpu_to_be64(t + tv->tv_nsec);
> +		return;
> +	}
> +
>  	ts->t_sec = cpu_to_be32(tv->tv_sec);
>  	ts->t_nsec = cpu_to_be32(tv->tv_nsec);
>  }
> @@ -285,9 +317,9 @@ xfs_inode_to_disk(
>  	to->di_projid_hi = cpu_to_be16(from->di_projid >> 16);
>  
>  	memset(to->di_pad, 0, sizeof(to->di_pad));
> -	xfs_inode_to_disk_timestamp(&to->di_atime, &inode->i_atime);
> -	xfs_inode_to_disk_timestamp(&to->di_mtime, &inode->i_mtime);
> -	xfs_inode_to_disk_timestamp(&to->di_ctime, &inode->i_ctime);
> +	xfs_inode_to_disk_timestamp(from, &to->di_atime, &inode->i_atime);
> +	xfs_inode_to_disk_timestamp(from, &to->di_mtime, &inode->i_mtime);
> +	xfs_inode_to_disk_timestamp(from, &to->di_ctime, &inode->i_ctime);
>  	to->di_nlink = cpu_to_be32(inode->i_nlink);
>  	to->di_gen = cpu_to_be32(inode->i_generation);
>  	to->di_mode = cpu_to_be16(inode->i_mode);
> @@ -306,7 +338,8 @@ xfs_inode_to_disk(
>  	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
>  		to->di_version = 3;
>  		to->di_changecount = cpu_to_be64(inode_peek_iversion(inode));
> -		xfs_inode_to_disk_timestamp(&to->di_crtime, &from->di_crtime);
> +		xfs_inode_to_disk_timestamp(from, &to->di_crtime,
> +				&from->di_crtime);
>  		to->di_flags2 = cpu_to_be64(from->di_flags2);
>  		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
>  		to->di_ino = cpu_to_be64(ip->i_ino);
> @@ -526,6 +559,11 @@ xfs_dinode_verify(
>  	if (fa)
>  		return fa;
>  
> +	/* bigtime iflag can only happen on bigtime filesystems */
> +	if ((flags2 & (XFS_DIFLAG2_BIGTIME)) &&
> +	    !xfs_sb_version_hasbigtime(&mp->m_sb))

	if (xfs_dinode_has_bigtime(dip) &&
	    !xfs_sb_version_hasbigtime(&mp->m_sb))

> +void xfs_inode_to_disk_timestamp(struct xfs_icdinode *from,
> +		union xfs_timestamp *ts, const struct timespec64 *tv);
>  
>  #endif	/* __XFS_INODE_BUF_H__ */
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 17c83d29998c..569721f7f9e5 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -373,6 +373,9 @@ union xfs_ictimestamp {
>  		int32_t		t_sec;		/* timestamp seconds */
>  		int32_t		t_nsec;		/* timestamp nanoseconds */
>  	};
> +
> +	/* Nanoseconds since the bigtime epoch. */
> +	uint64_t		t_bigtime;
>  };

Where are we using this again? Right now the timestamps are
converted directly into the VFS inode timestamp fields so we can get
rid of these incore timestamp fields. So shouldn't we be trying to
get rid of this structure rather than adding more functionality to
it?

> @@ -131,6 +131,17 @@ xfs_trans_log_inode(
>  			iversion_flags = XFS_ILOG_CORE;
>  	}
>  
> +	/*
> +	 * If we're updating the inode core or the timestamps and it's possible
> +	 * to upgrade this inode to bigtime format, do so now.
> +	 */
> +	if ((flags & (XFS_ILOG_CORE | XFS_ILOG_TIMESTAMP)) &&
> +	    xfs_sb_version_hasbigtime(&tp->t_mountp->m_sb) &&
> +	    !(ip->i_d.di_flags2 & XFS_DIFLAG2_BIGTIME)) {

The latter two checks could be wrapped up into a helper named
something obvious like xfs_inode_need_bigtime(ip)?

> +		ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
> +		flags |= XFS_ILOG_CORE;
> +	}
> +
>  	/*
>  	 * Record the specific change for fdatasync optimisation. This allows
>  	 * fdatasync to skip log forces for inodes that are only timestamp
> diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> index 9f036053fdb7..6f00309de2d4 100644
> --- a/fs/xfs/scrub/inode.c
> +++ b/fs/xfs/scrub/inode.c
> @@ -190,6 +190,11 @@ xchk_inode_flags2(
>  	if ((flags2 & XFS_DIFLAG2_DAX) && (flags2 & XFS_DIFLAG2_REFLINK))
>  		goto bad;
>  
> +	/* no bigtime iflag without the bigtime feature */
> +	if (!xfs_sb_version_hasbigtime(&mp->m_sb) &&
> +	    (flags2 & XFS_DIFLAG2_BIGTIME))

Can we get all these open coded checks wrapped up into a single
helper?

> +		xchk_dinode_nsec(sc, ino, dip, &dip->di_crtime);
>  		xchk_inode_flags2(sc, dip, ino, mode, flags, flags2);
>  		xchk_inode_cowextsize(sc, dip, ino, mode, flags,
>  				flags2);
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c06129cffba9..bbc309bc70c4 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -841,6 +841,8 @@ xfs_ialloc(
>  	if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
>  		inode_set_iversion(inode, 1);
>  		ip->i_d.di_flags2 = 0;
> +		if (xfs_sb_version_hasbigtime(&mp->m_sb))
> +			ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;

Rather than calculate the initial inode falgs on every allocation,
shouldn't we just have the defaults pre-calculated at mount time?

>  		ip->i_d.di_cowextsize = 0;
>  		ip->i_d.di_crtime = tv;
>  	}
> @@ -2717,7 +2719,11 @@ xfs_ifree(
>  
>  	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
>  	ip->i_d.di_flags = 0;
> -	ip->i_d.di_flags2 = 0;
> +	/*
> +	 * Preserve the bigtime flag so that di_ctime accurately stores the
> +	 * deletion time.
> +	 */
> +	ip->i_d.di_flags2 &= XFS_DIFLAG2_BIGTIME;

Oh, that's a nasty wart.

>  	ip->i_d.di_dmevmask = 0;
>  	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
>  	ip->i_df.if_format = XFS_DINODE_FMT_EXTENTS;
> @@ -3503,6 +3509,13 @@ xfs_iflush(
>  			__func__, ip->i_ino, ip->i_d.di_forkoff, ip);
>  		goto flush_out;
>  	}
> +	if (xfs_sb_version_hasbigtime(&mp->m_sb) &&
> +	    !(ip->i_d.di_flags2 & XFS_DIFLAG2_BIGTIME)) {
> +		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
> +			"%s: bad inode %Lu, bigtime unset, ptr "PTR_FMT,
> +			__func__, ip->i_ino, ip);
> +		goto flush_out;
> +	}

Why is this a fatal corruption error? if the bigtime flag is not
set, we can still store and read the timestamp if it is within
the unix epoch range...

>  
>  	/*
>  	 * Inode item log recovery for v2 inodes are dependent on the
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index cec6d4d82bc4..c38af3eea48f 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -298,9 +298,16 @@ xfs_inode_item_format_attr_fork(
>  /* Write a log_dinode timestamp into an ondisk inode timestamp. */
>  static inline void
>  xfs_log_dinode_to_disk_ts(
> +	struct xfs_log_dinode		*from,
>  	union xfs_timestamp		*ts,
>  	const union xfs_ictimestamp	*its)
>  {
> +	if (from->di_version >= 3 &&
> +	    (from->di_flags2 & XFS_DIFLAG2_BIGTIME)) {

helper.

> +		ts->t_bigtime = cpu_to_be64(its->t_bigtime);
> +		return;
> +	}
> +
>  	ts->t_sec = cpu_to_be32(its->t_sec);
>  	ts->t_nsec = cpu_to_be32(its->t_nsec);
>  }
> @@ -322,9 +329,9 @@ xfs_log_dinode_to_disk(
>  	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
>  	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
>  
> -	xfs_log_dinode_to_disk_ts(&to->di_atime, &from->di_atime);
> -	xfs_log_dinode_to_disk_ts(&to->di_mtime, &from->di_mtime);
> -	xfs_log_dinode_to_disk_ts(&to->di_ctime, &from->di_ctime);
> +	xfs_log_dinode_to_disk_ts(from, &to->di_atime, &from->di_atime);
> +	xfs_log_dinode_to_disk_ts(from, &to->di_mtime, &from->di_mtime);
> +	xfs_log_dinode_to_disk_ts(from, &to->di_ctime, &from->di_ctime);
>  
>  	to->di_size = cpu_to_be64(from->di_size);
>  	to->di_nblocks = cpu_to_be64(from->di_nblocks);
> @@ -340,7 +347,7 @@ xfs_log_dinode_to_disk(
>  
>  	if (from->di_version == 3) {
>  		to->di_changecount = cpu_to_be64(from->di_changecount);
> -		xfs_log_dinode_to_disk_ts(&to->di_crtime, &from->di_crtime);
> +		xfs_log_dinode_to_disk_ts(from, &to->di_crtime, &from->di_crtime);
>  		to->di_flags2 = cpu_to_be64(from->di_flags2);
>  		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
>  		to->di_ino = cpu_to_be64(from->di_ino);
> @@ -356,9 +363,19 @@ xfs_log_dinode_to_disk(
>  /* Write an incore inode timestamp into a log_dinode timestamp. */
>  static inline void
>  xfs_inode_to_log_dinode_ts(
> +	struct xfs_icdinode		*from,
>  	union xfs_ictimestamp		*its,
>  	const struct timespec64		*ts)
>  {
> +	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {
> +		uint64_t		t;
> +
> +		t = (uint64_t)(ts->tv_sec + XFS_INO_BIGTIME_EPOCH);
> +		t *= NSEC_PER_SEC;
> +		its->t_bigtime = t + ts->tv_nsec;

helper,

> +		return;
> +	}
> +
>  	its->t_sec = ts->tv_sec;
>  	its->t_nsec = ts->tv_nsec;
>  }
> @@ -381,9 +398,9 @@ xfs_inode_to_log_dinode(
>  
>  	memset(to->di_pad, 0, sizeof(to->di_pad));
>  	memset(to->di_pad3, 0, sizeof(to->di_pad3));
> -	xfs_inode_to_log_dinode_ts(&to->di_atime, &inode->i_atime);
> -	xfs_inode_to_log_dinode_ts(&to->di_mtime, &inode->i_mtime);
> -	xfs_inode_to_log_dinode_ts(&to->di_ctime, &inode->i_ctime);
> +	xfs_inode_to_log_dinode_ts(from, &to->di_atime, &inode->i_atime);
> +	xfs_inode_to_log_dinode_ts(from, &to->di_mtime, &inode->i_mtime);
> +	xfs_inode_to_log_dinode_ts(from, &to->di_ctime, &inode->i_ctime);
>  	to->di_nlink = inode->i_nlink;
>  	to->di_gen = inode->i_generation;
>  	to->di_mode = inode->i_mode;
> @@ -405,7 +422,7 @@ xfs_inode_to_log_dinode(
>  	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
>  		to->di_version = 3;
>  		to->di_changecount = inode_peek_iversion(inode);
> -		xfs_inode_to_log_dinode_ts(&to->di_crtime, &from->di_crtime);
> +		xfs_inode_to_log_dinode_ts(from, &to->di_crtime, &from->di_crtime);
>  		to->di_flags2 = from->di_flags2;
>  		to->di_cowextsize = from->di_cowextsize;
>  		to->di_ino = ip->i_ino;
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 6f22a66777cd..13396c3665d1 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1190,7 +1190,8 @@ xfs_flags2diflags2(
>  	unsigned int		xflags)
>  {
>  	uint64_t		di_flags2 =
> -		(ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK);
> +		(ip->i_d.di_flags2 & (XFS_DIFLAG2_REFLINK |
> +				      XFS_DIFLAG2_BIGTIME));
>  
>  	if (xflags & FS_XFLAG_DAX)
>  		di_flags2 |= XFS_DIFLAG2_DAX;
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 7158a8de719f..3e0c677cff15 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -25,6 +25,9 @@ xfs_check_limits(void)
>  	/* make sure timestamp limits are correct */
>  	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
>  	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_EPOCH,			2147483648LL);
> +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MIN,			-2147483648LL);

That still just doesn't look right to me :/

This implies that the epoch is 2^32 seconds after then minimum
supported time (2038), when in fact it is only 2^31 seconds after the
minimum supported timestamp (1970). :/

> +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MAX,			16299260425LL);

Hmmm. I got 16299260424 when I just ran this through a simple calc.
Mind you, no calculator app I found could handle unsigned 64 bit
values natively (signed 64 bit is good enough for everyone!) so
maybe I got an off-by one here...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 05/11] xfs: move xfs_log_dinode_to_disk to the log code

[Darrick J. Wong]

On Sat, Aug 22, 2020 at 08:16:31AM +0100, Christoph Hellwig wrote:
> On Thu, Aug 20, 2020 at 07:12:02PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Move this function to xfs_inode_item.c to match the encoding function
> > that's already there.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> An good reason to not move it towards its only caller in
> fs/xfs/xfs_inode_item_recover.c?

"Darrick forgot to do that after refactoring the log recovery code"? :D

Yeah, I'll take care of that.

--D

Re: [PATCH 01/11] xfs: explicitly define inode timestamp range

[Darrick J. Wong]

On Mon, Aug 24, 2020 at 09:54:29AM +1000, Dave Chinner wrote:
> On Thu, Aug 20, 2020 at 07:11:34PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Formally define the inode timestamp ranges that existing filesystems
> > support, and switch the vfs timetamp ranges to use it.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/xfs/libxfs/xfs_format.h |   19 +++++++++++++++++++
> >  fs/xfs/xfs_ondisk.h        |   12 ++++++++++++
> >  fs/xfs/xfs_super.c         |    5 +++--
> >  3 files changed, 34 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > index be86fa1a5556..b1b8a5c05cea 100644
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -849,11 +849,30 @@ struct xfs_agfl {
> >  	    ASSERT(xfs_daddr_to_agno(mp, d) == \
> >  		   xfs_daddr_to_agno(mp, (d) + (len) - 1)))
> >  
> > +/*
> > + * XFS Timestamps
> > + * ==============
> > + *
> > + * Inode timestamps consist of signed 32-bit counters for seconds and
> > + * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
> > + */
> >  typedef struct xfs_timestamp {
> >  	__be32		t_sec;		/* timestamp seconds */
> >  	__be32		t_nsec;		/* timestamp nanoseconds */
> >  } xfs_timestamp_t;
> >  
> > +/*
> > + * Smallest possible timestamp with traditional timestamps, which is
> > + * Dec 13 20:45:52 UTC 1901.
> > + */
> > +#define XFS_INO_TIME_MIN	((int64_t)S32_MIN)
> > +
> > +/*
> > + * Largest possible timestamp with traditional timestamps, which is
> > + * Jan 19 03:14:07 UTC 2038.
> > + */
> > +#define XFS_INO_TIME_MAX	((int64_t)S32_MAX)
> 
> These are based on the Unix epoch. Can we call them something like
> XFS_INO_UNIX_TIME_{MIN,MAX} to indicate what epoch they reference?

I'll update the comment and name to make it clear that this is the Unix
epoch we're talking about.

> >  /*
> >   * On-disk inode structure.
> >   *
> > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > index acb9b737fe6b..48a64fa49f91 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -15,6 +15,18 @@
> >  		"XFS: offsetof(" #structname ", " #member ") is wrong, " \
> >  		"expected " #off)
> >  
> > +#define XFS_CHECK_VALUE(value, expected) \
> > +	BUILD_BUG_ON_MSG((value) != (expected), \
> > +		"XFS: value of " #value " is wrong, expected " #expected)
> > +
> > +static inline void __init
> > +xfs_check_limits(void)
> > +{
> > +	/* make sure timestamp limits are correct */
> > +	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
> > +	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> > +}
> 
> Not sure this really gains us anything? All it does is check that
> S32_MIN/S32_MAX haven't changed value....

Will remove.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

Re: [PATCH 02/11] xfs: refactor quota expiration timer modification

[Darrick J. Wong]

On Mon, Aug 24, 2020 at 09:57:37AM +1000, Dave Chinner wrote:
> On Thu, Aug 20, 2020 at 07:11:40PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Define explicit limits on the range of quota grace period expiration
> > timeouts and refactor the code that modifies the timeouts into helpers
> > that clamp the values appropriately.  Note that we'll deal with the
> > grace period timer separately.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/xfs/libxfs/xfs_format.h |   22 ++++++++++++++++++++++
> >  fs/xfs/xfs_dquot.c         |   13 ++++++++++++-
> >  fs/xfs/xfs_dquot.h         |    2 ++
> >  fs/xfs/xfs_ondisk.h        |    2 ++
> >  fs/xfs/xfs_qm_syscalls.c   |    9 +++++++--
> >  5 files changed, 45 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > index b1b8a5c05cea..ef36978239ac 100644
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -1197,6 +1197,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> >  
> >  #define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK)
> >  
> > +/*
> > + * XFS Quota Timers
> > + * ================
> > + *
> > + * Quota grace period expiration timers are an unsigned 32-bit seconds counter;
> > + * time zero is the Unix epoch, Jan  1 00:00:01 UTC 1970.  An expiration value
> > + * of zero means that the quota limit has not been reached, and therefore no
> > + * expiration has been set.
> > + */
> > +
> > +/*
> > + * Smallest possible quota expiration with traditional timestamps, which is
> > + * Jan  1 00:00:01 UTC 1970.
> > + */
> > +#define XFS_DQ_TIMEOUT_MIN	((int64_t)1)
> > +
> > +/*
> > + * Largest possible quota expiration with traditional timestamps, which is
> > + * Feb  7 06:28:15 UTC 2106.
> > + */
> > +#define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
> 
> Same as last patch - these reference the unix epoch, so perhaps
> they should be named that way....
> 
> >   * This is the main portion of the on-disk representation of quota information
> >   * for a user.  We pad this with some more expansion room to construct the on
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index bcd73b9c2994..2425b1c30d11 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -98,6 +98,16 @@ xfs_qm_adjust_dqlimits(
> >  		xfs_dquot_set_prealloc_limits(dq);
> >  }
> >  
> > +/* Set the expiration time of a quota's grace period. */
> > +void
> > +xfs_dquot_set_timeout(
> > +	time64_t		*timer,
> > +	time64_t		value)
> > +{
> > +	*timer = clamp_t(time64_t, value, XFS_DQ_TIMEOUT_MIN,
> > +					  XFS_DQ_TIMEOUT_MAX);
> > +}
> > +
> 
> Not sure I see any benefit in passing *timer as a parameter over
> just returning a time64_t value...

I'll look into changing them, both for this and the next patch.
I /think/ it should be fairly straightforward.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

Re: [PATCH 07/11] xfs: convert struct xfs_timestamp to union

[Darrick J. Wong]

On Sat, Aug 22, 2020 at 08:18:30AM +0100, Christoph Hellwig wrote:
> On Thu, Aug 20, 2020 at 07:12:15PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Convert the xfs_timestamp struct to a union so that we can overload it
> > in the next patch.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/xfs/libxfs/xfs_format.h     |   16 +++++++++-------
> >  fs/xfs/libxfs/xfs_inode_buf.c  |    4 ++--
> >  fs/xfs/libxfs/xfs_inode_buf.h  |    4 ++--
> >  fs/xfs/libxfs/xfs_log_format.h |   16 +++++++++-------
> >  fs/xfs/scrub/inode.c           |    2 +-
> >  fs/xfs/xfs_inode_item.c        |    6 +++---
> >  fs/xfs/xfs_ondisk.h            |    4 ++--
> >  7 files changed, 28 insertions(+), 24 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > index 1f3a2be6c396..772113db41aa 100644
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -856,9 +856,11 @@ struct xfs_agfl {
> >   * Inode timestamps consist of signed 32-bit counters for seconds and
> >   * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
> >   */
> > -struct xfs_timestamp {
> > -	__be32		t_sec;		/* timestamp seconds */
> > -	__be32		t_nsec;		/* timestamp nanoseconds */
> > +union xfs_timestamp {
> > +	struct {
> > +		__be32		t_sec;		/* timestamp seconds */
> > +		__be32		t_nsec;		/* timestamp nanoseconds */
> > +	};
> >  };
> 
> Wouldn't it make sense to merge the typedef removal patch into this
> one to avoid touching all the users twice?

Will fix.  I wasn't sure if people would howl about making both changes
at once, and it's easier to combine two patches. ;)

--D

Re: [PATCH 09/11] xfs: refactor quota timestamp coding

[Darrick J. Wong]

On Sat, Aug 22, 2020 at 08:33:56AM +0100, Christoph Hellwig wrote:
> > +/* Convert an on-disk timer value into an incore timer value. */
> > +void
> > +xfs_dquot_from_disk_timestamp(
> > +	struct xfs_disk_dquot	*ddq,
> > +	time64_t		*timer,
> > +	__be32			dtimer)
> > +{
> > +	*timer = be32_to_cpu(dtimer);
> > +}
> > +
> > +/* Convert an incore timer value into an on-disk timer value. */
> > +void
> > +xfs_dquot_to_disk_timestamp(
> > +	struct xfs_dquot	*dqp,
> > +	__be32			*dtimer,
> > +	time64_t		timer)
> > +{
> > +	*dtimer = cpu_to_be32(timer);
> 
> Why not return the values?

I'll fix those return vlaues too.

--D

Re: [PATCH 10/11] xfs: enable bigtime for quota timers

[Darrick J. Wong]

On Sat, Aug 22, 2020 at 08:36:00AM +0100, Christoph Hellwig wrote:
> On Thu, Aug 20, 2020 at 07:12:34PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Enable the bigtime feature for quota timers.  We decrease the accuracy
> > of the timers to ~4s in exchange for being able to set timers up to the
> > bigtime maximum.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/xfs/libxfs/xfs_dquot_buf.c  |   32 ++++++++++++++++++++++++++++++--
> >  fs/xfs/libxfs/xfs_format.h     |   27 ++++++++++++++++++++++++++-
> >  fs/xfs/libxfs/xfs_quota_defs.h |    3 ++-
> >  fs/xfs/xfs_dquot.c             |   25 +++++++++++++++++++++----
> >  fs/xfs/xfs_dquot.h             |    3 ++-
> >  fs/xfs/xfs_ondisk.h            |    7 +++++++
> >  fs/xfs/xfs_qm.c                |    2 ++
> >  fs/xfs/xfs_qm_syscalls.c       |    9 +++++----
> >  fs/xfs/xfs_trans_dquot.c       |    6 ++++++
> >  9 files changed, 101 insertions(+), 13 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> > index 7f5291022b11..f5997fbdd308 100644
> > --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> > +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> > @@ -69,6 +69,13 @@ xfs_dquot_verify(
> >  	    ddq_type != XFS_DQTYPE_GROUP)
> >  		return __this_address;
> >  
> > +	if ((ddq->d_type & XFS_DQTYPE_BIGTIME) &&
> > +	    !xfs_sb_version_hasbigtime(&mp->m_sb))
> > +		return __this_address;
> > +
> > +	if ((ddq->d_type & XFS_DQTYPE_BIGTIME) && !ddq->d_id)
> > +		return __this_address;
> > +
> >  	if (id != -1 && id != be32_to_cpu(ddq->d_id))
> >  		return __this_address;
> >  
> > @@ -296,7 +303,15 @@ xfs_dquot_from_disk_timestamp(
> >  	time64_t		*timer,
> >  	__be32			dtimer)
> >  {
> > -	*timer = be32_to_cpu(dtimer);
> > +	uint64_t		t;
> > +
> > +	if (!timer || !(ddq->d_type & XFS_DQTYPE_BIGTIME)) {
> > +		*timer = be32_to_cpu(dtimer);
> 
> I don't think setting *time makes any sense if time is NULL..

Oops, yeah, I introduced that bug when reworking this function. :(

> > +		return;
> > +	}
> > +
> > +	t = be32_to_cpu(dtimer);
> > +	*timer = t << XFS_DQ_BIGTIME_SHIFT;
> 
> Why not:
> 
> 	*timer = (time64_t)be32_to_cpu(dtimer) << XFS_DQ_BIGTIME_SHIFT;
> 
> or (with my previous suggestion):
> 
> 	return (time64_t)be32_to_cpu(dtimer) << XFS_DQ_BIGTIME_SHIFT;

<nod>  Looks good to me.

--D

> 
> ?
> 
> > @@ -1227,13 +1227,15 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> >  #define XFS_DQTYPE_USER		0x01		/* user dquot record */
> >  #define XFS_DQTYPE_PROJ		0x02		/* project dquot record */
> >  #define XFS_DQTYPE_GROUP	0x04		/* group dquot record */
> > +#define XFS_DQTYPE_BIGTIME	0x08		/* large expiry timestamps */
> >  
> >  /* bitmask to determine if this is a user/group/project dquot */
> >  #define XFS_DQTYPE_REC_MASK	(XFS_DQTYPE_USER | \
> >  				 XFS_DQTYPE_PROJ | \
> >  				 XFS_DQTYPE_GROUP)
> >  
> > -#define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK)
> > +#define XFS_DQTYPE_ANY		(XFS_DQTYPE_REC_MASK | \
> > +				 XFS_DQTYPE_BIGTIME)
> >  
> >  /*
> >   * XFS Quota Timers
> > @@ -1270,6 +1272,29 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> >  #define XFS_DQ_GRACE_MIN	((int64_t)0)
> >  #define XFS_DQ_GRACE_MAX	((int64_t)U32_MAX)
> >  
> > +/*
> > + * When bigtime is enabled, we trade a few bits of precision to expand the
> > + * expiration timeout range to match that of big inode timestamps.  The grace
> > + * periods stored in dquot 0 are not shifted, since they record an interval,
> > + * not a timestamp.
> > + */
> > +#define XFS_DQ_BIGTIME_SHIFT	(2)
> > +#define XFS_DQ_BIGTIME_SLACK	((int64_t)(1ULL << XFS_DQ_BIGTIME_SHIFT) - 1)
> > +
> > +/*
> > + * Smallest possible quota expiration with big timestamps, which is
> > + * Jan  1 00:00:01 UTC 1970.
> > + */
> > +#define XFS_DQ_BIGTIMEOUT_MIN		(XFS_DQ_TIMEOUT_MIN)
> > +
> > +/*
> > + * Largest supported quota expiration with traditional timestamps, which is
> > + * the largest bigtime inode timestamp, or Jul  2 20:20:25 UTC 2486.  The field
> 
> This adds and > 80 char line.

Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Darrick J. Wong]

On Sat, Aug 22, 2020 at 08:33:19AM +0100, Christoph Hellwig wrote:
> >   * in the AGI header so that we can skip the finobt walk at mount time when
> > @@ -855,12 +862,18 @@ struct xfs_agfl {
> >   *
> >   * Inode timestamps consist of signed 32-bit counters for seconds and
> >   * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
> > + *
> > + * When bigtime is enabled, timestamps become an unsigned 64-bit nanoseconds
> > + * counter.  Time zero is the start of the classic timestamp range.
> >   */
> >  union xfs_timestamp {
> >  	struct {
> >  		__be32		t_sec;		/* timestamp seconds */
> >  		__be32		t_nsec;		/* timestamp nanoseconds */
> >  	};
> > +
> > +	/* Nanoseconds since the bigtime epoch. */
> > +	__be64			t_bigtime;
> >  };
> 
> So do we really need the union here?  What about:
> 
>  (1) keep the typedef instead of removing it
>  (2) switch the typedef to be just a __be64, and use trivial helpers
>      to extract the two separate legacy sec/nsec field
>  (3) PROFIT!!!

Been there, done that.  Dave suggested some replacement code (which
corrupted the values), then I modified that into a correct version,
which then made smatch angry because it doesn't like code that does bit
shifts on __be64 values.

> > +/* Convert an ondisk timestamp into the 64-bit safe incore format. */
> >  void
> >  xfs_inode_from_disk_timestamp(
> > +	struct xfs_dinode		*dip,
> >  	struct timespec64		*tv,
> >  	const union xfs_timestamp	*ts)
> 
> I think passing ts by value might lead to somewhat better code
> generation on modern ABIs (and older ABIs just fall back to pass
> by reference transparently).

Hm, ok.  I did not know that. :)

> >  {
> > +	if (dip->di_version >= 3 &&
> > +	    (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) {
> 
> Do we want a helper for this condition?

Yes, yes we do.  Will add.

> > +		uint64_t		t = be64_to_cpu(ts->t_bigtime);
> > +		uint64_t		s;
> > +		uint32_t		n;
> > +
> > +		s = div_u64_rem(t, NSEC_PER_SEC, &n);
> > +		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
> > +		tv->tv_nsec = n;
> > +		return;
> > +	}
> > +
> >  	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
> >  	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
> 
> Nit: for these kinds of symmetric conditions and if/else feels a little
> more natural.
> 
> > +		xfs_log_dinode_to_disk_ts(from, &to->di_crtime, &from->di_crtime);
> 
> This adds a > 80 char line.

Do we care now that checkpatch has been changed to allow up to 100
columns?

> > +	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {
> > +		uint64_t		t;
> > +
> > +		t = (uint64_t)(ts->tv_sec + XFS_INO_BIGTIME_EPOCH);
> > +		t *= NSEC_PER_SEC;
> > +		its->t_bigtime = t + ts->tv_nsec;
> 
> This calculation is dupliated in two places, might be worth
> adding a little helper (which will need to get the sec/nsec values
> passed separately due to the different structures).
> 
> > +		xfs_inode_to_log_dinode_ts(from, &to->di_crtime, &from->di_crtime);
> 
> Another line over 8 characters here.
> 
> > +	if (xfs_sb_version_hasbigtime(&mp->m_sb)) {
> > +		sb->s_time_min = XFS_INO_BIGTIME_MIN;
> > +		sb->s_time_max = XFS_INO_BIGTIME_MAX;
> > +	} else {
> > +		sb->s_time_min = XFS_INO_TIME_MIN;
> > +		sb->s_time_max = XFS_INO_TIME_MAX;
> > +	}
> 
> This is really a comment on the earlier patch, but maybe we should
> name the old constants with "OLD" or "LEGACY" or "SMALL" in the name?

Yes, good suggestion!

> > @@ -1494,6 +1499,10 @@ xfs_fc_fill_super(
> >  	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
> >  		sb->s_flags |= SB_I_VERSION;
> >  
> > +	if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > +		xfs_warn(mp,
> > + "EXPERIMENTAL big timestamp feature in use. Use at your own risk!");
> > +
> 
> Is there any good reason to mark this experimental?

As you and Dave have both pointed out, there are plenty of stupid bugs
still in this.  I think I'd like to have at least one EXPERIMENTAL cycle
to make sure I didn't commit anything pathologically stupid in here.

<cough> ext4 34-bit sign extension bug <cough>.

--D

Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Darrick J. Wong]

On Mon, Aug 25, 2020 at 11:25:27AM +1000, Dave Chinner wrote:
> On Thu, Aug 20, 2020 at 07:12:21PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Redesign the ondisk timestamps to be a simple unsigned 64-bit counter of
> > nanoseconds since 14 Dec 1901 (i.e. the minimum time in the 32-bit unix
> > time epoch).  This enables us to handle dates up to 2486, which solves
> > the y2038 problem.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> ....
> 
> > @@ -875,6 +888,25 @@ union xfs_timestamp {
> >   */
> >  #define XFS_INO_TIME_MAX	((int64_t)S32_MAX)
> >  
> > +/*
> > + * Number of seconds between the start of the bigtime timestamp range and the
> > + * start of the Unix epoch.
> > + */
> > +#define XFS_INO_BIGTIME_EPOCH	(-XFS_INO_TIME_MIN)
> 
> This is confusing. It's taken me 15 minutes so far to get my head
> around this because the reference frame for all these definitions is
> not clear. I though these had something to do with nanosecond
> timestamp limits because that's what BIGTIME records, but.....
> 
> The start of the epoch is a negative number based on the definition
> of the on-disk format for the minimum number of seconds that the
> "Unix" timestamp format can store?  Why is this not defined in
> nanoseconds given that is what is stored on disk?
> 
> XFS_INO_BIGTIME_EPOCH = (-XFS_INO_TIME_MIN)
> 			= (-((int64_t)S32_MIN))
> 			= (-((int64_t)-2^31))
> 			= 2^31?
> 
> So the bigtime epoch is considered to be 2^31 *seconds* into the
> range of the on-disk nanosecond timestamp? Huh?

They're the incore limits, not the ondisk limits.

Prior to bigtime, the ondisk timestamp epoch was the Unix epoch.  This
isn't the case anymore in bigtime (bigtime's epoch is Dec. 1901, aka the
minimum timestamp under the old scheme), so that misnamed
XFS_INO_BIGTIME_EPOCH value is the conversion factor between epochs.

(I'll come back to this at the bottom.)

> > +
> > +/*
> > + * Smallest possible timestamp with big timestamps, which is
> > + * Dec 13 20:45:52 UTC 1901.
> > + */
> > +#define XFS_INO_BIGTIME_MIN	(XFS_INO_TIME_MIN)
> 
> Same here. The reference here is "seconds before the Unix epoch",
> not the minimum valid on disk nanosecond value.
> 
> Oh, these are defining the post-disk-to-in-memory-format conversion
> limits? They have nothing to do with on-disk limits nor that on-disk
> format is stored in nanosecond? i.e. the reference frame for these
> limits is actually still the in-kernel Unix epoch definition?

Yes.  The on-disk limits are 0 and -1ULL with an epoch of Dec 1901, and
these XFS_INO_BIGTIME_{MIN,MAX} constants are the incore limits, which
of course have to be relative to the Unix epoch.

> > +/*
> > + * Largest possible timestamp with big timestamps, which is
> > + * Jul  2 20:20:25 UTC 2486.
> > + */
> > +#define XFS_INO_BIGTIME_MAX	((int64_t)((-1ULL / NSEC_PER_SEC) - \
> > +					   XFS_INO_BIGTIME_EPOCH))
> 
> Urk. This makes my head -hurt-.
> 
> It's converting the maximum on-disk format nanosecond count to a
> maximum second count then taking away the second count for the epoch
> and then casting it to a signed int because the in-memory format for
> seconds is signed? Oh, and the 64 bit division is via constants, so
> the compiler does it and doesn't need to use something like
> div_u64(), right?

Right.

> Mind you, I'm just guessing here that the "-1ULL" is the
> representation of the maximum on-disk nanosecond timestamp here,
> because that doesn't seem to be defined anywhere....

Yes.  Clearly I shouldn't have taken that shortcut.

> > diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> > index cc1316a5fe0c..c59ddb56bb90 100644
> > --- a/fs/xfs/libxfs/xfs_inode_buf.c
> > +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> > @@ -157,11 +157,25 @@ xfs_imap_to_bp(
> >  	return 0;
> >  }
> >  
> > +/* Convert an ondisk timestamp into the 64-bit safe incore format. */
> >  void
> >  xfs_inode_from_disk_timestamp(
> > +	struct xfs_dinode		*dip,
> >  	struct timespec64		*tv,
> >  	const union xfs_timestamp	*ts)
> >  {
> > +	if (dip->di_version >= 3 &&
> > +	    (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) {
> 
> Helper. xfs_dinode_has_bigtime() was what I suggested previously.
> 
> > +		uint64_t		t = be64_to_cpu(ts->t_bigtime);
> > +		uint64_t		s;
> > +		uint32_t		n;
> > +
> > +		s = div_u64_rem(t, NSEC_PER_SEC, &n);
> > +		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
> > +		tv->tv_nsec = n;
> > +		return;
> > +	}
> > +
> >  	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
> >  	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
> >  }
> 
> I still don't really like the way this turned out :(

I'll think about this further and hope that hch comes up with something
that's both functional and doesn't piss off smatch/sparse.  Note that I
also don't have any big endian machines anymore, so I don't really have
a good way to test this.  powerpc32 and sparc are verrrrry dead now.

> > @@ -220,9 +234,9 @@ xfs_inode_from_disk(
> >  	 * a time before epoch is converted to a time long after epoch
> >  	 * on 64 bit systems.
> >  	 */
> > -	xfs_inode_from_disk_timestamp(&inode->i_atime, &from->di_atime);
> > -	xfs_inode_from_disk_timestamp(&inode->i_mtime, &from->di_mtime);
> > -	xfs_inode_from_disk_timestamp(&inode->i_ctime, &from->di_ctime);
> > +	xfs_inode_from_disk_timestamp(from, &inode->i_atime, &from->di_atime);
> > +	xfs_inode_from_disk_timestamp(from, &inode->i_mtime, &from->di_mtime);
> > +	xfs_inode_from_disk_timestamp(from, &inode->i_ctime, &from->di_ctime);
> >  
> >  	to->di_size = be64_to_cpu(from->di_size);
> >  	to->di_nblocks = be64_to_cpu(from->di_nblocks);
> > @@ -235,9 +249,17 @@ xfs_inode_from_disk(
> >  	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
> >  		inode_set_iversion_queried(inode,
> >  					   be64_to_cpu(from->di_changecount));
> > -		xfs_inode_from_disk_timestamp(&to->di_crtime, &from->di_crtime);
> > +		xfs_inode_from_disk_timestamp(from, &to->di_crtime,
> > +				&from->di_crtime);
> >  		to->di_flags2 = be64_to_cpu(from->di_flags2);
> >  		to->di_cowextsize = be32_to_cpu(from->di_cowextsize);
> > +		/*
> > +		 * Set the bigtime flag incore so that we automatically convert
> > +		 * this inode's ondisk timestamps to bigtime format the next
> > +		 * time we write the inode core to disk.
> > +		 */
> > +		if (xfs_sb_version_hasbigtime(&ip->i_mount->m_sb))
> > +			to->di_flags2 |= XFS_DIFLAG2_BIGTIME;
> >  	}
> 
> iAs I said before, you can't set this flag here - it needs to be
> done transactionally when the timestamp is next logged.

ARRGH.  I added code to xfs_trans_log_inode to do the conversion, and
then forgot to remove this.  I /swear/ I removed it, but there it still
is.

> 
> > @@ -259,9 +281,19 @@ xfs_inode_from_disk(
> >  
> >  void
> >  xfs_inode_to_disk_timestamp(
> > +	struct xfs_icdinode		*from,
> >  	union xfs_timestamp		*ts,
> >  	const struct timespec64		*tv)
> >  {
> > +	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {
> 
> Shouldn't this also check the inode version number like the dinode
> decoder? Perhaps a helper like xfs_inode_has_bigtime(ip)?

Yes, will add.

> > +		uint64_t		t;
> > +
> > +		t = (uint64_t)(tv->tv_sec + XFS_INO_BIGTIME_EPOCH);
> 
> tv_sec can still overflow, right?
> 
> 		t = (uint64_t)tv->tv_sec + XFS_INO_BIGTIME_EPOCH;

It /shouldn't/ since we also set the superblock timestamp limits such
that the VFS will never pass us an over-large value, but I guess we
should be defensive here anyway.

> 
> > +		t *= NSEC_PER_SEC;
> > +		ts->t_bigtime = cpu_to_be64(t + tv->tv_nsec);
> > +		return;
> > +	}
> > +
> >  	ts->t_sec = cpu_to_be32(tv->tv_sec);
> >  	ts->t_nsec = cpu_to_be32(tv->tv_nsec);
> >  }
> > @@ -285,9 +317,9 @@ xfs_inode_to_disk(
> >  	to->di_projid_hi = cpu_to_be16(from->di_projid >> 16);
> >  
> >  	memset(to->di_pad, 0, sizeof(to->di_pad));
> > -	xfs_inode_to_disk_timestamp(&to->di_atime, &inode->i_atime);
> > -	xfs_inode_to_disk_timestamp(&to->di_mtime, &inode->i_mtime);
> > -	xfs_inode_to_disk_timestamp(&to->di_ctime, &inode->i_ctime);
> > +	xfs_inode_to_disk_timestamp(from, &to->di_atime, &inode->i_atime);
> > +	xfs_inode_to_disk_timestamp(from, &to->di_mtime, &inode->i_mtime);
> > +	xfs_inode_to_disk_timestamp(from, &to->di_ctime, &inode->i_ctime);
> >  	to->di_nlink = cpu_to_be32(inode->i_nlink);
> >  	to->di_gen = cpu_to_be32(inode->i_generation);
> >  	to->di_mode = cpu_to_be16(inode->i_mode);
> > @@ -306,7 +338,8 @@ xfs_inode_to_disk(
> >  	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
> >  		to->di_version = 3;
> >  		to->di_changecount = cpu_to_be64(inode_peek_iversion(inode));
> > -		xfs_inode_to_disk_timestamp(&to->di_crtime, &from->di_crtime);
> > +		xfs_inode_to_disk_timestamp(from, &to->di_crtime,
> > +				&from->di_crtime);
> >  		to->di_flags2 = cpu_to_be64(from->di_flags2);
> >  		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
> >  		to->di_ino = cpu_to_be64(ip->i_ino);
> > @@ -526,6 +559,11 @@ xfs_dinode_verify(
> >  	if (fa)
> >  		return fa;
> >  
> > +	/* bigtime iflag can only happen on bigtime filesystems */
> > +	if ((flags2 & (XFS_DIFLAG2_BIGTIME)) &&
> > +	    !xfs_sb_version_hasbigtime(&mp->m_sb))
> 
> 	if (xfs_dinode_has_bigtime(dip) &&
> 	    !xfs_sb_version_hasbigtime(&mp->m_sb))

<nod>

> 
> > +void xfs_inode_to_disk_timestamp(struct xfs_icdinode *from,
> > +		union xfs_timestamp *ts, const struct timespec64 *tv);
> >  
> >  #endif	/* __XFS_INODE_BUF_H__ */
> > diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> > index 17c83d29998c..569721f7f9e5 100644
> > --- a/fs/xfs/libxfs/xfs_log_format.h
> > +++ b/fs/xfs/libxfs/xfs_log_format.h
> > @@ -373,6 +373,9 @@ union xfs_ictimestamp {
> >  		int32_t		t_sec;		/* timestamp seconds */
> >  		int32_t		t_nsec;		/* timestamp nanoseconds */
> >  	};
> > +
> > +	/* Nanoseconds since the bigtime epoch. */
> > +	uint64_t		t_bigtime;
> >  };
> 
> Where are we using this again? Right now the timestamps are
> converted directly into the VFS inode timestamp fields so we can get
> rid of these incore timestamp fields. So shouldn't we be trying to
> get rid of this structure rather than adding more functionality to
> it?

We would have to enlarge xfs_log_dinode to log a full timespec64-like
entity.   I understand that it's annoying to convert a vfs timestamp
back into a u64 nanoseconds counter for the sake of the log, but doing
so will add complexity to the log for absolutely zero gain because
having 96 bits per timestamp in the log doesn't buy us anything.

> > @@ -131,6 +131,17 @@ xfs_trans_log_inode(
> >  			iversion_flags = XFS_ILOG_CORE;
> >  	}
> >  
> > +	/*
> > +	 * If we're updating the inode core or the timestamps and it's possible
> > +	 * to upgrade this inode to bigtime format, do so now.
> > +	 */
> > +	if ((flags & (XFS_ILOG_CORE | XFS_ILOG_TIMESTAMP)) &&
> > +	    xfs_sb_version_hasbigtime(&tp->t_mountp->m_sb) &&
> > +	    !(ip->i_d.di_flags2 & XFS_DIFLAG2_BIGTIME)) {
> 
> The latter two checks could be wrapped up into a helper named
> something obvious like xfs_inode_need_bigtime(ip)?

Ok.

> > +		ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
> > +		flags |= XFS_ILOG_CORE;
> > +	}
> > +
> >  	/*
> >  	 * Record the specific change for fdatasync optimisation. This allows
> >  	 * fdatasync to skip log forces for inodes that are only timestamp
> > diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> > index 9f036053fdb7..6f00309de2d4 100644
> > --- a/fs/xfs/scrub/inode.c
> > +++ b/fs/xfs/scrub/inode.c
> > @@ -190,6 +190,11 @@ xchk_inode_flags2(
> >  	if ((flags2 & XFS_DIFLAG2_DAX) && (flags2 & XFS_DIFLAG2_REFLINK))
> >  		goto bad;
> >  
> > +	/* no bigtime iflag without the bigtime feature */
> > +	if (!xfs_sb_version_hasbigtime(&mp->m_sb) &&
> > +	    (flags2 & XFS_DIFLAG2_BIGTIME))
> 
> Can we get all these open coded checks wrapped up into a single
> helper?

Ok.

> > +		xchk_dinode_nsec(sc, ino, dip, &dip->di_crtime);
> >  		xchk_inode_flags2(sc, dip, ino, mode, flags, flags2);
> >  		xchk_inode_cowextsize(sc, dip, ino, mode, flags,
> >  				flags2);
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index c06129cffba9..bbc309bc70c4 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -841,6 +841,8 @@ xfs_ialloc(
> >  	if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
> >  		inode_set_iversion(inode, 1);
> >  		ip->i_d.di_flags2 = 0;
> > +		if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > +			ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
> 
> Rather than calculate the initial inode falgs on every allocation,
> shouldn't we just have the defaults pre-calculated at mount time?

Hm, yes.  Add that to the inode geometry structure?

> >  		ip->i_d.di_cowextsize = 0;
> >  		ip->i_d.di_crtime = tv;
> >  	}
> > @@ -2717,7 +2719,11 @@ xfs_ifree(
> >  
> >  	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
> >  	ip->i_d.di_flags = 0;
> > -	ip->i_d.di_flags2 = 0;
> > +	/*
> > +	 * Preserve the bigtime flag so that di_ctime accurately stores the
> > +	 * deletion time.
> > +	 */
> > +	ip->i_d.di_flags2 &= XFS_DIFLAG2_BIGTIME;
> 
> Oh, that's a nasty wart.

And here again?

> >  	ip->i_d.di_forkoff = 0;		/* mark the attr fork not in use */
> >  	ip->i_df.if_format = XFS_DINODE_FMT_EXTENTS;
> > @@ -3503,6 +3509,13 @@ xfs_iflush(
> >  			__func__, ip->i_ino, ip->i_d.di_forkoff, ip);
> >  		goto flush_out;
> >  	}
> > +	if (xfs_sb_version_hasbigtime(&mp->m_sb) &&
> > +	    !(ip->i_d.di_flags2 & XFS_DIFLAG2_BIGTIME)) {
> > +		xfs_alert_tag(mp, XFS_PTAG_IFLUSH,
> > +			"%s: bad inode %Lu, bigtime unset, ptr "PTR_FMT,
> > +			__func__, ip->i_ino, ip);
> > +		goto flush_out;
> > +	}
> 
> Why is this a fatal corruption error? if the bigtime flag is not
> set, we can still store and read the timestamp if it is within
> the unix epoch range...

Ugh, it's not.  It's a leftover artifact from when I would just set the
flag incore unconditionally, all of which should have been removed when
I added the modifications to xfs_trans_log_inode, but clearly I forgot.

> >  
> >  	/*
> >  	 * Inode item log recovery for v2 inodes are dependent on the
> > diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> > index cec6d4d82bc4..c38af3eea48f 100644
> > --- a/fs/xfs/xfs_inode_item.c
> > +++ b/fs/xfs/xfs_inode_item.c
> > @@ -298,9 +298,16 @@ xfs_inode_item_format_attr_fork(
> >  /* Write a log_dinode timestamp into an ondisk inode timestamp. */
> >  static inline void
> >  xfs_log_dinode_to_disk_ts(
> > +	struct xfs_log_dinode		*from,
> >  	union xfs_timestamp		*ts,
> >  	const union xfs_ictimestamp	*its)
> >  {
> > +	if (from->di_version >= 3 &&
> > +	    (from->di_flags2 & XFS_DIFLAG2_BIGTIME)) {
> 
> helper.
> 
> > +		ts->t_bigtime = cpu_to_be64(its->t_bigtime);
> > +		return;
> > +	}
> > +
> >  	ts->t_sec = cpu_to_be32(its->t_sec);
> >  	ts->t_nsec = cpu_to_be32(its->t_nsec);
> >  }
> > @@ -322,9 +329,9 @@ xfs_log_dinode_to_disk(
> >  	to->di_projid_hi = cpu_to_be16(from->di_projid_hi);
> >  	memcpy(to->di_pad, from->di_pad, sizeof(to->di_pad));
> >  
> > -	xfs_log_dinode_to_disk_ts(&to->di_atime, &from->di_atime);
> > -	xfs_log_dinode_to_disk_ts(&to->di_mtime, &from->di_mtime);
> > -	xfs_log_dinode_to_disk_ts(&to->di_ctime, &from->di_ctime);
> > +	xfs_log_dinode_to_disk_ts(from, &to->di_atime, &from->di_atime);
> > +	xfs_log_dinode_to_disk_ts(from, &to->di_mtime, &from->di_mtime);
> > +	xfs_log_dinode_to_disk_ts(from, &to->di_ctime, &from->di_ctime);
> >  
> >  	to->di_size = cpu_to_be64(from->di_size);
> >  	to->di_nblocks = cpu_to_be64(from->di_nblocks);
> > @@ -340,7 +347,7 @@ xfs_log_dinode_to_disk(
> >  
> >  	if (from->di_version == 3) {
> >  		to->di_changecount = cpu_to_be64(from->di_changecount);
> > -		xfs_log_dinode_to_disk_ts(&to->di_crtime, &from->di_crtime);
> > +		xfs_log_dinode_to_disk_ts(from, &to->di_crtime, &from->di_crtime);
> >  		to->di_flags2 = cpu_to_be64(from->di_flags2);
> >  		to->di_cowextsize = cpu_to_be32(from->di_cowextsize);
> >  		to->di_ino = cpu_to_be64(from->di_ino);
> > @@ -356,9 +363,19 @@ xfs_log_dinode_to_disk(
> >  /* Write an incore inode timestamp into a log_dinode timestamp. */
> >  static inline void
> >  xfs_inode_to_log_dinode_ts(
> > +	struct xfs_icdinode		*from,
> >  	union xfs_ictimestamp		*its,
> >  	const struct timespec64		*ts)
> >  {
> > +	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {
> > +		uint64_t		t;
> > +
> > +		t = (uint64_t)(ts->tv_sec + XFS_INO_BIGTIME_EPOCH);
> > +		t *= NSEC_PER_SEC;
> > +		its->t_bigtime = t + ts->tv_nsec;
> 
> helper,
> 
> > +		return;
> > +	}
> > +
> >  	its->t_sec = ts->tv_sec;
> >  	its->t_nsec = ts->tv_nsec;
> >  }
> > @@ -381,9 +398,9 @@ xfs_inode_to_log_dinode(
> >  
> >  	memset(to->di_pad, 0, sizeof(to->di_pad));
> >  	memset(to->di_pad3, 0, sizeof(to->di_pad3));
> > -	xfs_inode_to_log_dinode_ts(&to->di_atime, &inode->i_atime);
> > -	xfs_inode_to_log_dinode_ts(&to->di_mtime, &inode->i_mtime);
> > -	xfs_inode_to_log_dinode_ts(&to->di_ctime, &inode->i_ctime);
> > +	xfs_inode_to_log_dinode_ts(from, &to->di_atime, &inode->i_atime);
> > +	xfs_inode_to_log_dinode_ts(from, &to->di_mtime, &inode->i_mtime);
> > +	xfs_inode_to_log_dinode_ts(from, &to->di_ctime, &inode->i_ctime);
> >  	to->di_nlink = inode->i_nlink;
> >  	to->di_gen = inode->i_generation;
> >  	to->di_mode = inode->i_mode;
> > @@ -405,7 +422,7 @@ xfs_inode_to_log_dinode(
> >  	if (xfs_sb_version_has_v3inode(&ip->i_mount->m_sb)) {
> >  		to->di_version = 3;
> >  		to->di_changecount = inode_peek_iversion(inode);
> > -		xfs_inode_to_log_dinode_ts(&to->di_crtime, &from->di_crtime);
> > +		xfs_inode_to_log_dinode_ts(from, &to->di_crtime, &from->di_crtime);
> >  		to->di_flags2 = from->di_flags2;
> >  		to->di_cowextsize = from->di_cowextsize;
> >  		to->di_ino = ip->i_ino;
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 6f22a66777cd..13396c3665d1 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -1190,7 +1190,8 @@ xfs_flags2diflags2(
> >  	unsigned int		xflags)
> >  {
> >  	uint64_t		di_flags2 =
> > -		(ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK);
> > +		(ip->i_d.di_flags2 & (XFS_DIFLAG2_REFLINK |
> > +				      XFS_DIFLAG2_BIGTIME));
> >  
> >  	if (xflags & FS_XFLAG_DAX)
> >  		di_flags2 |= XFS_DIFLAG2_DAX;
> > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > index 7158a8de719f..3e0c677cff15 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -25,6 +25,9 @@ xfs_check_limits(void)
> >  	/* make sure timestamp limits are correct */
> >  	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
> >  	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_EPOCH,			2147483648LL);
> > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MIN,			-2147483648LL);
> 
> That still just doesn't look right to me :/
> 
> This implies that the epoch is 2^32 seconds after then minimum
> supported time (2038), when in fact it is only 2^31 seconds after the
> minimum supported timestamp (1970). :/

Ok, so XFS_INO_UNIX_BIGTIME_MIN is -2147483648, to signify that the
smallest bigtime timestamp is (still) December 1901.

That thing currently known as XFS_INO_BIGTIME_EPOCH should probably get
renamed to something less confusing, like...

/*
 * Since the bigtime epoch is Dec. 1901, add this number of seconds to
 * an ondisk bigtime timestamp to convert it to the Unix epoch.
 */
#define XFS_BIGTIME_TO_UNIX		(-XFS_INO_UNIX_BIGTIME_MIN)

/*
 * Subtract this many seconds from a Unix epoch timestamp to get the
 * ondisk bigtime timestamp.
 */
#define XFS_UNIX_TO_BIGTIME		(-XFS_BIGTIME_TO_UNIX)

Is that clearer?

> > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MAX,			16299260425LL);
> 
> Hmmm. I got 16299260424 when I just ran this through a simple calc.
> Mind you, no calculator app I found could handle unsigned 64 bit
> values natively (signed 64 bit is good enough for everyone!) so
> maybe I got an off-by one here...

-1ULL = 18,446,744,073,709,551,615
-1ULL / NSEC_PER_SEC = 18,446,744,073
(-1ULL / NSEC_PER_SEC) - XFS_INO_BIGTIME_EPOCH = 16,299,260,425

Assuming you accept XFS_INO_BIGTIME_EPOCH being 2^31.

> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Dave Chinner]

On Sun, Aug 23, 2020 at 08:13:54PM -0700, Darrick J. Wong wrote:
> On Mon, Aug 25, 2020 at 11:25:27AM +1000, Dave Chinner wrote:
> > On Thu, Aug 20, 2020 at 07:12:21PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Redesign the ondisk timestamps to be a simple unsigned 64-bit counter of
> > > nanoseconds since 14 Dec 1901 (i.e. the minimum time in the 32-bit unix
> > > time epoch).  This enables us to handle dates up to 2486, which solves
> > > the y2038 problem.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > 
> > ....
> > 
> > > @@ -875,6 +888,25 @@ union xfs_timestamp {
> > >   */
> > >  #define XFS_INO_TIME_MAX	((int64_t)S32_MAX)
> > >  
> > > +/*
> > > + * Number of seconds between the start of the bigtime timestamp range and the
> > > + * start of the Unix epoch.
> > > + */
> > > +#define XFS_INO_BIGTIME_EPOCH	(-XFS_INO_TIME_MIN)
> > 
> > This is confusing. It's taken me 15 minutes so far to get my head
> > around this because the reference frame for all these definitions is
> > not clear. I though these had something to do with nanosecond
> > timestamp limits because that's what BIGTIME records, but.....
> > 
> > The start of the epoch is a negative number based on the definition
> > of the on-disk format for the minimum number of seconds that the
> > "Unix" timestamp format can store?  Why is this not defined in
> > nanoseconds given that is what is stored on disk?
> > 
> > XFS_INO_BIGTIME_EPOCH = (-XFS_INO_TIME_MIN)
> > 			= (-((int64_t)S32_MIN))
> > 			= (-((int64_t)-2^31))
> > 			= 2^31?
> > 
> > So the bigtime epoch is considered to be 2^31 *seconds* into the
> > range of the on-disk nanosecond timestamp? Huh?
> 
> They're the incore limits, not the ondisk limits.
> 
> Prior to bigtime, the ondisk timestamp epoch was the Unix epoch.  This
> isn't the case anymore in bigtime (bigtime's epoch is Dec. 1901, aka the
> minimum timestamp under the old scheme), so that misnamed
> XFS_INO_BIGTIME_EPOCH value is the conversion factor between epochs.
> 
> (I'll come back to this at the bottom.)

Ok, I'll come back to that at the bottom :)

> > > +		uint64_t		t = be64_to_cpu(ts->t_bigtime);
> > > +		uint64_t		s;
> > > +		uint32_t		n;
> > > +
> > > +		s = div_u64_rem(t, NSEC_PER_SEC, &n);
> > > +		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
> > > +		tv->tv_nsec = n;
> > > +		return;
> > > +	}
> > > +
> > >  	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
> > >  	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
> > >  }
> > 
> > I still don't really like the way this turned out :(
> 
> I'll think about this further and hope that hch comes up with something
> that's both functional and doesn't piss off smatch/sparse.  Note that I
> also don't have any big endian machines anymore, so I don't really have
> a good way to test this.  powerpc32 and sparc are verrrrry dead now.

I'm not sure that anyone has current BE machines to test on....

> > > +void xfs_inode_to_disk_timestamp(struct xfs_icdinode *from,
> > > +		union xfs_timestamp *ts, const struct timespec64 *tv);
> > >  
> > >  #endif	/* __XFS_INODE_BUF_H__ */
> > > diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> > > index 17c83d29998c..569721f7f9e5 100644
> > > --- a/fs/xfs/libxfs/xfs_log_format.h
> > > +++ b/fs/xfs/libxfs/xfs_log_format.h
> > > @@ -373,6 +373,9 @@ union xfs_ictimestamp {
> > >  		int32_t		t_sec;		/* timestamp seconds */
> > >  		int32_t		t_nsec;		/* timestamp nanoseconds */
> > >  	};
> > > +
> > > +	/* Nanoseconds since the bigtime epoch. */
> > > +	uint64_t		t_bigtime;
> > >  };
> > 
> > Where are we using this again? Right now the timestamps are
> > converted directly into the VFS inode timestamp fields so we can get
> > rid of these incore timestamp fields. So shouldn't we be trying to
> > get rid of this structure rather than adding more functionality to
> > it?
> 
> We would have to enlarge xfs_log_dinode to log a full timespec64-like
> entity.   I understand that it's annoying to convert a vfs timestamp
> back into a u64 nanoseconds counter for the sake of the log, but doing
> so will add complexity to the log for absolutely zero gain because
> having 96 bits per timestamp in the log doesn't buy us anything.

Sure, I understand that we only need to log a 64bit value, but we
don't actually need a structure for that as the log is in native
endian format. Hence it can just be a 64 bit field that we mask and
shift for !bigtime inodes...

Note that we have to be real careful about dynamic conversion,
especially in recovery, as the inode read from disk might be in
small time format, but logged and recovered in bigtime format. I
didn't actually check the recovery code does that correctly, because
it only just occurred to me that the logged timestamp format may not
match the inode flags read from disk during recovery...

> > > --- a/fs/xfs/xfs_inode.c
> > > +++ b/fs/xfs/xfs_inode.c
> > > @@ -841,6 +841,8 @@ xfs_ialloc(
> > >  	if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
> > >  		inode_set_iversion(inode, 1);
> > >  		ip->i_d.di_flags2 = 0;
> > > +		if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > > +			ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
> > 
> > Rather than calculate the initial inode falgs on every allocation,
> > shouldn't we just have the defaults pre-calculated at mount time?
> 
> Hm, yes.  Add that to the inode geometry structure?

Sounds like a reasonable place to me.

> > >  		ip->i_d.di_cowextsize = 0;
> > >  		ip->i_d.di_crtime = tv;
> > >  	}
> > > @@ -2717,7 +2719,11 @@ xfs_ifree(
> > >  
> > >  	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
> > >  	ip->i_d.di_flags = 0;
> > > -	ip->i_d.di_flags2 = 0;
> > > +	/*
> > > +	 * Preserve the bigtime flag so that di_ctime accurately stores the
> > > +	 * deletion time.
> > > +	 */
> > > +	ip->i_d.di_flags2 &= XFS_DIFLAG2_BIGTIME;
> > 
> > Oh, that's a nasty wart.
> 
> And here again?

*nod*. Good idea - we will have logged the inode core and converted
it in-core to bigtime by this point...

> > > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > > index 7158a8de719f..3e0c677cff15 100644
> > > --- a/fs/xfs/xfs_ondisk.h
> > > +++ b/fs/xfs/xfs_ondisk.h
> > > @@ -25,6 +25,9 @@ xfs_check_limits(void)
> > >  	/* make sure timestamp limits are correct */
> > >  	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
> > >  	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> > > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_EPOCH,			2147483648LL);
> > > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MIN,			-2147483648LL);
> > 
> > That still just doesn't look right to me :/
> > 
> > This implies that the epoch is 2^32 seconds after then minimum
> > supported time (2038), when in fact it is only 2^31 seconds after the
> > minimum supported timestamp (1970). :/
> 
> Ok, so XFS_INO_UNIX_BIGTIME_MIN is -2147483648, to signify that the
> smallest bigtime timestamp is (still) December 1901.

Let's drop the "ino" from the name - it's unnecessary, I think.

> That thing currently known as XFS_INO_BIGTIME_EPOCH should probably get
> renamed to something less confusing, like...
>
> /*
>  * Since the bigtime epoch is Dec. 1901, add this number of seconds to
>  * an ondisk bigtime timestamp to convert it to the Unix epoch.
>  */
> #define XFS_BIGTIME_TO_UNIX		(-XFS_INO_UNIX_BIGTIME_MIN)
> 
> /*
>  * Subtract this many seconds from a Unix epoch timestamp to get the
>  * ondisk bigtime timestamp.
>  */
> #define XFS_UNIX_TO_BIGTIME		(-XFS_BIGTIME_TO_UNIX)
> 
> Is that clearer?

Hmmm. Definitely better, but how about:

/*
 * Bigtime epoch is set exactly to the minimum time value that a
 * traditional 32 bit timestamp can represent when using the Unix
 * epoch as a reference. Hence the Unix epoch is at a fixed offset
 * into the supported bigtime timestamp range.
 *
 * The bigtime epoch also matches the minimum value an on-disk 32
 * bit XFS timestamp can represent so we will not lose any fidelity
 * in converting to/from unix and bigtime timestamps.
 */
#define XFS_BIGTIME_EPOCH_OFFSET	(XFS_INO_TIME_MIN)

And then two static inline helpers follow immediately -
xfs_bigtime_to_unix() and xfs_bigtime_from_unix() can do the
conversion between the two formats and the XFS_BIGTIME_EPOCH_OFFSET
variable never gets seen anywhere else in the code. To set the max
timestamp value the superblock holds for the filesystem, just
calculate it directly via a call to xfs_bigtime_to_unix(-1ULL, ...)

> > Hmmm. I got 16299260424 when I just ran this through a simple calc.
> > Mind you, no calculator app I found could handle unsigned 64 bit
> > values natively (signed 64 bit is good enough for everyone!) so
> > maybe I got an off-by one here...
> 
> -1ULL = 18,446,744,073,709,551,615
> -1ULL / NSEC_PER_SEC = 18,446,744,073
> (-1ULL / NSEC_PER_SEC) - XFS_INO_BIGTIME_EPOCH = 16,299,260,425

Yup, I got an off by one thanks to integer rounding on the
division. I should have just done it long hand like that...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Darrick J. Wong]

On Mon, Aug 24, 2020 at 04:15:31PM +1000, Dave Chinner wrote:
> On Sun, Aug 23, 2020 at 08:13:54PM -0700, Darrick J. Wong wrote:
> > On Mon, Aug 25, 2020 at 11:25:27AM +1000, Dave Chinner wrote:
> > > On Thu, Aug 20, 2020 at 07:12:21PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > 
> > > > Redesign the ondisk timestamps to be a simple unsigned 64-bit counter of
> > > > nanoseconds since 14 Dec 1901 (i.e. the minimum time in the 32-bit unix
> > > > time epoch).  This enables us to handle dates up to 2486, which solves
> > > > the y2038 problem.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > 
> > > ....
> > > 
> > > > @@ -875,6 +888,25 @@ union xfs_timestamp {
> > > >   */
> > > >  #define XFS_INO_TIME_MAX	((int64_t)S32_MAX)
> > > >  
> > > > +/*
> > > > + * Number of seconds between the start of the bigtime timestamp range and the
> > > > + * start of the Unix epoch.
> > > > + */
> > > > +#define XFS_INO_BIGTIME_EPOCH	(-XFS_INO_TIME_MIN)
> > > 
> > > This is confusing. It's taken me 15 minutes so far to get my head
> > > around this because the reference frame for all these definitions is
> > > not clear. I though these had something to do with nanosecond
> > > timestamp limits because that's what BIGTIME records, but.....
> > > 
> > > The start of the epoch is a negative number based on the definition
> > > of the on-disk format for the minimum number of seconds that the
> > > "Unix" timestamp format can store?  Why is this not defined in
> > > nanoseconds given that is what is stored on disk?
> > > 
> > > XFS_INO_BIGTIME_EPOCH = (-XFS_INO_TIME_MIN)
> > > 			= (-((int64_t)S32_MIN))
> > > 			= (-((int64_t)-2^31))
> > > 			= 2^31?
> > > 
> > > So the bigtime epoch is considered to be 2^31 *seconds* into the
> > > range of the on-disk nanosecond timestamp? Huh?
> > 
> > They're the incore limits, not the ondisk limits.
> > 
> > Prior to bigtime, the ondisk timestamp epoch was the Unix epoch.  This
> > isn't the case anymore in bigtime (bigtime's epoch is Dec. 1901, aka the
> > minimum timestamp under the old scheme), so that misnamed
> > XFS_INO_BIGTIME_EPOCH value is the conversion factor between epochs.
> > 
> > (I'll come back to this at the bottom.)
> 
> Ok, I'll come back to that at the bottom :)
> 
> > > > +		uint64_t		t = be64_to_cpu(ts->t_bigtime);
> > > > +		uint64_t		s;
> > > > +		uint32_t		n;
> > > > +
> > > > +		s = div_u64_rem(t, NSEC_PER_SEC, &n);
> > > > +		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
> > > > +		tv->tv_nsec = n;
> > > > +		return;
> > > > +	}
> > > > +
> > > >  	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
> > > >  	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
> > > >  }
> > > 
> > > I still don't really like the way this turned out :(
> > 
> > I'll think about this further and hope that hch comes up with something
> > that's both functional and doesn't piss off smatch/sparse.  Note that I
> > also don't have any big endian machines anymore, so I don't really have
> > a good way to test this.  powerpc32 and sparc are verrrrry dead now.
> 
> I'm not sure that anyone has current BE machines to test on....

...which makes me all the more nervous about replacing the timestamp
union with open-coded bit shifting.  We know the existing code does the
conversions properly with the separate sec/nsec fields since that code
has been around for a while.  We can use BUILD_BUG_ON macros to ensure
that inside the union, the bigtime nanoseconds counter is overlayed
/exactly/ on top of the old structure.  There's a feature flag within
the ondisk structure, which means that reasoning about this code is no
more difficult than any other tagged union.

Flag == 0?  Use the same old code from before.
Flag == 1?  Use the new code.

I was about to say that I'll experiment with this as a new patch at the
end of the series, but I guess converting xfs_timestamp back to a
typedef is more churn and belongs at the start of the series...

> > > > +void xfs_inode_to_disk_timestamp(struct xfs_icdinode *from,
> > > > +		union xfs_timestamp *ts, const struct timespec64 *tv);
> > > >  
> > > >  #endif	/* __XFS_INODE_BUF_H__ */
> > > > diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> > > > index 17c83d29998c..569721f7f9e5 100644
> > > > --- a/fs/xfs/libxfs/xfs_log_format.h
> > > > +++ b/fs/xfs/libxfs/xfs_log_format.h
> > > > @@ -373,6 +373,9 @@ union xfs_ictimestamp {
> > > >  		int32_t		t_sec;		/* timestamp seconds */
> > > >  		int32_t		t_nsec;		/* timestamp nanoseconds */
> > > >  	};
> > > > +
> > > > +	/* Nanoseconds since the bigtime epoch. */
> > > > +	uint64_t		t_bigtime;
> > > >  };
> > > 
> > > Where are we using this again? Right now the timestamps are
> > > converted directly into the VFS inode timestamp fields so we can get
> > > rid of these incore timestamp fields. So shouldn't we be trying to
> > > get rid of this structure rather than adding more functionality to
> > > it?
> > 
> > We would have to enlarge xfs_log_dinode to log a full timespec64-like
> > entity.   I understand that it's annoying to convert a vfs timestamp
> > back into a u64 nanoseconds counter for the sake of the log, but doing
> > so will add complexity to the log for absolutely zero gain because
> > having 96 bits per timestamp in the log doesn't buy us anything.
> 
> Sure, I understand that we only need to log a 64bit value, but we
> don't actually need a structure for that as the log is in native
> endian format. Hence it can just be a 64 bit field that we mask and
> shift for !bigtime inodes...
> 
> Note that we have to be real careful about dynamic conversion,
> especially in recovery, as the inode read from disk might be in
> small time format, but logged and recovered in bigtime format. I
> didn't actually check the recovery code does that correctly, because
> it only just occurred to me that the logged timestamp format may not
> match the inode flags read from disk during recovery...

Oh my, you're right, that xfs_log_dinode_to_disk_timestamp needs to be
more careful to convert whatever we logged into something that is
agnostic to disk format, and then convert it to whatever is the
xfs_dinode format.

I'll throw that on the fixme pile too.

> > > > --- a/fs/xfs/xfs_inode.c
> > > > +++ b/fs/xfs/xfs_inode.c
> > > > @@ -841,6 +841,8 @@ xfs_ialloc(
> > > >  	if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
> > > >  		inode_set_iversion(inode, 1);
> > > >  		ip->i_d.di_flags2 = 0;
> > > > +		if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > > > +			ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
> > > 
> > > Rather than calculate the initial inode falgs on every allocation,
> > > shouldn't we just have the defaults pre-calculated at mount time?
> > 
> > Hm, yes.  Add that to the inode geometry structure?
> 
> Sounds like a reasonable place to me.
> 
> > > >  		ip->i_d.di_cowextsize = 0;
> > > >  		ip->i_d.di_crtime = tv;
> > > >  	}
> > > > @@ -2717,7 +2719,11 @@ xfs_ifree(
> > > >  
> > > >  	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
> > > >  	ip->i_d.di_flags = 0;
> > > > -	ip->i_d.di_flags2 = 0;
> > > > +	/*
> > > > +	 * Preserve the bigtime flag so that di_ctime accurately stores the
> > > > +	 * deletion time.
> > > > +	 */
> > > > +	ip->i_d.di_flags2 &= XFS_DIFLAG2_BIGTIME;
> > > 
> > > Oh, that's a nasty wart.
> > 
> > And here again?
> 
> *nod*. Good idea - we will have logged the inode core and converted
> it in-core to bigtime by this point...
> 
> > > > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > > > index 7158a8de719f..3e0c677cff15 100644
> > > > --- a/fs/xfs/xfs_ondisk.h
> > > > +++ b/fs/xfs/xfs_ondisk.h
> > > > @@ -25,6 +25,9 @@ xfs_check_limits(void)
> > > >  	/* make sure timestamp limits are correct */
> > > >  	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
> > > >  	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> > > > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_EPOCH,			2147483648LL);
> > > > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MIN,			-2147483648LL);
> > > 
> > > That still just doesn't look right to me :/
> > > 
> > > This implies that the epoch is 2^32 seconds after then minimum
> > > supported time (2038), when in fact it is only 2^31 seconds after the
> > > minimum supported timestamp (1970). :/
> > 
> > Ok, so XFS_INO_UNIX_BIGTIME_MIN is -2147483648, to signify that the
> > smallest bigtime timestamp is (still) December 1901.
> 
> Let's drop the "ino" from the name - it's unnecessary, I think.

Ok.

> > That thing currently known as XFS_INO_BIGTIME_EPOCH should probably get
> > renamed to something less confusing, like...
> >
> > /*
> >  * Since the bigtime epoch is Dec. 1901, add this number of seconds to
> >  * an ondisk bigtime timestamp to convert it to the Unix epoch.
> >  */
> > #define XFS_BIGTIME_TO_UNIX		(-XFS_INO_UNIX_BIGTIME_MIN)
> > 
> > /*
> >  * Subtract this many seconds from a Unix epoch timestamp to get the
> >  * ondisk bigtime timestamp.
> >  */
> > #define XFS_UNIX_TO_BIGTIME		(-XFS_BIGTIME_TO_UNIX)
> > 
> > Is that clearer?
> 
> Hmmm. Definitely better, but how about:
> 
> /*
>  * Bigtime epoch is set exactly to the minimum time value that a
>  * traditional 32 bit timestamp can represent when using the Unix
>  * epoch as a reference. Hence the Unix epoch is at a fixed offset
>  * into the supported bigtime timestamp range.
>  *
>  * The bigtime epoch also matches the minimum value an on-disk 32
>  * bit XFS timestamp can represent so we will not lose any fidelity
>  * in converting to/from unix and bigtime timestamps.
>  */
> #define XFS_BIGTIME_EPOCH_OFFSET	(XFS_INO_TIME_MIN)
> 
> And then two static inline helpers follow immediately -
> xfs_bigtime_to_unix() and xfs_bigtime_from_unix() can do the
> conversion between the two formats and the XFS_BIGTIME_EPOCH_OFFSET
> variable never gets seen anywhere else in the code. To set the max
> timestamp value the superblock holds for the filesystem, just
> calculate it directly via a call to xfs_bigtime_to_unix(-1ULL, ...)

<nod>

--D

> > > Hmmm. I got 16299260424 when I just ran this through a simple calc.
> > > Mind you, no calculator app I found could handle unsigned 64 bit
> > > values natively (signed 64 bit is good enough for everyone!) so
> > > maybe I got an off-by one here...
> > 
> > -1ULL = 18,446,744,073,709,551,615
> > -1ULL / NSEC_PER_SEC = 18,446,744,073
> > (-1ULL / NSEC_PER_SEC) - XFS_INO_BIGTIME_EPOCH = 16,299,260,425
> 
> Yup, I got an off by one thanks to integer rounding on the
> division. I should have just done it long hand like that...
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

Re: [PATCH 01/11] xfs: explicitly define inode timestamp range

[Darrick J. Wong]

On Sat, Aug 22, 2020 at 08:12:18AM +0100, Christoph Hellwig wrote:
> > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > index acb9b737fe6b..48a64fa49f91 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -15,6 +15,18 @@
> >  		"XFS: offsetof(" #structname ", " #member ") is wrong, " \
> >  		"expected " #off)
> >  
> > +#define XFS_CHECK_VALUE(value, expected) \
> > +	BUILD_BUG_ON_MSG((value) != (expected), \
> > +		"XFS: value of " #value " is wrong, expected " #expected)
> > +
> > +static inline void __init
> > +xfs_check_limits(void)
> > +{
> > +	/* make sure timestamp limits are correct */
> > +	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
> > +	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> 
> I have to admit I don't get why you'd define a constant and then
> check that it has the value you defined it to.  Seems a little cargo
> cult.

Testing the timestamp min/max is more important for bigtime, since the
supported ranges are not conveniently aligned with S32_MIN/MAX.  But it
felt strange to build-check bigtime without doing the same for !bigtime,
so I included it here for completeness.

--D

Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Darrick J. Wong]

On Mon, Aug 24, 2020 at 09:24:20AM -0700, Darrick J. Wong wrote:
> On Mon, Aug 24, 2020 at 04:15:31PM +1000, Dave Chinner wrote:
> > On Sun, Aug 23, 2020 at 08:13:54PM -0700, Darrick J. Wong wrote:
> > > On Mon, Aug 25, 2020 at 11:25:27AM +1000, Dave Chinner wrote:
> > > > On Thu, Aug 20, 2020 at 07:12:21PM -0700, Darrick J. Wong wrote:
> > > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > > 
> > > > > Redesign the ondisk timestamps to be a simple unsigned 64-bit counter of
> > > > > nanoseconds since 14 Dec 1901 (i.e. the minimum time in the 32-bit unix
> > > > > time epoch).  This enables us to handle dates up to 2486, which solves
> > > > > the y2038 problem.
> > > > > 
> > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > > 
> > > > ....
> > > > 
> > > > > @@ -875,6 +888,25 @@ union xfs_timestamp {
> > > > >   */
> > > > >  #define XFS_INO_TIME_MAX	((int64_t)S32_MAX)
> > > > >  
> > > > > +/*
> > > > > + * Number of seconds between the start of the bigtime timestamp range and the
> > > > > + * start of the Unix epoch.
> > > > > + */
> > > > > +#define XFS_INO_BIGTIME_EPOCH	(-XFS_INO_TIME_MIN)
> > > > 
> > > > This is confusing. It's taken me 15 minutes so far to get my head
> > > > around this because the reference frame for all these definitions is
> > > > not clear. I though these had something to do with nanosecond
> > > > timestamp limits because that's what BIGTIME records, but.....
> > > > 
> > > > The start of the epoch is a negative number based on the definition
> > > > of the on-disk format for the minimum number of seconds that the
> > > > "Unix" timestamp format can store?  Why is this not defined in
> > > > nanoseconds given that is what is stored on disk?
> > > > 
> > > > XFS_INO_BIGTIME_EPOCH = (-XFS_INO_TIME_MIN)
> > > > 			= (-((int64_t)S32_MIN))
> > > > 			= (-((int64_t)-2^31))
> > > > 			= 2^31?
> > > > 
> > > > So the bigtime epoch is considered to be 2^31 *seconds* into the
> > > > range of the on-disk nanosecond timestamp? Huh?
> > > 
> > > They're the incore limits, not the ondisk limits.
> > > 
> > > Prior to bigtime, the ondisk timestamp epoch was the Unix epoch.  This
> > > isn't the case anymore in bigtime (bigtime's epoch is Dec. 1901, aka the
> > > minimum timestamp under the old scheme), so that misnamed
> > > XFS_INO_BIGTIME_EPOCH value is the conversion factor between epochs.
> > > 
> > > (I'll come back to this at the bottom.)
> > 
> > Ok, I'll come back to that at the bottom :)
> > 
> > > > > +		uint64_t		t = be64_to_cpu(ts->t_bigtime);
> > > > > +		uint64_t		s;
> > > > > +		uint32_t		n;
> > > > > +
> > > > > +		s = div_u64_rem(t, NSEC_PER_SEC, &n);
> > > > > +		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
> > > > > +		tv->tv_nsec = n;
> > > > > +		return;
> > > > > +	}
> > > > > +
> > > > >  	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
> > > > >  	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
> > > > >  }
> > > > 
> > > > I still don't really like the way this turned out :(
> > > 
> > > I'll think about this further and hope that hch comes up with something
> > > that's both functional and doesn't piss off smatch/sparse.  Note that I
> > > also don't have any big endian machines anymore, so I don't really have
> > > a good way to test this.  powerpc32 and sparc are verrrrry dead now.
> > 
> > I'm not sure that anyone has current BE machines to test on....
> 
> ...which makes me all the more nervous about replacing the timestamp
> union with open-coded bit shifting.  We know the existing code does the
> conversions properly with the separate sec/nsec fields since that code
> has been around for a while.  We can use BUILD_BUG_ON macros to ensure
> that inside the union, the bigtime nanoseconds counter is overlayed
> /exactly/ on top of the old structure.  There's a feature flag within
> the ondisk structure, which means that reasoning about this code is no
> more difficult than any other tagged union.
> 
> Flag == 0?  Use the same old code from before.
> Flag == 1?  Use the new code.
> 
> I was about to say that I'll experiment with this as a new patch at the
> end of the series, but I guess converting xfs_timestamp back to a
> typedef is more churn and belongs at the start of the series...
> 
> > > > > +void xfs_inode_to_disk_timestamp(struct xfs_icdinode *from,
> > > > > +		union xfs_timestamp *ts, const struct timespec64 *tv);
> > > > >  
> > > > >  #endif	/* __XFS_INODE_BUF_H__ */
> > > > > diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> > > > > index 17c83d29998c..569721f7f9e5 100644
> > > > > --- a/fs/xfs/libxfs/xfs_log_format.h
> > > > > +++ b/fs/xfs/libxfs/xfs_log_format.h
> > > > > @@ -373,6 +373,9 @@ union xfs_ictimestamp {
> > > > >  		int32_t		t_sec;		/* timestamp seconds */
> > > > >  		int32_t		t_nsec;		/* timestamp nanoseconds */
> > > > >  	};
> > > > > +
> > > > > +	/* Nanoseconds since the bigtime epoch. */
> > > > > +	uint64_t		t_bigtime;
> > > > >  };
> > > > 
> > > > Where are we using this again? Right now the timestamps are
> > > > converted directly into the VFS inode timestamp fields so we can get
> > > > rid of these incore timestamp fields. So shouldn't we be trying to
> > > > get rid of this structure rather than adding more functionality to
> > > > it?
> > > 
> > > We would have to enlarge xfs_log_dinode to log a full timespec64-like
> > > entity.   I understand that it's annoying to convert a vfs timestamp
> > > back into a u64 nanoseconds counter for the sake of the log, but doing
> > > so will add complexity to the log for absolutely zero gain because
> > > having 96 bits per timestamp in the log doesn't buy us anything.
> > 
> > Sure, I understand that we only need to log a 64bit value, but we
> > don't actually need a structure for that as the log is in native
> > endian format. Hence it can just be a 64 bit field that we mask and
> > shift for !bigtime inodes...
> > 
> > Note that we have to be real careful about dynamic conversion,
> > especially in recovery, as the inode read from disk might be in
> > small time format, but logged and recovered in bigtime format. I
> > didn't actually check the recovery code does that correctly, because
> > it only just occurred to me that the logged timestamp format may not
> > match the inode flags read from disk during recovery...
> 
> Oh my, you're right, that xfs_log_dinode_to_disk_timestamp needs to be
> more careful to convert whatever we logged into something that is
> agnostic to disk format, and then convert it to whatever is the
> xfs_dinode format.

On second thought, I think the inode logging code handles the timestamp
format correctly as it is now written because we always log the inode
core even if we're doing an XFS_ILOG_TIMESTAMP update, right?  Which
means that we always log the timestamps along with flags2 in
xfs_log_dinode, and therefore there's no chance for inconsistency.

--D

> I'll throw that on the fixme pile too.
> 
> > > > > --- a/fs/xfs/xfs_inode.c
> > > > > +++ b/fs/xfs/xfs_inode.c
> > > > > @@ -841,6 +841,8 @@ xfs_ialloc(
> > > > >  	if (xfs_sb_version_has_v3inode(&mp->m_sb)) {
> > > > >  		inode_set_iversion(inode, 1);
> > > > >  		ip->i_d.di_flags2 = 0;
> > > > > +		if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > > > > +			ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
> > > > 
> > > > Rather than calculate the initial inode falgs on every allocation,
> > > > shouldn't we just have the defaults pre-calculated at mount time?
> > > 
> > > Hm, yes.  Add that to the inode geometry structure?
> > 
> > Sounds like a reasonable place to me.
> > 
> > > > >  		ip->i_d.di_cowextsize = 0;
> > > > >  		ip->i_d.di_crtime = tv;
> > > > >  	}
> > > > > @@ -2717,7 +2719,11 @@ xfs_ifree(
> > > > >  
> > > > >  	VFS_I(ip)->i_mode = 0;		/* mark incore inode as free */
> > > > >  	ip->i_d.di_flags = 0;
> > > > > -	ip->i_d.di_flags2 = 0;
> > > > > +	/*
> > > > > +	 * Preserve the bigtime flag so that di_ctime accurately stores the
> > > > > +	 * deletion time.
> > > > > +	 */
> > > > > +	ip->i_d.di_flags2 &= XFS_DIFLAG2_BIGTIME;
> > > > 
> > > > Oh, that's a nasty wart.
> > > 
> > > And here again?
> > 
> > *nod*. Good idea - we will have logged the inode core and converted
> > it in-core to bigtime by this point...
> > 
> > > > > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > > > > index 7158a8de719f..3e0c677cff15 100644
> > > > > --- a/fs/xfs/xfs_ondisk.h
> > > > > +++ b/fs/xfs/xfs_ondisk.h
> > > > > @@ -25,6 +25,9 @@ xfs_check_limits(void)
> > > > >  	/* make sure timestamp limits are correct */
> > > > >  	XFS_CHECK_VALUE(XFS_INO_TIME_MIN,			-2147483648LL);
> > > > >  	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> > > > > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_EPOCH,			2147483648LL);
> > > > > +	XFS_CHECK_VALUE(XFS_INO_BIGTIME_MIN,			-2147483648LL);
> > > > 
> > > > That still just doesn't look right to me :/
> > > > 
> > > > This implies that the epoch is 2^32 seconds after then minimum
> > > > supported time (2038), when in fact it is only 2^31 seconds after the
> > > > minimum supported timestamp (1970). :/
> > > 
> > > Ok, so XFS_INO_UNIX_BIGTIME_MIN is -2147483648, to signify that the
> > > smallest bigtime timestamp is (still) December 1901.
> > 
> > Let's drop the "ino" from the name - it's unnecessary, I think.
> 
> Ok.
> 
> > > That thing currently known as XFS_INO_BIGTIME_EPOCH should probably get
> > > renamed to something less confusing, like...
> > >
> > > /*
> > >  * Since the bigtime epoch is Dec. 1901, add this number of seconds to
> > >  * an ondisk bigtime timestamp to convert it to the Unix epoch.
> > >  */
> > > #define XFS_BIGTIME_TO_UNIX		(-XFS_INO_UNIX_BIGTIME_MIN)
> > > 
> > > /*
> > >  * Subtract this many seconds from a Unix epoch timestamp to get the
> > >  * ondisk bigtime timestamp.
> > >  */
> > > #define XFS_UNIX_TO_BIGTIME		(-XFS_BIGTIME_TO_UNIX)
> > > 
> > > Is that clearer?
> > 
> > Hmmm. Definitely better, but how about:
> > 
> > /*
> >  * Bigtime epoch is set exactly to the minimum time value that a
> >  * traditional 32 bit timestamp can represent when using the Unix
> >  * epoch as a reference. Hence the Unix epoch is at a fixed offset
> >  * into the supported bigtime timestamp range.
> >  *
> >  * The bigtime epoch also matches the minimum value an on-disk 32
> >  * bit XFS timestamp can represent so we will not lose any fidelity
> >  * in converting to/from unix and bigtime timestamps.
> >  */
> > #define XFS_BIGTIME_EPOCH_OFFSET	(XFS_INO_TIME_MIN)
> > 
> > And then two static inline helpers follow immediately -
> > xfs_bigtime_to_unix() and xfs_bigtime_from_unix() can do the
> > conversion between the two formats and the XFS_BIGTIME_EPOCH_OFFSET
> > variable never gets seen anywhere else in the code. To set the max
> > timestamp value the superblock holds for the filesystem, just
> > calculate it directly via a call to xfs_bigtime_to_unix(-1ULL, ...)
> 
> <nod>
> 
> --D
> 
> > > > Hmmm. I got 16299260424 when I just ran this through a simple calc.
> > > > Mind you, no calculator app I found could handle unsigned 64 bit
> > > > values natively (signed 64 bit is good enough for everyone!) so
> > > > maybe I got an off-by one here...
> > > 
> > > -1ULL = 18,446,744,073,709,551,615
> > > -1ULL / NSEC_PER_SEC = 18,446,744,073
> > > (-1ULL / NSEC_PER_SEC) - XFS_INO_BIGTIME_EPOCH = 16,299,260,425
> > 
> > Yup, I got an off by one thanks to integer rounding on the
> > division. I should have just done it long hand like that...
> > 
> > Cheers,
> > 
> > Dave.
> > -- 
> > Dave Chinner
> > david@fromorbit.com

Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

[Darrick J. Wong]

On Sun, Aug 23, 2020 at 07:43:41PM -0700, Darrick J. Wong wrote:
> On Sat, Aug 22, 2020 at 08:33:19AM +0100, Christoph Hellwig wrote:
> > >   * in the AGI header so that we can skip the finobt walk at mount time when
> > > @@ -855,12 +862,18 @@ struct xfs_agfl {
> > >   *
> > >   * Inode timestamps consist of signed 32-bit counters for seconds and
> > >   * nanoseconds; time zero is the Unix epoch, Jan  1 00:00:00 UTC 1970.
> > > + *
> > > + * When bigtime is enabled, timestamps become an unsigned 64-bit nanoseconds
> > > + * counter.  Time zero is the start of the classic timestamp range.
> > >   */
> > >  union xfs_timestamp {
> > >  	struct {
> > >  		__be32		t_sec;		/* timestamp seconds */
> > >  		__be32		t_nsec;		/* timestamp nanoseconds */
> > >  	};
> > > +
> > > +	/* Nanoseconds since the bigtime epoch. */
> > > +	__be64			t_bigtime;
> > >  };
> > 
> > So do we really need the union here?  What about:
> > 
> >  (1) keep the typedef instead of removing it
> >  (2) switch the typedef to be just a __be64, and use trivial helpers
> >      to extract the two separate legacy sec/nsec field
> >  (3) PROFIT!!!
> 
> Been there, done that.  Dave suggested some replacement code (which
> corrupted the values), then I modified that into a correct version,
> which then made smatch angry because it doesn't like code that does bit
> shifts on __be64 values.

Backing up here, I've realized that my own analysis of Dave's pseudocode
was incorrect.

On a little endian machine, we'll start with the following.  A is the
LSB of seconds; D is the MSB of seconds; E is the LSB of nsec, and H is
the MSB of nsec.

  sec  nsec (incore)
  l  m l  m
  ABCD EFGH

Now we encode that with an old kernel, which calls cpu_to_be32 to turn
that into:

  sec  nsec (ondisk)
  m  l m  l
  DCBA HGFE

Move over to a new kernel, and that becomes:

  tstamp (ondisk)
  m      l
  DCBAHGFE

Next we decode with be64_to_cpu:

  tstamp (incore)
  l      m
  EFGHABCD

Now we extract nsec from (tstamp & -1U) and sec from (tstamp >> 32):

  sec  nsec
  l  m l  m
  ABCD EFGH

So yes, masking and shifting /after/ the endian conversion works just
fine and doesn't throw any sparse/smatch errors.

Now on a big endian machine:

  sec  nsec (incore)
  m  l m  l
  DCBA HGFE

Now we encode that with an old kernel, which calls cpu_to_be32 (a nop)
to turn that into:

  sec  nsec (ondisk)
  m  l m  l
  DCBA HGFE

Move over to a new kernel, and that becomes:

  tstamp (ondisk)
  m      l
  DCBAHGFE

Next we decode with be64_to_cpu (a nop):

  tstamp (incore)
  m      l
  DCBAHGFE

Now we extract nsec from (tstamp & -1U) and sec from (tstamp >> 32):

  sec  nsec
  m  l m  l
  DCBA HGFE

Works fine here too.

Now the /truly/ nasty case here is xfs_ictimestamp, since we log the
inode core in host endian format.  If we start with this the vfs
timestamp on a new kernel:

  sec  nsec (incore)
  l  m l  m
  ABCD EFGH

We need to encode that as:

  tstamp (ondisk)
  l      m
  ABCDEFGH

The only way to do this is: (nsec << 32) | (sec & -1U).  That makes the
log timestamp encoding is the opposite of what we do for the ondisk
inodes, because log formats don't use cpu_to_be64.

At least for a big endian machine, log timestamp coding is easy:

  sec  nsec (incore)
  m  l m  l
  DCBA HGFE

We need to encode that as:

  tstamp (ondisk)
  m      l
  DCBAHGFE

And the only way to get there is (sec << 32) | (nsec & -1U), which is
what the ondisk inode timestamp coding does.

I still think this is grody, but at least now now I have a new fstest to
make sure that log recovery doesn't trip over this.  So, you were
technically right and I was wrong.  We'll see how you like the new
stuff. ;)

--D

> > > +/* Convert an ondisk timestamp into the 64-bit safe incore format. */
> > >  void
> > >  xfs_inode_from_disk_timestamp(
> > > +	struct xfs_dinode		*dip,
> > >  	struct timespec64		*tv,
> > >  	const union xfs_timestamp	*ts)
> > 
> > I think passing ts by value might lead to somewhat better code
> > generation on modern ABIs (and older ABIs just fall back to pass
> > by reference transparently).
> 
> Hm, ok.  I did not know that. :)
> 
> > >  {
> > > +	if (dip->di_version >= 3 &&
> > > +	    (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) {
> > 
> > Do we want a helper for this condition?
> 
> Yes, yes we do.  Will add.
> 
> > > +		uint64_t		t = be64_to_cpu(ts->t_bigtime);
> > > +		uint64_t		s;
> > > +		uint32_t		n;
> > > +
> > > +		s = div_u64_rem(t, NSEC_PER_SEC, &n);
> > > +		tv->tv_sec = s - XFS_INO_BIGTIME_EPOCH;
> > > +		tv->tv_nsec = n;
> > > +		return;
> > > +	}
> > > +
> > >  	tv->tv_sec = (int)be32_to_cpu(ts->t_sec);
> > >  	tv->tv_nsec = (int)be32_to_cpu(ts->t_nsec);
> > 
> > Nit: for these kinds of symmetric conditions and if/else feels a little
> > more natural.
> > 
> > > +		xfs_log_dinode_to_disk_ts(from, &to->di_crtime, &from->di_crtime);
> > 
> > This adds a > 80 char line.
> 
> Do we care now that checkpatch has been changed to allow up to 100
> columns?
> 
> > > +	if (from->di_flags2 & XFS_DIFLAG2_BIGTIME) {
> > > +		uint64_t		t;
> > > +
> > > +		t = (uint64_t)(ts->tv_sec + XFS_INO_BIGTIME_EPOCH);
> > > +		t *= NSEC_PER_SEC;
> > > +		its->t_bigtime = t + ts->tv_nsec;
> > 
> > This calculation is dupliated in two places, might be worth
> > adding a little helper (which will need to get the sec/nsec values
> > passed separately due to the different structures).
> > 
> > > +		xfs_inode_to_log_dinode_ts(from, &to->di_crtime, &from->di_crtime);
> > 
> > Another line over 8 characters here.
> > 
> > > +	if (xfs_sb_version_hasbigtime(&mp->m_sb)) {
> > > +		sb->s_time_min = XFS_INO_BIGTIME_MIN;
> > > +		sb->s_time_max = XFS_INO_BIGTIME_MAX;
> > > +	} else {
> > > +		sb->s_time_min = XFS_INO_TIME_MIN;
> > > +		sb->s_time_max = XFS_INO_TIME_MAX;
> > > +	}
> > 
> > This is really a comment on the earlier patch, but maybe we should
> > name the old constants with "OLD" or "LEGACY" or "SMALL" in the name?
> 
> Yes, good suggestion!
> 
> > > @@ -1494,6 +1499,10 @@ xfs_fc_fill_super(
> > >  	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
> > >  		sb->s_flags |= SB_I_VERSION;
> > >  
> > > +	if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > > +		xfs_warn(mp,
> > > + "EXPERIMENTAL big timestamp feature in use. Use at your own risk!");
> > > +
> > 
> > Is there any good reason to mark this experimental?
> 
> As you and Dave have both pointed out, there are plenty of stupid bugs
> still in this.  I think I'd like to have at least one EXPERIMENTAL cycle
> to make sure I didn't commit anything pathologically stupid in here.
> 
> <cough> ext4 34-bit sign extension bug <cough>.
> 
> --D

[PATCH 03/11] xfs: refactor default quota grace period setting code

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor the code that sets the default quota grace period into a helper
function so that we can override the ondisk behavior later.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
---
 fs/xfs/libxfs/xfs_format.h |   13 +++++++++++++
 fs/xfs/xfs_dquot.c         |    8 ++++++++
 fs/xfs/xfs_dquot.h         |    1 +
 fs/xfs/xfs_qm_syscalls.c   |    4 ++--
 4 files changed, 24 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index cb316053d3db..4b68a473b090 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1209,6 +1209,11 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  * been reached, and therefore no expiration has been set.  Therefore, the
  * ondisk min and max defined here can be used directly to constrain the incore
  * quota expiration timestamps on a Unix system.
+ *
+ * The grace period for each quota type is stored in the root dquot (id = 0)
+ * and is applied to a non-root dquot when it exceeds the soft or hard limits.
+ * The length of quota grace periods are unsigned 32-bit quantities measured in
+ * units of seconds.  A value of zero means to use the default period.
  */
 
 /*
@@ -1223,6 +1228,14 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  */
 #define XFS_DQ_LEGACY_EXPIRY_MAX	((int64_t)U32_MAX)
 
+/*
+ * Default quota grace periods, ranging from zero (use the compiled defaults)
+ * to ~136 years.  These are applied to a non-root dquot that has exceeded
+ * either limit.
+ */
+#define XFS_DQ_GRACE_MIN		((int64_t)0)
+#define XFS_DQ_GRACE_MAX		((int64_t)U32_MAX)
+
 /*
  * This is the main portion of the on-disk representation of quota information
  * for a user.  We pad this with some more expansion room to construct the on
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index f34841f98d44..e63a933413a3 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -110,6 +110,14 @@ xfs_dquot_set_timeout(
 					  qi->qi_expiry_max);
 }
 
+/* Set the length of the default grace period. */
+time64_t
+xfs_dquot_set_grace_period(
+	time64_t		grace)
+{
+	return clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN, XFS_DQ_GRACE_MAX);
+}
+
 /*
  * Determine if this quota counter is over either limit and set the quota
  * timers as appropriate.
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 0e449101c861..f642884a6834 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -238,5 +238,6 @@ int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type,
 		xfs_qm_dqiterate_fn iter_fn, void *priv);
 
 time64_t xfs_dquot_set_timeout(struct xfs_mount *mp, time64_t timeout);
+time64_t xfs_dquot_set_grace_period(time64_t grace);
 
 #endif /* __XFS_DQUOT_H__ */
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 750f775ae915..ca1b57d291dc 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -486,8 +486,8 @@ xfs_setqlim_timer(
 {
 	if (qlim) {
 		/* Set the length of the default grace period. */
-		res->timer = timer;
-		qlim->time = timer;
+		res->timer = xfs_dquot_set_grace_period(timer);
+		qlim->time = res->timer;
 	} else {
 		/* Set the grace period expiration on a quota. */
 		res->timer = xfs_dquot_set_timeout(mp, timer);

[PATCH 03/11] xfs: refactor default quota grace period setting code

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor the code that sets the default quota grace period into a helper
function so that we can override the ondisk behavior later.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
---
 fs/xfs/libxfs/xfs_format.h |   13 +++++++++++++
 fs/xfs/xfs_dquot.c         |    8 ++++++++
 fs/xfs/xfs_dquot.h         |    1 +
 fs/xfs/xfs_qm_syscalls.c   |    4 ++--
 4 files changed, 24 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index cb316053d3db..4b68a473b090 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1209,6 +1209,11 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  * been reached, and therefore no expiration has been set.  Therefore, the
  * ondisk min and max defined here can be used directly to constrain the incore
  * quota expiration timestamps on a Unix system.
+ *
+ * The grace period for each quota type is stored in the root dquot (id = 0)
+ * and is applied to a non-root dquot when it exceeds the soft or hard limits.
+ * The length of quota grace periods are unsigned 32-bit quantities measured in
+ * units of seconds.  A value of zero means to use the default period.
  */
 
 /*
@@ -1223,6 +1228,14 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  */
 #define XFS_DQ_LEGACY_EXPIRY_MAX	((int64_t)U32_MAX)
 
+/*
+ * Default quota grace periods, ranging from zero (use the compiled defaults)
+ * to ~136 years.  These are applied to a non-root dquot that has exceeded
+ * either limit.
+ */
+#define XFS_DQ_GRACE_MIN		((int64_t)0)
+#define XFS_DQ_GRACE_MAX		((int64_t)U32_MAX)
+
 /*
  * This is the main portion of the on-disk representation of quota information
  * for a user.  We pad this with some more expansion room to construct the on
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index f34841f98d44..e63a933413a3 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -110,6 +110,14 @@ xfs_dquot_set_timeout(
 					  qi->qi_expiry_max);
 }
 
+/* Set the length of the default grace period. */
+time64_t
+xfs_dquot_set_grace_period(
+	time64_t		grace)
+{
+	return clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN, XFS_DQ_GRACE_MAX);
+}
+
 /*
  * Determine if this quota counter is over either limit and set the quota
  * timers as appropriate.
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 0e449101c861..f642884a6834 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -238,5 +238,6 @@ int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type,
 		xfs_qm_dqiterate_fn iter_fn, void *priv);
 
 time64_t xfs_dquot_set_timeout(struct xfs_mount *mp, time64_t timeout);
+time64_t xfs_dquot_set_grace_period(time64_t grace);
 
 #endif /* __XFS_DQUOT_H__ */
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 750f775ae915..ca1b57d291dc 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -486,8 +486,8 @@ xfs_setqlim_timer(
 {
 	if (qlim) {
 		/* Set the length of the default grace period. */
-		res->timer = timer;
-		qlim->time = timer;
+		res->timer = xfs_dquot_set_grace_period(timer);
+		qlim->time = res->timer;
 	} else {
 		/* Set the grace period expiration on a quota. */
 		res->timer = xfs_dquot_set_timeout(mp, timer);

Re: [PATCH 03/11] xfs: refactor default quota grace period setting code

[Allison Collins]

On 8/26/20 3:05 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor the code that sets the default quota grace period into a helper
> function so that we can override the ondisk behavior later.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Looks ok
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   fs/xfs/libxfs/xfs_format.h |   13 +++++++++++++
>   fs/xfs/xfs_dquot.c         |    8 ++++++++
>   fs/xfs/xfs_dquot.h         |    1 +
>   fs/xfs/xfs_qm_syscalls.c   |    4 ++--
>   4 files changed, 24 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index cb316053d3db..4b68a473b090 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1209,6 +1209,11 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>    * been reached, and therefore no expiration has been set.  Therefore, the
>    * ondisk min and max defined here can be used directly to constrain the incore
>    * quota expiration timestamps on a Unix system.
> + *
> + * The grace period for each quota type is stored in the root dquot (id = 0)
> + * and is applied to a non-root dquot when it exceeds the soft or hard limits.
> + * The length of quota grace periods are unsigned 32-bit quantities measured in
> + * units of seconds.  A value of zero means to use the default period.
>    */
>   
>   /*
> @@ -1223,6 +1228,14 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>    */
>   #define XFS_DQ_LEGACY_EXPIRY_MAX	((int64_t)U32_MAX)
>   
> +/*
> + * Default quota grace periods, ranging from zero (use the compiled defaults)
> + * to ~136 years.  These are applied to a non-root dquot that has exceeded
> + * either limit.
> + */
> +#define XFS_DQ_GRACE_MIN		((int64_t)0)
> +#define XFS_DQ_GRACE_MAX		((int64_t)U32_MAX)
> +
>   /*
>    * This is the main portion of the on-disk representation of quota information
>    * for a user.  We pad this with some more expansion room to construct the on
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index f34841f98d44..e63a933413a3 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -110,6 +110,14 @@ xfs_dquot_set_timeout(
>   					  qi->qi_expiry_max);
>   }
>   
> +/* Set the length of the default grace period. */
> +time64_t
> +xfs_dquot_set_grace_period(
> +	time64_t		grace)
> +{
> +	return clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN, XFS_DQ_GRACE_MAX);
> +}
> +
>   /*
>    * Determine if this quota counter is over either limit and set the quota
>    * timers as appropriate.
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index 0e449101c861..f642884a6834 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -238,5 +238,6 @@ int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type,
>   		xfs_qm_dqiterate_fn iter_fn, void *priv);
>   
>   time64_t xfs_dquot_set_timeout(struct xfs_mount *mp, time64_t timeout);
> +time64_t xfs_dquot_set_grace_period(time64_t grace);
>   
>   #endif /* __XFS_DQUOT_H__ */
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 750f775ae915..ca1b57d291dc 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -486,8 +486,8 @@ xfs_setqlim_timer(
>   {
>   	if (qlim) {
>   		/* Set the length of the default grace period. */
> -		res->timer = timer;
> -		qlim->time = timer;
> +		res->timer = xfs_dquot_set_grace_period(timer);
> +		qlim->time = res->timer;
>   	} else {
>   		/* Set the grace period expiration on a quota. */
>   		res->timer = xfs_dquot_set_timeout(mp, timer);
> 

Re: [PATCH 03/11] xfs: refactor default quota grace period setting code

[Christoph Hellwig]

On Wed, Aug 26, 2020 at 03:05:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor the code that sets the default quota grace period into a helper
> function so that we can override the ondisk behavior later.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 03/11] xfs: refactor default quota grace period setting code

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor the code that sets the default quota grace period into a helper
function so that we can override the ondisk behavior later.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_format.h |   13 +++++++++++++
 fs/xfs/xfs_dquot.c         |    8 ++++++++
 fs/xfs/xfs_dquot.h         |    1 +
 fs/xfs/xfs_qm_syscalls.c   |    4 ++--
 4 files changed, 24 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index cb316053d3db..4b68a473b090 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1209,6 +1209,11 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  * been reached, and therefore no expiration has been set.  Therefore, the
  * ondisk min and max defined here can be used directly to constrain the incore
  * quota expiration timestamps on a Unix system.
+ *
+ * The grace period for each quota type is stored in the root dquot (id = 0)
+ * and is applied to a non-root dquot when it exceeds the soft or hard limits.
+ * The length of quota grace periods are unsigned 32-bit quantities measured in
+ * units of seconds.  A value of zero means to use the default period.
  */
 
 /*
@@ -1223,6 +1228,14 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  */
 #define XFS_DQ_LEGACY_EXPIRY_MAX	((int64_t)U32_MAX)
 
+/*
+ * Default quota grace periods, ranging from zero (use the compiled defaults)
+ * to ~136 years.  These are applied to a non-root dquot that has exceeded
+ * either limit.
+ */
+#define XFS_DQ_GRACE_MIN		((int64_t)0)
+#define XFS_DQ_GRACE_MAX		((int64_t)U32_MAX)
+
 /*
  * This is the main portion of the on-disk representation of quota information
  * for a user.  We pad this with some more expansion room to construct the on
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index f34841f98d44..e63a933413a3 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -110,6 +110,14 @@ xfs_dquot_set_timeout(
 					  qi->qi_expiry_max);
 }
 
+/* Set the length of the default grace period. */
+time64_t
+xfs_dquot_set_grace_period(
+	time64_t		grace)
+{
+	return clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN, XFS_DQ_GRACE_MAX);
+}
+
 /*
  * Determine if this quota counter is over either limit and set the quota
  * timers as appropriate.
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 0e449101c861..f642884a6834 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -238,5 +238,6 @@ int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type,
 		xfs_qm_dqiterate_fn iter_fn, void *priv);
 
 time64_t xfs_dquot_set_timeout(struct xfs_mount *mp, time64_t timeout);
+time64_t xfs_dquot_set_grace_period(time64_t grace);
 
 #endif /* __XFS_DQUOT_H__ */
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 750f775ae915..ca1b57d291dc 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -486,8 +486,8 @@ xfs_setqlim_timer(
 {
 	if (qlim) {
 		/* Set the length of the default grace period. */
-		res->timer = timer;
-		qlim->time = timer;
+		res->timer = xfs_dquot_set_grace_period(timer);
+		qlim->time = res->timer;
 	} else {
 		/* Set the grace period expiration on a quota. */
 		res->timer = xfs_dquot_set_timeout(mp, timer);

Re: [PATCH 03/11] xfs: refactor default quota grace period setting code

[Amir Goldstein]

On Tue, Aug 18, 2020 at 1:59 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Refactor the code that sets the default quota grace period into a helper
> function so that we can override the ondisk behavior later.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

[PATCH 03/11] xfs: refactor default quota grace period setting code

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor the code that sets the default quota grace period into a helper
function so that we can override the ondisk behavior later.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_format.h |   13 +++++++++++++
 fs/xfs/xfs_dquot.c         |    9 +++++++++
 fs/xfs/xfs_dquot.h         |    1 +
 fs/xfs/xfs_ondisk.h        |    2 ++
 fs/xfs/xfs_qm_syscalls.c   |    4 ++--
 5 files changed, 27 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index ef36978239ac..e9e6248b35be 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1205,6 +1205,11 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  * time zero is the Unix epoch, Jan  1 00:00:01 UTC 1970.  An expiration value
  * of zero means that the quota limit has not been reached, and therefore no
  * expiration has been set.
+ *
+ * The grace period for each quota type is stored in the root dquot (id = 0)
+ * and is applied to a non-root dquot when it exceeds the soft or hard limits.
+ * The length of quota grace periods are unsigned 32-bit quantities measured in
+ * units of seconds.  A value of zero means to use the default period.
  */
 
 /*
@@ -1219,6 +1224,14 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  */
 #define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
 
+/*
+ * Default quota grace periods, ranging from zero (use the compiled defaults)
+ * to ~136 years.  These are applied to a non-root dquot that has exceeded
+ * either limit.
+ */
+#define XFS_DQ_GRACE_MIN	((int64_t)0)
+#define XFS_DQ_GRACE_MAX	((int64_t)U32_MAX)
+
 /*
  * This is the main portion of the on-disk representation of quota information
  * for a user.  We pad this with some more expansion room to construct the on
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 2425b1c30d11..ed3fa6ada0d3 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -98,6 +98,15 @@ xfs_qm_adjust_dqlimits(
 		xfs_dquot_set_prealloc_limits(dq);
 }
 
+/* Set the length of the default grace period. */
+void
+xfs_dquot_set_grace_period(
+	time64_t		*timer,
+	time64_t		value)
+{
+	*timer = clamp_t(time64_t, value, XFS_DQ_GRACE_MIN, XFS_DQ_GRACE_MAX);
+}
+
 /* Set the expiration time of a quota's grace period. */
 void
 xfs_dquot_set_timeout(
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 11bd0ee9b0fa..0ba4d91c3a11 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -237,6 +237,7 @@ typedef int (*xfs_qm_dqiterate_fn)(struct xfs_dquot *dq,
 int xfs_qm_dqiterate(struct xfs_mount *mp, xfs_dqtype_t type,
 		xfs_qm_dqiterate_fn iter_fn, void *priv);
 
+void xfs_dquot_set_grace_period(time64_t *timer, time64_t limit);
 void xfs_dquot_set_timeout(time64_t *timer, time64_t limit);
 
 #endif /* __XFS_DQUOT_H__ */
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 38ccffcf3336..498e9063c605 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -27,6 +27,8 @@ xfs_check_limits(void)
 	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
 	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MIN,			1LL);
 	XFS_CHECK_VALUE(XFS_DQ_TIMEOUT_MAX,			4294967295LL);
+	XFS_CHECK_VALUE(XFS_DQ_GRACE_MIN,			0LL);
+	XFS_CHECK_VALUE(XFS_DQ_GRACE_MAX,			4294967295LL);
 }
 
 static inline void __init
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index b16d533a6feb..95b0c25b9969 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -485,8 +485,8 @@ xfs_setqlim_timer(
 {
 	if (qlim) {
 		/* Set the length of the default grace period. */
-		res->timer = timer;
-		qlim->time = timer;
+		xfs_dquot_set_grace_period(&res->timer, timer);
+		qlim->time = res->timer;
 	} else {
 		/* Set the grace period expiration on a quota. */
 		xfs_dquot_set_timeout(&res->timer, timer);