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

[PATCH 00/14] 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 refactors the quota
grace period expiration timer code a bit before implementing bit
shifting to widen the effective counter size to 34 bits.  This enables
correct time handling on XFS through the year 2486.

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

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

[Darrick J. Wong]

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

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_format.h |   19 +++++++++++++++++++
 fs/xfs/xfs_ondisk.h        |    8 ++++++++
 fs/xfs/xfs_super.c         |    4 ++--
 3 files changed, 29 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 9ff373962d10..82b15832ba32 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -841,11 +841,30 @@ typedef 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 fa0ec2fae14a..f67f3645efcd 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -15,9 +15,17 @@
 		"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_ondisk_structs(void)
 {
+	/* make sure timestamp limits are correct */
+	XFS_CHECK_VALUE(XFS_INO_TIME_MIN, 			-2147483648LL);
+	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
+
 	/* ag/file structures */
 	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_acl_entry,		12);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index f687181a2720..3bddf13cd8ea 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1582,8 +1582,8 @@ xfs_fc_fill_super(
 	sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
 	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);

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

[Eric Sandeen]

On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_format.h |   19 +++++++++++++++++++
>  fs/xfs/xfs_ondisk.h        |    8 ++++++++
>  fs/xfs/xfs_super.c         |    4 ++--
>  3 files changed, 29 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 9ff373962d10..82b15832ba32 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -841,11 +841,30 @@ typedef 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 fa0ec2fae14a..f67f3645efcd 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -15,9 +15,17 @@
>  		"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_ondisk_structs(void)
>  {
> +	/* make sure timestamp limits are correct */
> +	XFS_CHECK_VALUE(XFS_INO_TIME_MIN, 			-2147483648LL);
> +	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);

IMHO this really shouldn't be in a function with this name, as it's not checking
an ondisk struct.  And I'm not really sure what it's protecting against?
Basically you put an integer in one #define and check it in another?

> +
>  	/* ag/file structures */
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl_entry,		12);
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index f687181a2720..3bddf13cd8ea 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1582,8 +1582,8 @@ xfs_fc_fill_super(
>  	sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
>  	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);
> 

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

[Darrick J. Wong]

On Wed, Feb 12, 2020 at 05:00:59PM -0600, Eric Sandeen wrote:
> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_format.h |   19 +++++++++++++++++++
> >  fs/xfs/xfs_ondisk.h        |    8 ++++++++
> >  fs/xfs/xfs_super.c         |    4 ++--
> >  3 files changed, 29 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > index 9ff373962d10..82b15832ba32 100644
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -841,11 +841,30 @@ typedef 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 fa0ec2fae14a..f67f3645efcd 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -15,9 +15,17 @@
> >  		"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_ondisk_structs(void)
> >  {
> > +	/* make sure timestamp limits are correct */
> > +	XFS_CHECK_VALUE(XFS_INO_TIME_MIN, 			-2147483648LL);
> > +	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> 
> IMHO this really shouldn't be in a function with this name, as it's not checking
> an ondisk struct.  And I'm not really sure what it's protecting against?
> Basically you put an integer in one #define and check it in another?

Admittedly /this/ part isn't so crucial, because S32_MAX is never going
to be redefined.  However, I added this for completeness; notice that
the patch that widens xfs_timestamp_t adds similar checks for the new
minimum and maximum timestamp, whose values are not so straightforward.

Also, I get that this isn't directly checking an ondisk structure, but
given that we use these constants, there ought to be a check against
incorrect computation *somewhere*.  The BUILD_BUG_ON macros don't
produce any real code (and this function is called at __init time) so
what's the harm?

--D

> > +
> >  	/* ag/file structures */
> >  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
> >  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl_entry,		12);
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index f687181a2720..3bddf13cd8ea 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -1582,8 +1582,8 @@ xfs_fc_fill_super(
> >  	sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
> >  	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);
> > 

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

[Eric Sandeen]

On 2/12/20 7:26 PM, Darrick J. Wong wrote:
> On Wed, Feb 12, 2020 at 05:00:59PM -0600, Eric Sandeen wrote:
>> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
...

>>>  static inline void __init
>>>  xfs_check_ondisk_structs(void)
>>>  {
>>> +	/* make sure timestamp limits are correct */
>>> +	XFS_CHECK_VALUE(XFS_INO_TIME_MIN, 			-2147483648LL);
>>> +	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
>>
>> IMHO this really shouldn't be in a function with this name, as it's not checking
>> an ondisk struct.  And I'm not really sure what it's protecting against?
>> Basically you put an integer in one #define and check it in another?
> 
> Admittedly /this/ part isn't so crucial, because S32_MAX is never going
> to be redefined.  However, I added this for completeness; notice that
> the patch that widens xfs_timestamp_t adds similar checks for the new
> minimum and maximum timestamp, whose values are not so straightforward.
> 
> Also, I get that this isn't directly checking an ondisk structure, but
> given that we use these constants, there ought to be a check against
> incorrect computation *somewhere*.  The BUILD_BUG_ON macros don't
> produce any real code (and this function is called at __init time) so
> what's the harm?

OCD?  Maybe just make a xfs_check_time_vals() to go with
xfs_check_ondisk_structs() or something.

-Eric

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

[Darrick J. Wong]

On Wed, Feb 12, 2020 at 07:50:02PM -0600, Eric Sandeen wrote:
> On 2/12/20 7:26 PM, Darrick J. Wong wrote:
> > On Wed, Feb 12, 2020 at 05:00:59PM -0600, Eric Sandeen wrote:
> >> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> >>> From: Darrick J. Wong <darrick.wong@oracle.com>
> >>>
> >>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ...
> 
> >>>  static inline void __init
> >>>  xfs_check_ondisk_structs(void)
> >>>  {
> >>> +	/* make sure timestamp limits are correct */
> >>> +	XFS_CHECK_VALUE(XFS_INO_TIME_MIN, 			-2147483648LL);
> >>> +	XFS_CHECK_VALUE(XFS_INO_TIME_MAX,			2147483647LL);
> >>
> >> IMHO this really shouldn't be in a function with this name, as it's not checking
> >> an ondisk struct.  And I'm not really sure what it's protecting against?
> >> Basically you put an integer in one #define and check it in another?
> > 
> > Admittedly /this/ part isn't so crucial, because S32_MAX is never going
> > to be redefined.  However, I added this for completeness; notice that
> > the patch that widens xfs_timestamp_t adds similar checks for the new
> > minimum and maximum timestamp, whose values are not so straightforward.
> > 
> > Also, I get that this isn't directly checking an ondisk structure, but
> > given that we use these constants, there ought to be a check against
> > incorrect computation *somewhere*.  The BUILD_BUG_ON macros don't
> > produce any real code (and this function is called at __init time) so
> > what's the harm?
> 
> OCD?  Maybe just make a xfs_check_time_vals() to go with
> xfs_check_ondisk_structs() or something.

Done.

--D

> -Eric

[PATCH 02/14] xfs: preserve default grace interval during quotacheck

[Darrick J. Wong]

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

When quotacheck runs, it zeroes all the timer fields in every dquot.
Unfortunately, it also does this to the root dquot, which erases any
preconfigured grace interval that the administrator may have set.  Worse
yet, the incore copies of those variables remain set.  This cache
coherence problem manifests itself as the grace interval mysteriously
being reset back to the defaults at the /next/ mount.

Fix it by resetting the root disk dquot's timer fields to the incore
values.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_qm.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)


diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 0ce334c51d73..d4a9765c9502 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -842,6 +842,23 @@ xfs_qm_qino_alloc(
 	return error;
 }
 
+/* Save the grace period intervals when zeroing dquots for quotacheck. */
+static inline void
+xfs_qm_reset_dqintervals(
+	struct xfs_mount	*mp,
+	struct xfs_disk_dquot	*ddq)
+{
+	struct xfs_quotainfo	*qinf = mp->m_quotainfo;
+
+	if (qinf->qi_btimelimit != XFS_QM_BTIMELIMIT)
+		ddq->d_btimer = cpu_to_be32(qinf->qi_btimelimit);
+
+	if (qinf->qi_itimelimit != XFS_QM_ITIMELIMIT)
+		ddq->d_itimer = cpu_to_be32(qinf->qi_itimelimit);
+
+	if (qinf->qi_rtbtimelimit != XFS_QM_RTBTIMELIMIT)
+		ddq->d_rtbtimer = cpu_to_be32(qinf->qi_rtbtimelimit);
+}
 
 STATIC void
 xfs_qm_reset_dqcounts(
@@ -895,6 +912,8 @@ xfs_qm_reset_dqcounts(
 		ddq->d_bwarns = 0;
 		ddq->d_iwarns = 0;
 		ddq->d_rtbwarns = 0;
+		if (!ddq->d_id)
+			xfs_qm_reset_dqintervals(mp, ddq);
 
 		if (xfs_sb_version_hascrc(&mp->m_sb)) {
 			xfs_update_cksum((char *)&dqb[j],

Re: [PATCH 02/14] xfs: preserve default grace interval during quotacheck

[Eric Sandeen]

On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When quotacheck runs, it zeroes all the timer fields in every dquot.
> Unfortunately, it also does this to the root dquot, which erases any
> preconfigured grace interval that the administrator may have set.  Worse
> yet, the incore copies of those variables remain set.  This cache
> coherence problem manifests itself as the grace interval mysteriously
> being reset back to the defaults at the /next/ mount.

woot that's kind of a theme in xfs quota code :/

Is it my turn to ask for a testcase?

so: "quotacheck" on xfs means "mount with quota accounting enabled" I think,
just for clarity...

> Fix it by resetting the root disk dquot's timer fields to the incore
> values.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_qm.c |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> 
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 0ce334c51d73..d4a9765c9502 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -842,6 +842,23 @@ xfs_qm_qino_alloc(
>  	return error;
>  }
>  
> +/* Save the grace period intervals when zeroing dquots for quotacheck. */
> +static inline void
> +xfs_qm_reset_dqintervals(
> +	struct xfs_mount	*mp,
> +	struct xfs_disk_dquot	*ddq)
> +{
> +	struct xfs_quotainfo	*qinf = mp->m_quotainfo;
> +
> +	if (qinf->qi_btimelimit != XFS_QM_BTIMELIMIT)
> +		ddq->d_btimer = cpu_to_be32(qinf->qi_btimelimit);
> +
> +	if (qinf->qi_itimelimit != XFS_QM_ITIMELIMIT)
> +		ddq->d_itimer = cpu_to_be32(qinf->qi_itimelimit);
> +
> +	if (qinf->qi_rtbtimelimit != XFS_QM_RTBTIMELIMIT)
> +		ddq->d_rtbtimer = cpu_to_be32(qinf->qi_rtbtimelimit);

Probably need to handle warning counters here too, but ...

> +}
>  
>  STATIC void
>  xfs_qm_reset_dqcounts(
> @@ -895,6 +912,8 @@ 	(
>  		ddq->d_bwarns = 0;
>  		ddq->d_iwarns = 0;
>  		ddq->d_rtbwarns = 0;

a comment about why !ddq->d_id (i.e. it's the default quota)
would probably be good here.

> +		if (!ddq->d_id)
> +			xfs_qm_reset_dqintervals(mp, ddq);

Isn't it a little weird to clear it for ID 0, then immediately reset it?

Let's see, quotacheck only happens when we do a fresh mount where quota accounting
was not on during the previous mount.

The point of quotacheck is to get all of the block counters in sync with actual
block usage.

The timers (and warnings) for normal users are zero until they exceed soft limits,
then reflect the time at which EDQUOT will appear.

<aside: does quotacheck set timers for users who are already over soft limits
at quotacheck time...?  Yes: see xfs_qm_quotacheck_dqadjust()>

The timers (and warnings) for ID 0 (root/default) are where we store the default
grace times & warning limits, there is no need for quotacheck to change them;
they serve a different purpose.

So quotacheck really should never be touching the default timers or warn limits
on ID 0.  I'd suggest simply skipping them for id 0, as it is treated specially
in several other places as well, i.e.

-               ddq->d_btimer = 0;
-               ddq->d_itimer = 0;
-               ddq->d_rtbtimer = 0;
-               ddq->d_bwarns = 0;
-               ddq->d_iwarns = 0;
-               ddq->d_rtbwarns = 0;
+               /* Don't reset default quota timers & counters in root dquot */
+               if (ddq->d_id) {
+                       ddq->d_btimer = 0;
+                       ddq->d_itimer = 0;
+                       ddq->d_rtbtimer = 0;
+                       ddq->d_bwarns = 0;
+                       ddq->d_iwarns = 0;
+                       ddq->d_rtbwarns = 0;
+               }

>  
>  		if (xfs_sb_version_hascrc(&mp->m_sb)) {
>  			xfs_update_cksum((char *)&dqb[j],
> 

Re: [PATCH 02/14] xfs: preserve default grace interval during quotacheck

[Eric Sandeen]

On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When quotacheck runs, it zeroes all the timer fields in every dquot.
> Unfortunately, it also does this to the root dquot, which erases any
> preconfigured grace interval that the administrator may have set.  Worse
> yet, the incore copies of those variables remain set.  This cache
> coherence problem manifests itself as the grace interval mysteriously
> being reset back to the defaults at the /next/ mount.
> 
> Fix it by resetting the root disk dquot's timer fields to the incore
> values.

Uh, so, even with this, it seems that we don't properly set up default time
limits on the first mount.  Looking into it...

I think we need something like this but I need to look more closely.

Otherwise, xfs_qm_dqget_uncached fails if run before quotacheck has initialized
things, and so we fail to set up default quotas or timers.

(also, not sure if there's any point to initializing timers if that quota
is not enabled...)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index f94f6c34ee35..f4ac69fd946e 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -663,17 +671,6 @@ xfs_qm_init_quotainfo(
 
 	mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
 
-	xfs_qm_init_timelimits(mp, XFS_DQ_USER);
-	xfs_qm_init_timelimits(mp, XFS_DQ_GROUP);
-	xfs_qm_init_timelimits(mp, XFS_DQ_PROJ);
-
-	if (XFS_IS_UQUOTA_RUNNING(mp))
-		xfs_qm_set_defquota(mp, XFS_DQ_USER);
-	if (XFS_IS_GQUOTA_RUNNING(mp))
-		xfs_qm_set_defquota(mp, XFS_DQ_GROUP);
-	if (XFS_IS_PQUOTA_RUNNING(mp))
-		xfs_qm_set_defquota(mp, XFS_DQ_PROJ);
-
 	qinf->qi_shrinker.count_objects = xfs_qm_shrink_count;
 	qinf->qi_shrinker.scan_objects = xfs_qm_shrink_scan;
 	qinf->qi_shrinker.seeks = DEFAULT_SEEKS;
@@ -1423,6 +1420,18 @@ xfs_qm_mount_quotas(
 			return;
 		}
 	}
+
+	xfs_qm_init_timelimits(mp, XFS_DQ_USER);
+	xfs_qm_init_timelimits(mp, XFS_DQ_GROUP);
+	xfs_qm_init_timelimits(mp, XFS_DQ_PROJ);
+
+	if (XFS_IS_UQUOTA_RUNNING(mp))
+		xfs_qm_set_defquota(mp, XFS_DQ_USER);
+	if (XFS_IS_GQUOTA_RUNNING(mp))
+		xfs_qm_set_defquota(mp, XFS_DQ_GROUP);
+	if (XFS_IS_PQUOTA_RUNNING(mp))
+		xfs_qm_set_defquota(mp, XFS_DQ_PROJ);
+
 	/*
 	 * If one type of quotas is off, then it will lose its
 	 * quotachecked status, since we won't be doing accounting for

Re: [PATCH 02/14] xfs: preserve default grace interval during quotacheck

[Eric Sandeen]

On 2/18/20 8:55 PM, Eric Sandeen wrote:
> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> When quotacheck runs, it zeroes all the timer fields in every dquot.
>> Unfortunately, it also does this to the root dquot, which erases any
>> preconfigured grace interval that the administrator may have set.  Worse
>> yet, the incore copies of those variables remain set.  This cache
>> coherence problem manifests itself as the grace interval mysteriously
>> being reset back to the defaults at the /next/ mount.
>>
>> Fix it by resetting the root disk dquot's timer fields to the incore
>> values.
> 
> Uh, so, even with this, it seems that we don't properly set up default time
> limits on the first mount.  Looking into it...

Sorry.  This was actually a regression from my timer-per-type series.  :(

so ignore this critique, it's my fault. ;)

-Eric

Re: [PATCH 02/14] xfs: preserve default grace interval during quotacheck

[Darrick J. Wong]

On Mon, Mar 02, 2020 at 09:03:37PM -0600, Eric Sandeen wrote:
> On 2/18/20 8:55 PM, Eric Sandeen wrote:
> > On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> >> From: Darrick J. Wong <darrick.wong@oracle.com>
> >>
> >> When quotacheck runs, it zeroes all the timer fields in every dquot.
> >> Unfortunately, it also does this to the root dquot, which erases any
> >> preconfigured grace interval that the administrator may have set.  Worse
> >> yet, the incore copies of those variables remain set.  This cache
> >> coherence problem manifests itself as the grace interval mysteriously
> >> being reset back to the defaults at the /next/ mount.
> >>
> >> Fix it by resetting the root disk dquot's timer fields to the incore
> >> values.
> > 
> > Uh, so, even with this, it seems that we don't properly set up default time
> > limits on the first mount.  Looking into it...
> 
> Sorry.  This was actually a regression from my timer-per-type series.  :(
> 
> so ignore this critique, it's my fault. ;)

But will you (or anyone really) please review this fix?

--D

> -Eric

Re: [PATCH 02/14] xfs: preserve default grace interval during quotacheck

[Eric Sandeen]

On 3/3/20 7:48 AM, Darrick J. Wong wrote:
> On Mon, Mar 02, 2020 at 09:03:37PM -0600, Eric Sandeen wrote:
>> On 2/18/20 8:55 PM, Eric Sandeen wrote:
>>> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
>>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>>
>>>> When quotacheck runs, it zeroes all the timer fields in every dquot.
>>>> Unfortunately, it also does this to the root dquot, which erases any
>>>> preconfigured grace interval that the administrator may have set.  Worse
>>>> yet, the incore copies of those variables remain set.  This cache
>>>> coherence problem manifests itself as the grace interval mysteriously
>>>> being reset back to the defaults at the /next/ mount.
>>>>
>>>> Fix it by resetting the root disk dquot's timer fields to the incore
>>>> values.
>>>
>>> Uh, so, even with this, it seems that we don't properly set up default time
>>> limits on the first mount.  Looking into it...
>>
>> Sorry.  This was actually a regression from my timer-per-type series.  :(
>>
>> so ignore this critique, it's my fault. ;)
> 
> But will you (or anyone really) please review this fix?

I did; I have other comments on it up-thread.

-Eric

[PATCH 03/14] xfs: refactor quota exceeded test

[Darrick J. Wong]

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

Refactor the open-coded test for whether or not we're over quota.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c |   61 +++++++++++++++++++++-------------------------------
 1 file changed, 25 insertions(+), 36 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index e50c75d9d788..54e7fdcd1d4d 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -99,6 +99,17 @@ xfs_qm_adjust_dqlimits(
 		xfs_dquot_set_prealloc_limits(dq);
 }
 
+static inline bool
+xfs_quota_exceeded(
+	const __be64		*count,
+	const __be64		*softlimit,
+	const __be64		*hardlimit) {
+
+	if (*softlimit && be64_to_cpup(count) > be64_to_cpup(softlimit))
+		return true;
+	return *hardlimit && be64_to_cpup(count) > be64_to_cpup(hardlimit);
+}
+
 /*
  * Check the limits and timers of a dquot and start or reset timers
  * if necessary.
@@ -117,6 +128,8 @@ xfs_qm_adjust_dqtimers(
 	struct xfs_mount	*mp,
 	struct xfs_disk_dquot	*d)
 {
+	bool			over;
+
 	ASSERT(d->d_id);
 
 #ifdef DEBUG
@@ -131,71 +144,47 @@ xfs_qm_adjust_dqtimers(
 		       be64_to_cpu(d->d_rtb_hardlimit));
 #endif
 
+	over = xfs_quota_exceeded(&d->d_bcount, &d->d_blk_softlimit,
+			&d->d_blk_hardlimit);
 	if (!d->d_btimer) {
-		if ((d->d_blk_softlimit &&
-		     (be64_to_cpu(d->d_bcount) >
-		      be64_to_cpu(d->d_blk_softlimit))) ||
-		    (d->d_blk_hardlimit &&
-		     (be64_to_cpu(d->d_bcount) >
-		      be64_to_cpu(d->d_blk_hardlimit)))) {
+		if (over) {
 			d->d_btimer = cpu_to_be32(get_seconds() +
 					mp->m_quotainfo->qi_btimelimit);
 		} else {
 			d->d_bwarns = 0;
 		}
 	} else {
-		if ((!d->d_blk_softlimit ||
-		     (be64_to_cpu(d->d_bcount) <=
-		      be64_to_cpu(d->d_blk_softlimit))) &&
-		    (!d->d_blk_hardlimit ||
-		    (be64_to_cpu(d->d_bcount) <=
-		     be64_to_cpu(d->d_blk_hardlimit)))) {
+		if (!over) {
 			d->d_btimer = 0;
 		}
 	}
 
+	over = xfs_quota_exceeded(&d->d_icount, &d->d_ino_softlimit,
+			&d->d_ino_hardlimit);
 	if (!d->d_itimer) {
-		if ((d->d_ino_softlimit &&
-		     (be64_to_cpu(d->d_icount) >
-		      be64_to_cpu(d->d_ino_softlimit))) ||
-		    (d->d_ino_hardlimit &&
-		     (be64_to_cpu(d->d_icount) >
-		      be64_to_cpu(d->d_ino_hardlimit)))) {
+		if (over) {
 			d->d_itimer = cpu_to_be32(get_seconds() +
 					mp->m_quotainfo->qi_itimelimit);
 		} else {
 			d->d_iwarns = 0;
 		}
 	} else {
-		if ((!d->d_ino_softlimit ||
-		     (be64_to_cpu(d->d_icount) <=
-		      be64_to_cpu(d->d_ino_softlimit)))  &&
-		    (!d->d_ino_hardlimit ||
-		     (be64_to_cpu(d->d_icount) <=
-		      be64_to_cpu(d->d_ino_hardlimit)))) {
+		if (!over) {
 			d->d_itimer = 0;
 		}
 	}
 
+	over = xfs_quota_exceeded(&d->d_rtbcount, &d->d_rtb_softlimit,
+			&d->d_rtb_hardlimit);
 	if (!d->d_rtbtimer) {
-		if ((d->d_rtb_softlimit &&
-		     (be64_to_cpu(d->d_rtbcount) >
-		      be64_to_cpu(d->d_rtb_softlimit))) ||
-		    (d->d_rtb_hardlimit &&
-		     (be64_to_cpu(d->d_rtbcount) >
-		      be64_to_cpu(d->d_rtb_hardlimit)))) {
+		if (over) {
 			d->d_rtbtimer = cpu_to_be32(get_seconds() +
 					mp->m_quotainfo->qi_rtbtimelimit);
 		} else {
 			d->d_rtbwarns = 0;
 		}
 	} else {
-		if ((!d->d_rtb_softlimit ||
-		     (be64_to_cpu(d->d_rtbcount) <=
-		      be64_to_cpu(d->d_rtb_softlimit))) &&
-		    (!d->d_rtb_hardlimit ||
-		     (be64_to_cpu(d->d_rtbcount) <=
-		      be64_to_cpu(d->d_rtb_hardlimit)))) {
+		if (!over) {
 			d->d_rtbtimer = 0;
 		}
 	}

Re: [PATCH 03/14] xfs: refactor quota exceeded test

[Eric Sandeen]

On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor the open-coded test for whether or not we're over quota.

Ooh, nice.  This was horrible.

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_dquot.c |   61 +++++++++++++++++++++-------------------------------
>  1 file changed, 25 insertions(+), 36 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index e50c75d9d788..54e7fdcd1d4d 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -99,6 +99,17 @@ xfs_qm_adjust_dqlimits(
>  		xfs_dquot_set_prealloc_limits(dq);
>  }
>  
> +static inline bool
> +xfs_quota_exceeded(
> +	const __be64		*count,
> +	const __be64		*softlimit,
> +	const __be64		*hardlimit) {

why pass these all as pointers?

> +
> +	if (*softlimit && be64_to_cpup(count) > be64_to_cpup(softlimit))
> +		return true;
> +	return *hardlimit && be64_to_cpup(count) > be64_to_cpup(hardlimit);

The asymmetry bothers me a little but maybe that's just me.  Is

> +	if ((*softlimit && be64_to_cpup(count) > be64_to_cpup(softlimit)) ||
> +	    (*hardlimit && be64_to_cpup(count) > be64_to_cpup(hardlimit)))
> +		return true;
> +	return false;

any better? *shrug*

> +}
> +
>  /*
>   * Check the limits and timers of a dquot and start or reset timers
>   * if necessary.
> @@ -117,6 +128,8 @@ xfs_qm_adjust_dqtimers(
>  	struct xfs_mount	*mp,
>  	struct xfs_disk_dquot	*d)
>  {
> +	bool			over;
> +
>  	ASSERT(d->d_id);
>  
>  #ifdef DEBUG
> @@ -131,71 +144,47 @@ xfs_qm_adjust_dqtimers(
>  		       be64_to_cpu(d->d_rtb_hardlimit));
>  #endif
>  
> +	over = xfs_quota_exceeded(&d->d_bcount, &d->d_blk_softlimit,
> +			&d->d_blk_hardlimit);
>  	if (!d->d_btimer) {
> -		if ((d->d_blk_softlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_softlimit))) ||
> -		    (d->d_blk_hardlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_hardlimit)))) {
> +		if (over) {

I wonder why we check the hard limit.  Isn't exceeding the soft limit
enough to start the timer?  Unrelated to the refactoring tho.

>  			d->d_btimer = cpu_to_be32(get_seconds() +
>  					mp->m_quotainfo->qi_btimelimit);
>  		} else {
>  			d->d_bwarns = 0;
>  		}
>  	} else {
> -		if ((!d->d_blk_softlimit || (be64_to_cpu(d->d_bcount) <= be64_to_cpu(d->d_blk_softlimit))) &&
> -		    (!d->d_blk_hardlimit || (be64_to_cpu(d->d_bcount) <= be64_to_cpu(d->d_blk_hardlimit)))) {
> +		if (!over) {
>  			d->d_btimer = 0;
>  		}

I guess that could be

>  	} else if (!over) {
>  		d->d_btimer = 0;
>  	}

? but again *shrug* and that's beyond refactoring, isn't it.

Re: [PATCH 03/14] xfs: refactor quota exceeded test

[Darrick J. Wong]

On Wed, Feb 12, 2020 at 05:51:18PM -0600, Eric Sandeen wrote:
> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Refactor the open-coded test for whether or not we're over quota.
> 
> Ooh, nice.  This was horrible.
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_dquot.c |   61 +++++++++++++++++++++-------------------------------
> >  1 file changed, 25 insertions(+), 36 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index e50c75d9d788..54e7fdcd1d4d 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -99,6 +99,17 @@ xfs_qm_adjust_dqlimits(
> >  		xfs_dquot_set_prealloc_limits(dq);
> >  }
> >  
> > +static inline bool
> > +xfs_quota_exceeded(
> > +	const __be64		*count,
> > +	const __be64		*softlimit,
> > +	const __be64		*hardlimit) {
> 
> why pass these all as pointers?

I don't remember.  I think a previous iteration of bigtime had something
to do with messing with the dquot directly?

> > +
> > +	if (*softlimit && be64_to_cpup(count) > be64_to_cpup(softlimit))
> > +		return true;
> > +	return *hardlimit && be64_to_cpup(count) > be64_to_cpup(hardlimit);
> 
> The asymmetry bothers me a little but maybe that's just me.  Is
> 
> > +	if ((*softlimit && be64_to_cpup(count) > be64_to_cpup(softlimit)) ||
> > +	    (*hardlimit && be64_to_cpup(count) > be64_to_cpup(hardlimit)))
> > +		return true;
> > +	return false;
> 
> any better? *shrug*

Yeah, I could fix that function.

> > +}
> > +
> >  /*
> >   * Check the limits and timers of a dquot and start or reset timers
> >   * if necessary.
> > @@ -117,6 +128,8 @@ xfs_qm_adjust_dqtimers(
> >  	struct xfs_mount	*mp,
> >  	struct xfs_disk_dquot	*d)
> >  {
> > +	bool			over;
> > +
> >  	ASSERT(d->d_id);
> >  
> >  #ifdef DEBUG
> > @@ -131,71 +144,47 @@ xfs_qm_adjust_dqtimers(
> >  		       be64_to_cpu(d->d_rtb_hardlimit));
> >  #endif
> >  
> > +	over = xfs_quota_exceeded(&d->d_bcount, &d->d_blk_softlimit,
> > +			&d->d_blk_hardlimit);
> >  	if (!d->d_btimer) {
> > -		if ((d->d_blk_softlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_softlimit))) ||
> > -		    (d->d_blk_hardlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_hardlimit)))) {
> > +		if (over) {
> 
> I wonder why we check the hard limit.  Isn't exceeding the soft limit
> enough to start the timer?  Unrelated to the refactoring tho.

Suppose there's only a hard limit set?

> >  			d->d_btimer = cpu_to_be32(get_seconds() +
> >  					mp->m_quotainfo->qi_btimelimit);
> >  		} else {
> >  			d->d_bwarns = 0;
> >  		}
> >  	} else {
> > -		if ((!d->d_blk_softlimit || (be64_to_cpu(d->d_bcount) <= be64_to_cpu(d->d_blk_softlimit))) &&
> > -		    (!d->d_blk_hardlimit || (be64_to_cpu(d->d_bcount) <= be64_to_cpu(d->d_blk_hardlimit)))) {
> > +		if (!over) {
> >  			d->d_btimer = 0;
> >  		}
> 
> I guess that could be
> 
> >  	} else if (!over) {
> >  		d->d_btimer = 0;
> >  	}
> 
> ? but again *shrug* and that's beyond refactoring, isn't it.

Strictly speaking, yes, but I think they're logically equivalent.

--D

Re: [PATCH 03/14] xfs: refactor quota exceeded test

[Eric Sandeen]

On 2/12/20 7:41 PM, Darrick J. Wong wrote:
> On Wed, Feb 12, 2020 at 05:51:18PM -0600, Eric Sandeen wrote:
>> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> Refactor the open-coded test for whether or not we're over quota.
>>
>> Ooh, nice.  This was horrible.

...

>>> +}
>>> +
>>>  /*
>>>   * Check the limits and timers of a dquot and start or reset timers
>>>   * if necessary.
>>> @@ -117,6 +128,8 @@ xfs_qm_adjust_dqtimers(
>>>  	struct xfs_mount	*mp,
>>>  	struct xfs_disk_dquot	*d)
>>>  {
>>> +	bool			over;
>>> +
>>>  	ASSERT(d->d_id);
>>>  
>>>  #ifdef DEBUG
>>> @@ -131,71 +144,47 @@ xfs_qm_adjust_dqtimers(
>>>  		       be64_to_cpu(d->d_rtb_hardlimit));
>>>  #endif
>>>  
>>> +	over = xfs_quota_exceeded(&d->d_bcount, &d->d_blk_softlimit,
>>> +			&d->d_blk_hardlimit);
>>>  	if (!d->d_btimer) {
>>> -		if ((d->d_blk_softlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_softlimit))) ||
>>> -		    (d->d_blk_hardlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_hardlimit)))) {
>>> +		if (over) {
>>
>> I wonder why we check the hard limit.  Isn't exceeding the soft limit
>> enough to start the timer?  Unrelated to the refactoring tho.
> 
> Suppose there's only a hard limit set?

then you get EDQUOT straightaway and timers don't matter?

I guess if you set a soft limit after you go over a hard-limit-only and ...
meh, ain't broke don't fix?

-Eric

Re: [PATCH 03/14] xfs: refactor quota exceeded test

[Darrick J. Wong]

On Wed, Feb 12, 2020 at 07:52:30PM -0600, Eric Sandeen wrote:
> On 2/12/20 7:41 PM, Darrick J. Wong wrote:
> > On Wed, Feb 12, 2020 at 05:51:18PM -0600, Eric Sandeen wrote:
> >> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> >>> From: Darrick J. Wong <darrick.wong@oracle.com>
> >>>
> >>> Refactor the open-coded test for whether or not we're over quota.
> >>
> >> Ooh, nice.  This was horrible.
> 
> ...
> 
> >>> +}
> >>> +
> >>>  /*
> >>>   * Check the limits and timers of a dquot and start or reset timers
> >>>   * if necessary.
> >>> @@ -117,6 +128,8 @@ xfs_qm_adjust_dqtimers(
> >>>  	struct xfs_mount	*mp,
> >>>  	struct xfs_disk_dquot	*d)
> >>>  {
> >>> +	bool			over;
> >>> +
> >>>  	ASSERT(d->d_id);
> >>>  
> >>>  #ifdef DEBUG
> >>> @@ -131,71 +144,47 @@ xfs_qm_adjust_dqtimers(
> >>>  		       be64_to_cpu(d->d_rtb_hardlimit));
> >>>  #endif
> >>>  
> >>> +	over = xfs_quota_exceeded(&d->d_bcount, &d->d_blk_softlimit,
> >>> +			&d->d_blk_hardlimit);
> >>>  	if (!d->d_btimer) {
> >>> -		if ((d->d_blk_softlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_softlimit))) ||
> >>> -		    (d->d_blk_hardlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_hardlimit)))) {
> >>> +		if (over) {
> >>
> >> I wonder why we check the hard limit.  Isn't exceeding the soft limit
> >> enough to start the timer?  Unrelated to the refactoring tho.
> > 
> > Suppose there's only a hard limit set?
> 
> then you get EDQUOT straightaway and timers don't matter?

Hm.  Maybe the idea here was that you always start the timer even if you
just hard-failed the operation?  So that we don't have to deal with
weird cases where timers don't always get started?

> I guess if you set a soft limit after you go over a hard-limit-only and ...
> meh, ain't broke don't fix?

"Behavior changes should be not be in refactoring patches"? :)

> -Eric
> 

Re: [PATCH 03/14] xfs: refactor quota exceeded test

[Amir Goldstein]

> > >     } else {
> > > -           if ((!d->d_blk_softlimit || (be64_to_cpu(d->d_bcount) <= be64_to_cpu(d->d_blk_softlimit))) &&
> > > -               (!d->d_blk_hardlimit || (be64_to_cpu(d->d_bcount) <= be64_to_cpu(d->d_blk_hardlimit)))) {
> > > +           if (!over) {
> > >                     d->d_btimer = 0;
> > >             }
> >
> > I guess that could be
> >
> > >     } else if (!over) {
> > >             d->d_btimer = 0;
> > >     }
> >
> > ? but again *shrug* and that's beyond refactoring, isn't it.
>
> Strictly speaking, yes, but I think they're logically equivalent.
>

Of course they are.. chiming in to agree with Eric that else if
looks better after the nice cleanup.
But I won't stand in your way to keep the else { if {

Thanks,
Amir.

[PATCH 04/14] xfs: fix quota timer inactivation

[Darrick J. Wong]

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

We need to take the inactivated inodes' resource usage into account when
we decide if we're actually over quota.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_dquot.c       |   28 +++++++++++++++++-----------
 fs/xfs/xfs_dquot.h       |    2 +-
 fs/xfs/xfs_qm.c          |    2 +-
 fs/xfs/xfs_qm_syscalls.c |    2 +-
 fs/xfs/xfs_trans_dquot.c |    2 +-
 5 files changed, 21 insertions(+), 15 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 54e7fdcd1d4d..ae7bb6361a99 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -101,13 +101,16 @@ xfs_qm_adjust_dqlimits(
 
 static inline bool
 xfs_quota_exceeded(
-	const __be64		*count,
+	const __be64		*dcount,
+	xfs_qcnt_t		ina_count,
 	const __be64		*softlimit,
-	const __be64		*hardlimit) {
+	const __be64		*hardlimit)
+{
+	uint64_t		count = be64_to_cpup(dcount) - ina_count;
 
-	if (*softlimit && be64_to_cpup(count) > be64_to_cpup(softlimit))
+	if (*softlimit && count > be64_to_cpup(softlimit))
 		return true;
-	return *hardlimit && be64_to_cpup(count) > be64_to_cpup(hardlimit);
+	return *hardlimit && count > be64_to_cpup(hardlimit);
 }
 
 /*
@@ -126,8 +129,9 @@ xfs_quota_exceeded(
 void
 xfs_qm_adjust_dqtimers(
 	struct xfs_mount	*mp,
-	struct xfs_disk_dquot	*d)
+	struct xfs_dquot	*dqp)
 {
+	struct xfs_disk_dquot	*d = &dqp->q_core;
 	bool			over;
 
 	ASSERT(d->d_id);
@@ -144,8 +148,8 @@ xfs_qm_adjust_dqtimers(
 		       be64_to_cpu(d->d_rtb_hardlimit));
 #endif
 
-	over = xfs_quota_exceeded(&d->d_bcount, &d->d_blk_softlimit,
-			&d->d_blk_hardlimit);
+	over = xfs_quota_exceeded(&d->d_bcount, dqp->q_ina_bcount,
+			&d->d_blk_softlimit, &d->d_blk_hardlimit);
 	if (!d->d_btimer) {
 		if (over) {
 			d->d_btimer = cpu_to_be32(get_seconds() +
@@ -159,8 +163,8 @@ xfs_qm_adjust_dqtimers(
 		}
 	}
 
-	over = xfs_quota_exceeded(&d->d_icount, &d->d_ino_softlimit,
-			&d->d_ino_hardlimit);
+	over = xfs_quota_exceeded(&d->d_icount, dqp->q_ina_icount,
+			&d->d_ino_softlimit, &d->d_ino_hardlimit);
 	if (!d->d_itimer) {
 		if (over) {
 			d->d_itimer = cpu_to_be32(get_seconds() +
@@ -174,8 +178,8 @@ xfs_qm_adjust_dqtimers(
 		}
 	}
 
-	over = xfs_quota_exceeded(&d->d_rtbcount, &d->d_rtb_softlimit,
-			&d->d_rtb_hardlimit);
+	over = xfs_quota_exceeded(&d->d_rtbcount, dqp->q_ina_rtbcount,
+			&d->d_rtb_softlimit, &d->d_rtb_hardlimit);
 	if (!d->d_rtbtimer) {
 		if (over) {
 			d->d_rtbtimer = cpu_to_be32(get_seconds() +
@@ -1279,6 +1283,8 @@ xfs_dquot_adjust(
 	dqp->q_ina_icount += inodes;
 	dqp->q_ina_bcount += dblocks;
 	dqp->q_ina_rtbcount += rblocks;
+	if (dqp->q_core.d_id)
+		xfs_qm_adjust_dqtimers(dqp->q_mount, dqp);
 	xfs_dqunlock(dqp);
 }
 
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 0d58f4ae8349..d924da98f66a 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -164,7 +164,7 @@ void		xfs_qm_dqdestroy(struct xfs_dquot *dqp);
 int		xfs_qm_dqflush(struct xfs_dquot *dqp, struct xfs_buf **bpp);
 void		xfs_qm_dqunpin_wait(struct xfs_dquot *dqp);
 void		xfs_qm_adjust_dqtimers(struct xfs_mount *mp,
-						struct xfs_disk_dquot *d);
+				       struct xfs_dquot *dqp);
 void		xfs_qm_adjust_dqlimits(struct xfs_mount *mp,
 						struct xfs_dquot *d);
 xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip, uint type);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index d4a9765c9502..268e028c9ec8 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1141,7 +1141,7 @@ xfs_qm_quotacheck_dqadjust(
 	 */
 	if (dqp->q_core.d_id) {
 		xfs_qm_adjust_dqlimits(mp, dqp);
-		xfs_qm_adjust_dqtimers(mp, &dqp->q_core);
+		xfs_qm_adjust_dqtimers(mp, dqp);
 	}
 
 	dqp->dq_flags |= XFS_DQ_DIRTY;
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 43ba4e6b5e22..74220948a360 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -593,7 +593,7 @@ xfs_qm_scall_setqlim(
 		 * is on or off. We don't really want to bother with iterating
 		 * over all ondisk dquots and turning the timers on/off.
 		 */
-		xfs_qm_adjust_dqtimers(mp, ddq);
+		xfs_qm_adjust_dqtimers(mp, dqp);
 	}
 	dqp->dq_flags |= XFS_DQ_DIRTY;
 	xfs_trans_log_dquot(tp, dqp);
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index a6fe2d8dc40f..248cfc369efc 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -388,7 +388,7 @@ xfs_trans_apply_dquot_deltas(
 			 */
 			if (d->d_id) {
 				xfs_qm_adjust_dqlimits(tp->t_mountp, dqp);
-				xfs_qm_adjust_dqtimers(tp->t_mountp, d);
+				xfs_qm_adjust_dqtimers(tp->t_mountp, dqp);
 			}
 
 			dqp->dq_flags |= XFS_DQ_DIRTY;

Re: [PATCH 04/14] xfs: fix quota timer inactivation

[Amir Goldstein]

On Wed, Jan 1, 2020 at 3:14 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> We need to take the inactivated inodes' resource usage into account when
> we decide if we're actually over quota.
>

Does this patch belong in this series?
Anyway, fine by me.

Thanks,
Amir.

Re: [PATCH 04/14] xfs: fix quota timer inactivation

[Darrick J. Wong]

On Sun, May 31, 2020 at 06:04:50PM +0300, Amir Goldstein wrote:
> On Wed, Jan 1, 2020 at 3:14 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > We need to take the inactivated inodes' resource usage into account when
> > we decide if we're actually over quota.
> >
> 
> Does this patch belong in this series?
> Anyway, fine by me.

Oops, heh, this belongs in the deferred inactivation series.

--D

> Thanks,
> Amir.

[PATCH 05/14] 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.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_format.h |   22 ++++++++++++++++++++++
 fs/xfs/xfs_dquot.c         |   42 ++++++++++++++++++++++++++++++++++++------
 fs/xfs/xfs_ondisk.h        |    2 ++
 3 files changed, 60 insertions(+), 6 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 82b15832ba32..95761b38fe86 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1180,6 +1180,28 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
 #define XFS_DQUOT_MAGIC		0x4451		/* 'DQ' */
 #define XFS_DQUOT_VERSION	(uint8_t)0x01	/* latest version number */
 
+/*
+ * 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. This is the q_core of the struct xfs_dquot that
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index ae7bb6361a99..44bae5f16b55 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -113,6 +113,36 @@ xfs_quota_exceeded(
 	return *hardlimit && count > be64_to_cpup(hardlimit);
 }
 
+/*
+ * Clamp a quota grace period expiration timer to the range that we support.
+ */
+static inline time64_t
+xfs_dquot_clamp_timer(
+	time64_t			timer)
+{
+	return clamp_t(time64_t, timer, XFS_DQ_TIMEOUT_MIN, XFS_DQ_TIMEOUT_MAX);
+}
+
+/* Set a quota grace period expiration timer. */
+static inline void
+xfs_quota_set_timer(
+	__be32			*dtimer,
+	time_t			limit)
+{
+	time64_t		new_timeout;
+
+	new_timeout = xfs_dquot_clamp_timer(get_seconds() + limit);
+	*dtimer = cpu_to_be32(new_timeout);
+}
+
+/* Clear a quota grace period expiration timer. */
+static inline void
+xfs_quota_clear_timer(
+	__be32			*dtimer)
+{
+	*dtimer = cpu_to_be32(0);
+}
+
 /*
  * Check the limits and timers of a dquot and start or reset timers
  * if necessary.
@@ -152,14 +182,14 @@ xfs_qm_adjust_dqtimers(
 			&d->d_blk_softlimit, &d->d_blk_hardlimit);
 	if (!d->d_btimer) {
 		if (over) {
-			d->d_btimer = cpu_to_be32(get_seconds() +
+			xfs_quota_set_timer(&d->d_btimer,
 					mp->m_quotainfo->qi_btimelimit);
 		} else {
 			d->d_bwarns = 0;
 		}
 	} else {
 		if (!over) {
-			d->d_btimer = 0;
+			xfs_quota_clear_timer(&d->d_btimer);
 		}
 	}
 
@@ -167,14 +197,14 @@ xfs_qm_adjust_dqtimers(
 			&d->d_ino_softlimit, &d->d_ino_hardlimit);
 	if (!d->d_itimer) {
 		if (over) {
-			d->d_itimer = cpu_to_be32(get_seconds() +
+			xfs_quota_set_timer(&d->d_itimer,
 					mp->m_quotainfo->qi_itimelimit);
 		} else {
 			d->d_iwarns = 0;
 		}
 	} else {
 		if (!over) {
-			d->d_itimer = 0;
+			xfs_quota_clear_timer(&d->d_itimer);
 		}
 	}
 
@@ -182,14 +212,14 @@ xfs_qm_adjust_dqtimers(
 			&d->d_rtb_softlimit, &d->d_rtb_hardlimit);
 	if (!d->d_rtbtimer) {
 		if (over) {
-			d->d_rtbtimer = cpu_to_be32(get_seconds() +
+			xfs_quota_set_timer(&d->d_rtbtimer,
 					mp->m_quotainfo->qi_rtbtimelimit);
 		} else {
 			d->d_rtbwarns = 0;
 		}
 	} else {
 		if (!over) {
-			d->d_rtbtimer = 0;
+			xfs_quota_clear_timer(&d->d_rtbtimer);
 		}
 	}
 }
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index f67f3645efcd..52dc5326b7bf 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -25,6 +25,8 @@ xfs_check_ondisk_structs(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);
 
 	/* ag/file structures */
 	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);

Re: [PATCH 05/14] xfs: refactor quota expiration timer modification

[Eric Sandeen]

On 12/31/19 7:11 PM, 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.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

...

> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index ae7bb6361a99..44bae5f16b55 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -113,6 +113,36 @@ xfs_quota_exceeded(
>  	return *hardlimit && count > be64_to_cpup(hardlimit);
>  }
>  
> +/*
> + * Clamp a quota grace period expiration timer to the range that we support.
> + */
> +static inline time64_t
> +xfs_dquot_clamp_timer(
> +	time64_t			timer)
> +{
> +	return clamp_t(time64_t, timer, XFS_DQ_TIMEOUT_MIN, XFS_DQ_TIMEOUT_MAX);
> +}
> +
> +/* Set a quota grace period expiration timer. */
> +static inline void
> +xfs_quota_set_timer(
> +	__be32			*dtimer,
> +	time_t			limit)
> +{
> +	time64_t		new_timeout;
> +
> +	new_timeout = xfs_dquot_clamp_timer(get_seconds() + limit);
> +	*dtimer = cpu_to_be32(new_timeout);
> +}
> +
> +/* Clear a quota grace period expiration timer. */
> +static inline void
> +xfs_quota_clear_timer(
> +	__be32			*dtimer)
> +{
> +	*dtimer = cpu_to_be32(0);

do we need to endian convert 0 to make sparse happy?  I don't see us doing
that anywhere else.  TBH not really sure I see the reason for the function
at all unless you really, really like the symmetry.

> +}
> +
>  /*
>   * Check the limits and timers of a dquot and start or reset timers
>   * if necessary.
> @@ -152,14 +182,14 @@ xfs_qm_adjust_dqtimers(
>  			&d->d_blk_softlimit, &d->d_blk_hardlimit);
>  	if (!d->d_btimer) {
>  		if (over) {
> -			d->d_btimer = cpu_to_be32(get_seconds() +
> +			xfs_quota_set_timer(&d->d_btimer,
>  					mp->m_quotainfo->qi_btimelimit);
>  		} else {
>  			d->d_bwarns = 0;
>  		}
>  	} else {
>  		if (!over) {
> -			d->d_btimer = 0;
> +			xfs_quota_clear_timer(&d->d_btimer);

yeah that's a very fancy way to say "= 0" ;)

...

> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index f67f3645efcd..52dc5326b7bf 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -25,6 +25,8 @@ xfs_check_ondisk_structs(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);

again grumble grumble really not checking an ondisk structure.

>  	/* ag/file structures */
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
> 

Re: [PATCH 05/14] xfs: refactor quota expiration timer modification

[Darrick J. Wong]

On Wed, Feb 12, 2020 at 05:57:24PM -0600, Eric Sandeen wrote:
> On 12/31/19 7:11 PM, 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.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> ...
> 
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index ae7bb6361a99..44bae5f16b55 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -113,6 +113,36 @@ xfs_quota_exceeded(
> >  	return *hardlimit && count > be64_to_cpup(hardlimit);
> >  }
> >  
> > +/*
> > + * Clamp a quota grace period expiration timer to the range that we support.
> > + */
> > +static inline time64_t
> > +xfs_dquot_clamp_timer(
> > +	time64_t			timer)
> > +{
> > +	return clamp_t(time64_t, timer, XFS_DQ_TIMEOUT_MIN, XFS_DQ_TIMEOUT_MAX);
> > +}
> > +
> > +/* Set a quota grace period expiration timer. */
> > +static inline void
> > +xfs_quota_set_timer(
> > +	__be32			*dtimer,
> > +	time_t			limit)
> > +{
> > +	time64_t		new_timeout;
> > +
> > +	new_timeout = xfs_dquot_clamp_timer(get_seconds() + limit);
> > +	*dtimer = cpu_to_be32(new_timeout);
> > +}
> > +
> > +/* Clear a quota grace period expiration timer. */
> > +static inline void
> > +xfs_quota_clear_timer(
> > +	__be32			*dtimer)
> > +{
> > +	*dtimer = cpu_to_be32(0);
> 
> do we need to endian convert 0 to make sparse happy?  I don't see us doing
> that anywhere else.  TBH not really sure I see the reason for the function
> at all unless you really, really like the symmetry.
> 
> > +}
> > +
> >  /*
> >   * Check the limits and timers of a dquot and start or reset timers
> >   * if necessary.
> > @@ -152,14 +182,14 @@ xfs_qm_adjust_dqtimers(
> >  			&d->d_blk_softlimit, &d->d_blk_hardlimit);
> >  	if (!d->d_btimer) {
> >  		if (over) {
> > -			d->d_btimer = cpu_to_be32(get_seconds() +
> > +			xfs_quota_set_timer(&d->d_btimer,
> >  					mp->m_quotainfo->qi_btimelimit);
> >  		} else {
> >  			d->d_bwarns = 0;
> >  		}
> >  	} else {
> >  		if (!over) {
> > -			d->d_btimer = 0;
> > +			xfs_quota_clear_timer(&d->d_btimer);
> 
> yeah that's a very fancy way to say "= 0" ;)
> 
> ...

Yes, that's a fancy way to assign zero.  However, consider that for
bigtime support, I had to add an incore quota timer so that timers more
or less fire when they're supposed to, and now there's a function to
convert the incore timespec64 value to whatever is ondisk:

/* Clear a quota grace period expiration timer. */
static inline void
xfs_quota_clear_timer(
	struct xfs_disk_dquot	*ddq,
	time64_t		*itimer,
	__be32			*dtimer)
{
	struct timespec64	tv = { 0 };

	*itimer = tv.tv_sec;
	xfs_dquot_to_disk_timestamp(ddq, dtimer, &tv);
}

It was at *that* point in the patchset that it seemed easier to call a
small function three times than to open-code this three times.

> > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > index f67f3645efcd..52dc5326b7bf 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -25,6 +25,8 @@ xfs_check_ondisk_structs(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);
> 
> again grumble grumble really not checking an ondisk structure.

Same answer as before. :)

--D

> >  	/* ag/file structures */
> >  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
> > 

Re: [PATCH 05/14] xfs: refactor quota expiration timer modification

[Eric Sandeen]

On 2/12/20 7:46 PM, Darrick J. Wong wrote:
> On Wed, Feb 12, 2020 at 05:57:24PM -0600, Eric Sandeen wrote:
>> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>

...

>>>  	} else {
>>>  		if (!over) {
>>> -			d->d_btimer = 0;
>>> +			xfs_quota_clear_timer(&d->d_btimer);
>>
>> yeah that's a very fancy way to say "= 0" ;)
>>
>> ...
> 
> Yes, that's a fancy way to assign zero.  However, consider that for
> bigtime support, I had to add an incore quota timer so that timers more
> or less fire when they're supposed to, and now there's a function to
> convert the incore timespec64 value to whatever is ondisk:
> 
> /* Clear a quota grace period expiration timer. */
> static inline void
> xfs_quota_clear_timer(
> 	struct xfs_disk_dquot	*ddq,
> 	time64_t		*itimer,
> 	__be32			*dtimer)
> {
> 	struct timespec64	tv = { 0 };
> 
> 	*itimer = tv.tv_sec;
> 	xfs_dquot_to_disk_timestamp(ddq, dtimer, &tv);
> }
> 
> It was at *that* point in the patchset that it seemed easier to call a
> small function three times than to open-code this three times.

+void
+xfs_dquot_to_disk_timestamp(
+	__be32			*dtimer,
+	const struct timespec64	*tv)
+{
+	*dtimer = cpu_to_be32(tv->tv_sec);
+}

 static inline void
 xfs_quota_clear_timer(
+	time64_t		*itimer,
 	__be32			*dtimer)
 {
-	*dtimer = cpu_to_be32(0);
+	struct timespec64	tv = { 0 };
+
+	*itimer = tv.tv_sec;
+	xfs_dquot_to_disk_timestamp(dtimer, &tv);
 } 

xfs_quota_clear_timer(&dqp->q_btimer, &d->d_btimer);

That's still a very fancy way of saying:

        dqp->q_btimer = d->d_btimer = 0;

I think?  Can't really see what value the timespec64 adds here.

-Eric

Re: [PATCH 05/14] xfs: refactor quota expiration timer modification

[Eric Sandeen]

On 2/12/20 9:27 PM, Eric Sandeen wrote:
> On 2/12/20 7:46 PM, Darrick J. Wong wrote:
>> On Wed, Feb 12, 2020 at 05:57:24PM -0600, Eric Sandeen wrote:
>>> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
>>>> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> ...
> 
>>>>  	} else {
>>>>  		if (!over) {
>>>> -			d->d_btimer = 0;
>>>> +			xfs_quota_clear_timer(&d->d_btimer);
>>>
>>> yeah that's a very fancy way to say "= 0" ;)
>>>
>>> ...
>>
>> Yes, that's a fancy way to assign zero.  However, consider that for
>> bigtime support, I had to add an incore quota timer so that timers more
>> or less fire when they're supposed to, and now there's a function to
>> convert the incore timespec64 value to whatever is ondisk:
>>
>> /* Clear a quota grace period expiration timer. */
>> static inline void
>> xfs_quota_clear_timer(
>> 	struct xfs_disk_dquot	*ddq,
>> 	time64_t		*itimer,
>> 	__be32			*dtimer)
>> {
>> 	struct timespec64	tv = { 0 };
>>
>> 	*itimer = tv.tv_sec;
>> 	xfs_dquot_to_disk_timestamp(ddq, dtimer, &tv);
>> }
>>
>> It was at *that* point in the patchset that it seemed easier to call a
>> small function three times than to open-code this three times.
> 
> +void
> +xfs_dquot_to_disk_timestamp(
> +	__be32			*dtimer,
> +	const struct timespec64	*tv)
> +{
> +	*dtimer = cpu_to_be32(tv->tv_sec);
> +}
> 
>  static inline void
>  xfs_quota_clear_timer(
> +	time64_t		*itimer,
>  	__be32			*dtimer)
>  {
> -	*dtimer = cpu_to_be32(0);
> +	struct timespec64	tv = { 0 };
> +
> +	*itimer = tv.tv_sec;
> +	xfs_dquot_to_disk_timestamp(dtimer, &tv);
>  } 
> 
> xfs_quota_clear_timer(&dqp->q_btimer, &d->d_btimer);
> 
> That's still a very fancy way of saying:
> 
>         dqp->q_btimer = d->d_btimer = 0;
> 
> I think?  Can't really see what value the timespec64 adds here.
> 
> -Eric
> 

Actually,

 xfs_quota_set_timer(
+	time64_t		*itimer,
 	__be32			*dtimer,
 	time_t			limit)
 {
-	time64_t		new_timeout;
+	struct timespec64	tv = { 0 };
 
-	new_timeout = xfs_dquot_clamp_timer(get_seconds() + limit);
-	*dtimer = cpu_to_be32(new_timeout);
+	tv.tv_sec = xfs_dquot_clamp_timer(ktime_get_real_seconds() + limit);
+	*itimer = tv.tv_sec;
+	xfs_dquot_to_disk_timestamp(dtimer, &tv);
 }

I'm not sure why there's a timespec64 here either.  Isn't everything
we're dealing with on timers in seconds, using only tv_sec, and time64_t would
suffice instead of using a timespec64 just to carry around a seconds value?

-Eric

Re: [PATCH 05/14] xfs: refactor quota expiration timer modification

[Darrick J. Wong]

On Wed, Feb 12, 2020 at 09:32:33PM -0600, Eric Sandeen wrote:
> 
> 
> On 2/12/20 9:27 PM, Eric Sandeen wrote:
> > On 2/12/20 7:46 PM, Darrick J. Wong wrote:
> >> On Wed, Feb 12, 2020 at 05:57:24PM -0600, Eric Sandeen wrote:
> >>> On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> >>>> From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > ...
> > 
> >>>>  	} else {
> >>>>  		if (!over) {
> >>>> -			d->d_btimer = 0;
> >>>> +			xfs_quota_clear_timer(&d->d_btimer);
> >>>
> >>> yeah that's a very fancy way to say "= 0" ;)
> >>>
> >>> ...
> >>
> >> Yes, that's a fancy way to assign zero.  However, consider that for
> >> bigtime support, I had to add an incore quota timer so that timers more
> >> or less fire when they're supposed to, and now there's a function to
> >> convert the incore timespec64 value to whatever is ondisk:
> >>
> >> /* Clear a quota grace period expiration timer. */
> >> static inline void
> >> xfs_quota_clear_timer(
> >> 	struct xfs_disk_dquot	*ddq,
> >> 	time64_t		*itimer,
> >> 	__be32			*dtimer)
> >> {
> >> 	struct timespec64	tv = { 0 };
> >>
> >> 	*itimer = tv.tv_sec;
> >> 	xfs_dquot_to_disk_timestamp(ddq, dtimer, &tv);
> >> }
> >>
> >> It was at *that* point in the patchset that it seemed easier to call a
> >> small function three times than to open-code this three times.
> > 
> > +void
> > +xfs_dquot_to_disk_timestamp(
> > +	__be32			*dtimer,
> > +	const struct timespec64	*tv)
> > +{
> > +	*dtimer = cpu_to_be32(tv->tv_sec);
> > +}
> > 
> >  static inline void
> >  xfs_quota_clear_timer(
> > +	time64_t		*itimer,
> >  	__be32			*dtimer)
> >  {
> > -	*dtimer = cpu_to_be32(0);
> > +	struct timespec64	tv = { 0 };
> > +
> > +	*itimer = tv.tv_sec;
> > +	xfs_dquot_to_disk_timestamp(dtimer, &tv);
> >  } 
> > 
> > xfs_quota_clear_timer(&dqp->q_btimer, &d->d_btimer);
> > 
> > That's still a very fancy way of saying:
> > 
> >         dqp->q_btimer = d->d_btimer = 0;
> > 
> > I think?  Can't really see what value the timespec64 adds here.
> > 
> > -Eric
> > 
> 
> Actually,
> 
>  xfs_quota_set_timer(
> +	time64_t		*itimer,
>  	__be32			*dtimer,
>  	time_t			limit)
>  {
> -	time64_t		new_timeout;
> +	struct timespec64	tv = { 0 };
>  
> -	new_timeout = xfs_dquot_clamp_timer(get_seconds() + limit);
> -	*dtimer = cpu_to_be32(new_timeout);
> +	tv.tv_sec = xfs_dquot_clamp_timer(ktime_get_real_seconds() + limit);
> +	*itimer = tv.tv_sec;
> +	xfs_dquot_to_disk_timestamp(dtimer, &tv);
>  }
> 
> I'm not sure why there's a timespec64 here either.  Isn't everything
> we're dealing with on timers in seconds, using only tv_sec, and time64_t would
> suffice instead of using a timespec64 just to carry around a seconds value?

Yes, the grace periods recorded in the timer fields in dquot 0 are
intervals measured in seconds.

However, for dquot != 0, the timer fields store the time of the
expiration, so I settled on timespec64 as the incore structure so that
XFS consistently uses struct timespec64 to represent specific points in
time.

(That and time64_t doesn't exist in userspace.)

--D

> -Eric

[PATCH 06/14] 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 |    8 ++++++++
 fs/xfs/xfs_ondisk.h        |    2 ++
 fs/xfs/xfs_qm_syscalls.c   |   35 +++++++++++++++++++++++------------
 fs/xfs/xfs_trans_dquot.c   |   16 ++++++++++++----
 4 files changed, 45 insertions(+), 16 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 95761b38fe86..557db5e51eec 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1188,6 +1188,10 @@ 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 length of quota grace periods are unsigned 32-bit quantities in units of
+ * seconds (which are stored in the root dquot).  A value of zero means to use
+ * the default period.
  */
 
 /*
@@ -1202,6 +1206,10 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
  */
 #define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
 
+/* Quota grace periods, ranging from zero (use the defaults) to ~136 years. */
+#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. This is the q_core of the struct xfs_dquot that
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 52dc5326b7bf..b8811f927a3c 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -27,6 +27,8 @@ xfs_check_ondisk_structs(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);
 
 	/* ag/file structures */
 	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 74220948a360..20a6d304d1be 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -438,6 +438,20 @@ xfs_qm_scall_quotaon(
 	return 0;
 }
 
+/* Set a new quota grace period. */
+static inline void
+xfs_qm_set_grace(
+	time_t			*qi_limit,
+	__be32			*dtimer,
+	const s64		grace)
+{
+	time64_t		new_grace;
+
+	new_grace = clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN,
+					     XFS_DQ_GRACE_MAX);
+	*dtimer = cpu_to_be32(new_grace);
+}
+
 #define XFS_QC_MASK \
 	(QC_LIMIT_MASK | QC_TIMER_MASK | QC_WARNS_MASK)
 
@@ -567,18 +581,15 @@ xfs_qm_scall_setqlim(
 		 * soft and hard limit values (already done, above), and
 		 * for warnings.
 		 */
-		if (newlim->d_fieldmask & QC_SPC_TIMER) {
-			q->qi_btimelimit = newlim->d_spc_timer;
-			ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
-		}
-		if (newlim->d_fieldmask & QC_INO_TIMER) {
-			q->qi_itimelimit = newlim->d_ino_timer;
-			ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
-		}
-		if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
-			q->qi_rtbtimelimit = newlim->d_rt_spc_timer;
-			ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
-		}
+		if (newlim->d_fieldmask & QC_SPC_TIMER)
+			xfs_qm_set_grace(&q->qi_btimelimit, &ddq->d_btimer,
+					newlim->d_spc_timer);
+		if (newlim->d_fieldmask & QC_INO_TIMER)
+			xfs_qm_set_grace(&q->qi_itimelimit, &ddq->d_itimer,
+					newlim->d_ino_timer);
+		if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
+			xfs_qm_set_grace(&q->qi_rtbtimelimit, &ddq->d_rtbtimer,
+					newlim->d_rt_spc_timer);
 		if (newlim->d_fieldmask & QC_SPC_WARNS)
 			q->qi_bwarnlimit = newlim->d_spc_warns;
 		if (newlim->d_fieldmask & QC_INO_WARNS)
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 248cfc369efc..7a2a3bd11db9 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -563,6 +563,14 @@ xfs_quota_warn(
 			   mp->m_super->s_dev, type);
 }
 
+/* Has a quota grace period expired? */
+static inline bool
+xfs_quota_timer_exceeded(
+	time64_t		timer)
+{
+	return timer != 0 && get_seconds() > timer;
+}
+
 /*
  * This reserves disk blocks and inodes against a dquot.
  * Flags indicate if the dquot is to be locked here and also
@@ -580,7 +588,7 @@ xfs_trans_dqresv(
 {
 	xfs_qcnt_t		hardlimit;
 	xfs_qcnt_t		softlimit;
-	time_t			timer;
+	time64_t		timer;
 	xfs_qwarncnt_t		warns;
 	xfs_qwarncnt_t		warnlimit;
 	xfs_qcnt_t		total_count;
@@ -635,7 +643,7 @@ xfs_trans_dqresv(
 				goto error_return;
 			}
 			if (softlimit && total_count > softlimit) {
-				if ((timer != 0 && get_seconds() > timer) ||
+				if (xfs_quota_timer_exceeded(timer) ||
 				    (warns != 0 && warns >= warnlimit)) {
 					xfs_quota_warn(mp, dqp,
 						       QUOTA_NL_BSOFTLONGWARN);
@@ -662,8 +670,8 @@ xfs_trans_dqresv(
 				goto error_return;
 			}
 			if (softlimit && total_count > softlimit) {
-				if  ((timer != 0 && get_seconds() > timer) ||
-				     (warns != 0 && warns >= warnlimit)) {
+				if (xfs_quota_timer_exceeded(timer) ||
+				    (warns != 0 && warns >= warnlimit)) {
 					xfs_quota_warn(mp, dqp,
 						       QUOTA_NL_ISOFTLONGWARN);
 					goto error_return;

Re: [PATCH 06/14] xfs: refactor default quota grace period setting code

[Eric Sandeen]

On 12/31/19 7:11 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>
> ---
>  fs/xfs/libxfs/xfs_format.h |    8 ++++++++
>  fs/xfs/xfs_ondisk.h        |    2 ++
>  fs/xfs/xfs_qm_syscalls.c   |   35 +++++++++++++++++++++++------------
>  fs/xfs/xfs_trans_dquot.c   |   16 ++++++++++++----
>  4 files changed, 45 insertions(+), 16 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 95761b38fe86..557db5e51eec 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1188,6 +1188,10 @@ 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 length of quota grace periods are unsigned 32-bit quantities in units of
> + * seconds (which are stored in the root dquot).  A value of zero means to use
> + * the default period.

Doesn't a value of zero mean that the soft limit has not been exceeded, and no
timer is in force?  And when soft limit is exceeded, the timer starts ticking
based on the value in the root dquot?

i.e. you can't set a custom per-user grace period, can you?

Perhaps:

* The length of quota grace periods are unsigned 32-bit quantities in units of
* seconds.  The grace period for each quota type is stored in the root dquot
* and is applied/transferred to a user quota when it exceeds a soft limit.

>   */
>  
>  /*
> @@ -1202,6 +1206,10 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>   */
>  #define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
>  
> +/* Quota grace periods, ranging from zero (use the defaults) to ~136 years. */

same thing.  The default can be set between 0 and ~136 years, that gets transferred
to any user who exceeds soft quota, and it counts down from there.

> +#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. This is the q_core of the struct xfs_dquot that
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 52dc5326b7bf..b8811f927a3c 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -27,6 +27,8 @@ xfs_check_ondisk_structs(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);

*cough* notondisk *cough*

>  
>  	/* ag/file structures */
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 74220948a360..20a6d304d1be 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -438,6 +438,20 @@ xfs_qm_scall_quotaon(
>  	return 0;
>  }
>  
> +/* Set a new quota grace period. */
> +static inline void
> +xfs_qm_set_grace(
> +	time_t			*qi_limit,
                                 ^ doesn't get used?
> +	__be32			*dtimer,
> +	const s64		grace)
> +{
> +	time64_t		new_grace;
> +
> +	new_grace = clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN,
> +					     XFS_DQ_GRACE_MAX);
> +	*dtimer = cpu_to_be32(new_grace);

You've lost setting the qi_limit here (q->qi_btimelimit etc)

> +}
> +
>  #define XFS_QC_MASK \
>  	(QC_LIMIT_MASK | QC_TIMER_MASK | QC_WARNS_MASK)
>  
> @@ -567,18 +581,15 @@ xfs_qm_scall_setqlim(
>  		 * soft and hard limit values (already done, above), and
>  		 * for warnings.
>  		 */
> -		if (newlim->d_fieldmask & QC_SPC_TIMER) {
> -			q->qi_btimelimit = newlim->d_spc_timer;

i.e. qi_btimelimit never gets set now, which is what actually controls
the timers when a uid/gid/pid goes over softlimit.

> -			ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
> -		}
> -		if (newlim->d_fieldmask & QC_INO_TIMER) {
> -			q->qi_itimelimit = newlim->d_ino_timer;
> -			ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
> -		}
> -		if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
> -			q->qi_rtbtimelimit = newlim->d_rt_spc_timer;
> -			ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
> -		}
> +		if (newlim->d_fieldmask & QC_SPC_TIMER)
> +			xfs_qm_set_grace(&q->qi_btimelimit, &ddq->d_btimer,
> +					newlim->d_spc_timer);
> +		if (newlim->d_fieldmask & QC_INO_TIMER)
> +			xfs_qm_set_grace(&q->qi_itimelimit, &ddq->d_itimer,
> +					newlim->d_ino_timer);
> +		if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
> +			xfs_qm_set_grace(&q->qi_rtbtimelimit, &ddq->d_rtbtimer,
> +					newlim->d_rt_spc_timer);
>  		if (newlim->d_fieldmask & QC_SPC_WARNS)
>  			q->qi_bwarnlimit = newlim->d_spc_warns;
>  		if (newlim->d_fieldmask & QC_INO_WARNS)
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 248cfc369efc..7a2a3bd11db9 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -563,6 +563,14 @@ xfs_quota_warn(
>  			   mp->m_super->s_dev, type);
>  }
>  
> +/* Has a quota grace period expired? */

seems like this is not part of "quota grace period setting code"
- needs to be in a separate patch?

> +static inline bool
> +xfs_quota_timer_exceeded(
> +	time64_t		timer)
> +{
> +	return timer != 0 && get_seconds() > timer;
> +}
> +
>  /*
>   * This reserves disk blocks and inodes against a dquot.
>   * Flags indicate if the dquot is to be locked here and also
> @@ -580,7 +588,7 @@ xfs_trans_dqresv(
>  {
>  	xfs_qcnt_t		hardlimit;
>  	xfs_qcnt_t		softlimit;
> -	time_t			timer;
> +	time64_t		timer;

<this needs rebasing I guess, after b8a0880a37e2f43aa3bcd147182e95a4ebd82279>

>  	xfs_qwarncnt_t		warns;
>  	xfs_qwarncnt_t		warnlimit;
>  	xfs_qcnt_t		total_count;
> @@ -635,7 +643,7 @@ xfs_trans_dqresv(
>  				goto error_return;
>  			}
>  			if (softlimit && total_count > softlimit) {
> -				if ((timer != 0 && get_seconds() > timer) ||
> +				if (xfs_quota_timer_exceeded(timer) ||
>  				    (warns != 0 && warns >= warnlimit)) {
>  					xfs_quota_warn(mp, dqp,
>  						       QUOTA_NL_BSOFTLONGWARN);
> @@ -662,8 +670,8 @@ xfs_trans_dqresv(
>  				goto error_return;
>  			}
>  			if (softlimit && total_count > softlimit) {
> -				if  ((timer != 0 && get_seconds() > timer) ||
> -				     (warns != 0 && warns >= warnlimit)) {
> +				if (xfs_quota_timer_exceeded(timer) ||
> +				    (warns != 0 && warns >= warnlimit)) {

TBH don't really see the point of this refactoring/helper, especially if not
done for warns.  I think open coding is fine.

>  					xfs_quota_warn(mp, dqp,
>  						       QUOTA_NL_ISOFTLONGWARN);
>  					goto error_return;
> 

Re: [PATCH 06/14] xfs: refactor default quota grace period setting code

[Darrick J. Wong]

On Wed, Feb 12, 2020 at 06:15:18PM -0600, Eric Sandeen wrote:
> On 12/31/19 7:11 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>
> > ---
> >  fs/xfs/libxfs/xfs_format.h |    8 ++++++++
> >  fs/xfs/xfs_ondisk.h        |    2 ++
> >  fs/xfs/xfs_qm_syscalls.c   |   35 +++++++++++++++++++++++------------
> >  fs/xfs/xfs_trans_dquot.c   |   16 ++++++++++++----
> >  4 files changed, 45 insertions(+), 16 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > index 95761b38fe86..557db5e51eec 100644
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -1188,6 +1188,10 @@ 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 length of quota grace periods are unsigned 32-bit quantities in units of
> > + * seconds (which are stored in the root dquot).  A value of zero means to use
> > + * the default period.
> 
> Doesn't a value of zero mean that the soft limit has not been exceeded, and no
> timer is in force?  And when soft limit is exceeded, the timer starts ticking
> based on the value in the root dquot?

Yes and yes.

> i.e. you can't set a custom per-user grace period, can you?

And yes.

> Perhaps:
> 
> * The length of quota grace periods are unsigned 32-bit quantities in units of
> * seconds.  The grace period for each quota type is stored in the root dquot
> * and is applied/transferred to a user quota when it exceeds a soft limit.

Much better.  I'll crib your version. :)

> >   */
> >  
> >  /*
> > @@ -1202,6 +1206,10 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> >   */
> >  #define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
> >  
> > +/* Quota grace periods, ranging from zero (use the defaults) to ~136 years. */
> 
> same thing.  The default can be set between 0 and ~136 years, that gets transferred
> to any user who exceeds soft quota, and it counts down from there.

Yeah, I'll crib this too.

--D

> > +#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. This is the q_core of the struct xfs_dquot that
> > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > index 52dc5326b7bf..b8811f927a3c 100644
> > --- a/fs/xfs/xfs_ondisk.h
> > +++ b/fs/xfs/xfs_ondisk.h
> > @@ -27,6 +27,8 @@ xfs_check_ondisk_structs(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);
> 
> *cough* notondisk *cough*
> 
> >  
> >  	/* ag/file structures */
> >  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
> > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> > index 74220948a360..20a6d304d1be 100644
> > --- a/fs/xfs/xfs_qm_syscalls.c
> > +++ b/fs/xfs/xfs_qm_syscalls.c
> > @@ -438,6 +438,20 @@ xfs_qm_scall_quotaon(
> >  	return 0;
> >  }
> >  
> > +/* Set a new quota grace period. */
> > +static inline void
> > +xfs_qm_set_grace(
> > +	time_t			*qi_limit,
>                                  ^ doesn't get used?
> > +	__be32			*dtimer,
> > +	const s64		grace)
> > +{
> > +	time64_t		new_grace;
> > +
> > +	new_grace = clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN,
> > +					     XFS_DQ_GRACE_MAX);
> > +	*dtimer = cpu_to_be32(new_grace);
> 
> You've lost setting the qi_limit here (q->qi_btimelimit etc)
> 
> > +}
> > +
> >  #define XFS_QC_MASK \
> >  	(QC_LIMIT_MASK | QC_TIMER_MASK | QC_WARNS_MASK)
> >  
> > @@ -567,18 +581,15 @@ xfs_qm_scall_setqlim(
> >  		 * soft and hard limit values (already done, above), and
> >  		 * for warnings.
> >  		 */
> > -		if (newlim->d_fieldmask & QC_SPC_TIMER) {
> > -			q->qi_btimelimit = newlim->d_spc_timer;
> 
> i.e. qi_btimelimit never gets set now, which is what actually controls
> the timers when a uid/gid/pid goes over softlimit.
> 
> > -			ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
> > -		}
> > -		if (newlim->d_fieldmask & QC_INO_TIMER) {
> > -			q->qi_itimelimit = newlim->d_ino_timer;
> > -			ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
> > -		}
> > -		if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
> > -			q->qi_rtbtimelimit = newlim->d_rt_spc_timer;
> > -			ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
> > -		}
> > +		if (newlim->d_fieldmask & QC_SPC_TIMER)
> > +			xfs_qm_set_grace(&q->qi_btimelimit, &ddq->d_btimer,
> > +					newlim->d_spc_timer);
> > +		if (newlim->d_fieldmask & QC_INO_TIMER)
> > +			xfs_qm_set_grace(&q->qi_itimelimit, &ddq->d_itimer,
> > +					newlim->d_ino_timer);
> > +		if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
> > +			xfs_qm_set_grace(&q->qi_rtbtimelimit, &ddq->d_rtbtimer,
> > +					newlim->d_rt_spc_timer);
> >  		if (newlim->d_fieldmask & QC_SPC_WARNS)
> >  			q->qi_bwarnlimit = newlim->d_spc_warns;
> >  		if (newlim->d_fieldmask & QC_INO_WARNS)
> > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> > index 248cfc369efc..7a2a3bd11db9 100644
> > --- a/fs/xfs/xfs_trans_dquot.c
> > +++ b/fs/xfs/xfs_trans_dquot.c
> > @@ -563,6 +563,14 @@ xfs_quota_warn(
> >  			   mp->m_super->s_dev, type);
> >  }
> >  
> > +/* Has a quota grace period expired? */
> 
> seems like this is not part of "quota grace period setting code"
> - needs to be in a separate patch?

Yeah, it can be a separate refactor patch.


> > +static inline bool
> > +xfs_quota_timer_exceeded(
> > +	time64_t		timer)
> > +{
> > +	return timer != 0 && get_seconds() > timer;
> > +}
> > +
> >  /*
> >   * This reserves disk blocks and inodes against a dquot.
> >   * Flags indicate if the dquot is to be locked here and also
> > @@ -580,7 +588,7 @@ xfs_trans_dqresv(
> >  {
> >  	xfs_qcnt_t		hardlimit;
> >  	xfs_qcnt_t		softlimit;
> > -	time_t			timer;
> > +	time64_t		timer;
> 
> <this needs rebasing I guess, after b8a0880a37e2f43aa3bcd147182e95a4ebd82279>

Probably. :)

> >  	xfs_qwarncnt_t		warns;
> >  	xfs_qwarncnt_t		warnlimit;
> >  	xfs_qcnt_t		total_count;
> > @@ -635,7 +643,7 @@ xfs_trans_dqresv(
> >  				goto error_return;
> >  			}
> >  			if (softlimit && total_count > softlimit) {
> > -				if ((timer != 0 && get_seconds() > timer) ||
> > +				if (xfs_quota_timer_exceeded(timer) ||
> >  				    (warns != 0 && warns >= warnlimit)) {
> >  					xfs_quota_warn(mp, dqp,
> >  						       QUOTA_NL_BSOFTLONGWARN);
> > @@ -662,8 +670,8 @@ xfs_trans_dqresv(
> >  				goto error_return;
> >  			}
> >  			if (softlimit && total_count > softlimit) {
> > -				if  ((timer != 0 && get_seconds() > timer) ||
> > -				     (warns != 0 && warns >= warnlimit)) {
> > +				if (xfs_quota_timer_exceeded(timer) ||
> > +				    (warns != 0 && warns >= warnlimit)) {
> 
> TBH don't really see the point of this refactoring/helper, especially if not
> done for warns.  I think open coding is fine.

Yeah, in the end the helper doesn't add a lot anymore.  IIRC it did in
previous versions of this patch.

--D

> >  					xfs_quota_warn(mp, dqp,
> >  						       QUOTA_NL_ISOFTLONGWARN);
> >  					goto error_return;
> > 

Re: [PATCH 06/14] xfs: refactor default quota grace period setting code

[Darrick J. Wong]

On Wed, Feb 12, 2020 at 05:53:02PM -0800, Darrick J. Wong wrote:
> On Wed, Feb 12, 2020 at 06:15:18PM -0600, Eric Sandeen wrote:
> > On 12/31/19 7:11 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>
> > > ---
> > >  fs/xfs/libxfs/xfs_format.h |    8 ++++++++
> > >  fs/xfs/xfs_ondisk.h        |    2 ++
> > >  fs/xfs/xfs_qm_syscalls.c   |   35 +++++++++++++++++++++++------------
> > >  fs/xfs/xfs_trans_dquot.c   |   16 ++++++++++++----
> > >  4 files changed, 45 insertions(+), 16 deletions(-)
> > > 
> > > 
> > > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > > index 95761b38fe86..557db5e51eec 100644
> > > --- a/fs/xfs/libxfs/xfs_format.h
> > > +++ b/fs/xfs/libxfs/xfs_format.h
> > > @@ -1188,6 +1188,10 @@ 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 length of quota grace periods are unsigned 32-bit quantities in units of
> > > + * seconds (which are stored in the root dquot).  A value of zero means to use
> > > + * the default period.
> > 
> > Doesn't a value of zero mean that the soft limit has not been exceeded, and no
> > timer is in force?  And when soft limit is exceeded, the timer starts ticking
> > based on the value in the root dquot?
> 
> Yes and yes.
> 
> > i.e. you can't set a custom per-user grace period, can you?
> 
> And yes.
> 
> > Perhaps:
> > 
> > * The length of quota grace periods are unsigned 32-bit quantities in units of
> > * seconds.  The grace period for each quota type is stored in the root dquot
> > * and is applied/transferred to a user quota when it exceeds a soft limit.
> 
> Much better.  I'll crib your version. :)
> 
> > >   */
> > >  
> > >  /*
> > > @@ -1202,6 +1206,10 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> > >   */
> > >  #define XFS_DQ_TIMEOUT_MAX	((int64_t)U32_MAX)
> > >  
> > > +/* Quota grace periods, ranging from zero (use the defaults) to ~136 years. */
> > 
> > same thing.  The default can be set between 0 and ~136 years, that gets transferred
> > to any user who exceeds soft quota, and it counts down from there.
> 
> Yeah, I'll crib this too.
> 
> --D
> 
> > > +#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. This is the q_core of the struct xfs_dquot that
> > > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> > > index 52dc5326b7bf..b8811f927a3c 100644
> > > --- a/fs/xfs/xfs_ondisk.h
> > > +++ b/fs/xfs/xfs_ondisk.h
> > > @@ -27,6 +27,8 @@ xfs_check_ondisk_structs(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);
> > 
> > *cough* notondisk *cough*
> > 
> > >  
> > >  	/* ag/file structures */
> > >  	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
> > > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> > > index 74220948a360..20a6d304d1be 100644
> > > --- a/fs/xfs/xfs_qm_syscalls.c
> > > +++ b/fs/xfs/xfs_qm_syscalls.c
> > > @@ -438,6 +438,20 @@ xfs_qm_scall_quotaon(
> > >  	return 0;
> > >  }
> > >  
> > > +/* Set a new quota grace period. */
> > > +static inline void
> > > +xfs_qm_set_grace(
> > > +	time_t			*qi_limit,
> >                                  ^ doesn't get used?
> > > +	__be32			*dtimer,
> > > +	const s64		grace)
> > > +{
> > > +	time64_t		new_grace;
> > > +
> > > +	new_grace = clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN,
> > > +					     XFS_DQ_GRACE_MAX);
> > > +	*dtimer = cpu_to_be32(new_grace);
> > 
> > You've lost setting the qi_limit here (q->qi_btimelimit etc)
> > 
> > > +}
> > > +
> > >  #define XFS_QC_MASK \
> > >  	(QC_LIMIT_MASK | QC_TIMER_MASK | QC_WARNS_MASK)
> > >  
> > > @@ -567,18 +581,15 @@ xfs_qm_scall_setqlim(
> > >  		 * soft and hard limit values (already done, above), and
> > >  		 * for warnings.
> > >  		 */
> > > -		if (newlim->d_fieldmask & QC_SPC_TIMER) {
> > > -			q->qi_btimelimit = newlim->d_spc_timer;
> > 
> > i.e. qi_btimelimit never gets set now, which is what actually controls
> > the timers when a uid/gid/pid goes over softlimit.

Oops, good catch.  Thank you for starting a review of this!

--D

> > > -			ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
> > > -		}
> > > -		if (newlim->d_fieldmask & QC_INO_TIMER) {
> > > -			q->qi_itimelimit = newlim->d_ino_timer;
> > > -			ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
> > > -		}
> > > -		if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
> > > -			q->qi_rtbtimelimit = newlim->d_rt_spc_timer;
> > > -			ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
> > > -		}
> > > +		if (newlim->d_fieldmask & QC_SPC_TIMER)
> > > +			xfs_qm_set_grace(&q->qi_btimelimit, &ddq->d_btimer,
> > > +					newlim->d_spc_timer);
> > > +		if (newlim->d_fieldmask & QC_INO_TIMER)
> > > +			xfs_qm_set_grace(&q->qi_itimelimit, &ddq->d_itimer,
> > > +					newlim->d_ino_timer);
> > > +		if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
> > > +			xfs_qm_set_grace(&q->qi_rtbtimelimit, &ddq->d_rtbtimer,
> > > +					newlim->d_rt_spc_timer);
> > >  		if (newlim->d_fieldmask & QC_SPC_WARNS)
> > >  			q->qi_bwarnlimit = newlim->d_spc_warns;
> > >  		if (newlim->d_fieldmask & QC_INO_WARNS)
> > > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> > > index 248cfc369efc..7a2a3bd11db9 100644
> > > --- a/fs/xfs/xfs_trans_dquot.c
> > > +++ b/fs/xfs/xfs_trans_dquot.c
> > > @@ -563,6 +563,14 @@ xfs_quota_warn(
> > >  			   mp->m_super->s_dev, type);
> > >  }
> > >  
> > > +/* Has a quota grace period expired? */
> > 
> > seems like this is not part of "quota grace period setting code"
> > - needs to be in a separate patch?
> 
> Yeah, it can be a separate refactor patch.
> 
> 
> > > +static inline bool
> > > +xfs_quota_timer_exceeded(
> > > +	time64_t		timer)
> > > +{
> > > +	return timer != 0 && get_seconds() > timer;
> > > +}
> > > +
> > >  /*
> > >   * This reserves disk blocks and inodes against a dquot.
> > >   * Flags indicate if the dquot is to be locked here and also
> > > @@ -580,7 +588,7 @@ xfs_trans_dqresv(
> > >  {
> > >  	xfs_qcnt_t		hardlimit;
> > >  	xfs_qcnt_t		softlimit;
> > > -	time_t			timer;
> > > +	time64_t		timer;
> > 
> > <this needs rebasing I guess, after b8a0880a37e2f43aa3bcd147182e95a4ebd82279>
> 
> Probably. :)
> 
> > >  	xfs_qwarncnt_t		warns;
> > >  	xfs_qwarncnt_t		warnlimit;
> > >  	xfs_qcnt_t		total_count;
> > > @@ -635,7 +643,7 @@ xfs_trans_dqresv(
> > >  				goto error_return;
> > >  			}
> > >  			if (softlimit && total_count > softlimit) {
> > > -				if ((timer != 0 && get_seconds() > timer) ||
> > > +				if (xfs_quota_timer_exceeded(timer) ||
> > >  				    (warns != 0 && warns >= warnlimit)) {
> > >  					xfs_quota_warn(mp, dqp,
> > >  						       QUOTA_NL_BSOFTLONGWARN);
> > > @@ -662,8 +670,8 @@ xfs_trans_dqresv(
> > >  				goto error_return;
> > >  			}
> > >  			if (softlimit && total_count > softlimit) {
> > > -				if  ((timer != 0 && get_seconds() > timer) ||
> > > -				     (warns != 0 && warns >= warnlimit)) {
> > > +				if (xfs_quota_timer_exceeded(timer) ||
> > > +				    (warns != 0 && warns >= warnlimit)) {
> > 
> > TBH don't really see the point of this refactoring/helper, especially if not
> > done for warns.  I think open coding is fine.
> 
> Yeah, in the end the helper doesn't add a lot anymore.  IIRC it did in
> previous versions of this patch.
> 
> --D
> 
> > >  					xfs_quota_warn(mp, dqp,
> > >  						       QUOTA_NL_ISOFTLONGWARN);
> > >  					goto error_return;
> > > 

[PATCH 07/14] 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>
---
 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 557db5e51eec..b3ace144f6ce 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -848,10 +848,10 @@ typedef 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
@@ -896,9 +896,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 */
@@ -923,7 +923,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 8ef31d71a9c7..6694be6ddc3c 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 08/14] 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>
---
 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 dee6ff909c88..ea8b7f5bae59 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -323,58 +323,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 fd94b1078722..1351a9db68b0 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -58,8 +58,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);
 void	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);
 
 bool	xfs_dinode_good_version(struct xfs_mount *mp, __u8 version);
 
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 8bd5d0de6321..826bb6b777d3 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 07a60e74c39c..2b4d92d173fb 100644
--- a/fs/xfs/xfs_inode_item.h
+++ b/fs/xfs/xfs_inode_item.h
@@ -40,4 +40,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 09/14] xfs: refactor timestamp coding

[Darrick J. Wong]

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

Refactor 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>
---
 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       |   42 +++++++++++++++++++++++++----------------
 4 files changed, 74 insertions(+), 40 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index ea8b7f5bae59..66f7895a9fee 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -201,6 +201,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);
+}
+
 void
 xfs_inode_from_disk(
 	struct xfs_inode	*ip,
@@ -236,12 +245,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);
 	inode->i_generation = be32_to_cpu(from->di_gen);
 	inode->i_mode = be16_to_cpu(from->di_mode);
 
@@ -259,13 +265,21 @@ xfs_inode_from_disk(
 	if (to->di_version == 3) {
 		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);
 	}
 }
 
+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,
@@ -286,12 +300,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);
@@ -309,8 +320,7 @@ xfs_inode_to_disk(
 
 	if (from->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 1351a9db68b0..8eca71ac0c5a 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -75,4 +75,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 826bb6b777d3..40131cefc165 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -295,6 +295,15 @@ xfs_inode_item_format_attr_fork(
 	}
 }
 
+static inline void
+xfs_from_log_timestamp(
+	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 +321,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_from_log_timestamp(&to->di_atime, &from->di_atime);
+	xfs_from_log_timestamp(&to->di_mtime, &from->di_mtime);
+	xfs_from_log_timestamp(&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 +339,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_from_log_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(from->di_ino);
@@ -347,6 +352,15 @@ xfs_log_dinode_to_disk(
 	}
 }
 
+static inline void
+xfs_to_log_timestamp(
+	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,
@@ -367,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_to_log_timestamp(&to->di_atime, &inode->i_atime);
+	xfs_to_log_timestamp(&to->di_mtime, &inode->i_mtime);
+	xfs_to_log_timestamp(&to->di_ctime, &inode->i_ctime);
 	to->di_nlink = inode->i_nlink;
 	to->di_gen = inode->i_generation;
 	to->di_mode = inode->i_mode;
@@ -393,8 +404,7 @@ xfs_inode_to_log_dinode(
 
 	if (from->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_to_log_timestamp(&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 10/14] 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>
---
 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 b3ace144f6ce..8a1bb33ebdd5 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -848,9 +848,11 @@ typedef 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 */
+	};
 };
 
 /*
@@ -896,9 +898,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 */
@@ -923,7 +925,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 66f7895a9fee..f4725e4916ab 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -204,7 +204,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);
@@ -273,7 +273,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 8eca71ac0c5a..787a50df232f 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -76,8 +76,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 6694be6ddc3c..c98060115352 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 40131cefc165..a8d3c6aad1b8 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -297,8 +297,8 @@ xfs_inode_item_format_attr_fork(
 
 static inline void
 xfs_from_log_timestamp(
-	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);
@@ -354,7 +354,7 @@ xfs_log_dinode_to_disk(
 
 static inline void
 xfs_to_log_timestamp(
-	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 b8811f927a3c..5a3372fb6e6d 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -53,7 +53,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);
@@ -132,7 +132,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 11/14] 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>
---
 fs/xfs/libxfs/xfs_format.h     |   38 +++++++++++++++++++++++++++-
 fs/xfs/libxfs/xfs_fs.h         |    1 +
 fs/xfs/libxfs/xfs_inode_buf.c  |   55 +++++++++++++++++++++++++++++++++-------
 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/scrub/inode.c           |   16 ++++++++----
 fs/xfs/scrub/inode_repair.c    |    2 +
 fs/xfs/xfs_inode.c             |   19 +++++++++-----
 fs/xfs/xfs_inode_item.c        |   36 ++++++++++++++++++++------
 fs/xfs/xfs_ioctl.c             |    3 +-
 fs/xfs/xfs_ondisk.h            |    3 ++
 fs/xfs/xfs_super.c             |   13 ++++++++-
 13 files changed, 163 insertions(+), 36 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 8a1bb33ebdd5..9b127e4f4077 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|	\
@@ -548,6 +549,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
@@ -847,12 +854,18 @@ typedef 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;
 };
 
 /*
@@ -867,6 +880,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.
  *
@@ -1094,12 +1126,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 40bdea01eff4..86fd287017ca 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 f4725e4916ab..d0084f47f246 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -203,9 +203,22 @@ xfs_imap_to_bp(
 
 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);
 }
@@ -217,7 +230,7 @@ xfs_inode_from_disk(
 {
 	struct xfs_icdinode	*to = &ip->i_d;
 	struct inode		*inode = VFS_I(ip);
-
+	struct xfs_mount	*mp = ip->i_mount;
 
 	/*
 	 * Convert v1 inodes immediately to v2 inode format as this is the
@@ -245,9 +258,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);
 	inode->i_generation = be32_to_cpu(from->di_gen);
 	inode->i_mode = be16_to_cpu(from->di_mode);
 
@@ -265,17 +278,35 @@ xfs_inode_from_disk(
 	if (to->di_version == 3) {
 		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);
+		/*
+		 * Convert this inode's timestamps to bigtime format the next
+		 * time we write it out to disk.
+		 */
+		if (xfs_sb_version_hasbigtime(&mp->m_sb))
+			to->di_flags2 |= XFS_DIFLAG2_BIGTIME;
 	}
 }
 
 void
 xfs_inode_to_disk_timestamp(
+	struct xfs_icdinode		*from,
 	union xfs_timestamp		*ts,
 	const struct timespec64		*tv)
 {
+	if (from->di_version >= 3 &&
+	    (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);
 }
@@ -300,9 +331,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);
@@ -320,7 +351,8 @@ xfs_inode_to_disk(
 
 	if (from->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);
@@ -539,6 +571,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 787a50df232f..ea6818b7175f 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -75,9 +75,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 c98060115352..17789cf130e3 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 4a923545465d..4e5b27a4b4b4 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -1133,6 +1133,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/scrub/inode.c b/fs/xfs/scrub/inode.c
index 9f036053fdb7..b354825f4e51 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;
 
+	/* the incore bigtime iflag always follows the feature flag */
+	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/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 1569b720ee91..bd4374f49035 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -169,6 +169,8 @@ xrep_dinode_flags(
 		flags2 &= ~XFS_DIFLAG2_REFLINK;
 	if (flags2 & XFS_DIFLAG2_REFLINK)
 		flags2 &= ~XFS_DIFLAG2_DAX;
+	if (!xfs_sb_version_hasbigtime(&mp->m_sb))
+		flags2 &= ~XFS_DIFLAG2_BIGTIME;
 	dip->di_flags = cpu_to_be16(flags);
 	dip->di_flags2 = cpu_to_be64(flags2);
 }
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index b45f7bdb6122..9863081bdfe9 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -850,6 +850,8 @@ xfs_ialloc(
 	if (ip->i_d.di_version == 3) {
 		inode_set_iversion(inode, 1);
 		ip->i_d.di_flags2 = 0;
+		if (xfs_sb_version_hasbigtime(&ip->i_mount->m_sb))
+			ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME;
 		ip->i_d.di_cowextsize = 0;
 		ip->i_d.di_crtime = tv;
 	}
@@ -911,16 +913,12 @@ xfs_ialloc(
 		    (pip->i_d.di_flags2 & XFS_DIFLAG2_ANY) &&
 		    pip->i_d.di_version == 3 &&
 		    ip->i_d.di_version == 3) {
-			uint64_t	di_flags2 = 0;
-
 			if (pip->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE) {
-				di_flags2 |= XFS_DIFLAG2_COWEXTSIZE;
+				ip->i_d.di_flags2 |= XFS_DIFLAG2_COWEXTSIZE;
 				ip->i_d.di_cowextsize = pip->i_d.di_cowextsize;
 			}
 			if (pip->i_d.di_flags2 & XFS_DIFLAG2_DAX)
-				di_flags2 |= XFS_DIFLAG2_DAX;
-
-			ip->i_d.di_flags2 |= di_flags2;
+				ip->i_d.di_flags2 |= XFS_DIFLAG2_DAX;
 		}
 		/* FALLTHROUGH */
 	case S_IFLNK:
@@ -3000,6 +2998,8 @@ 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;
+	if (xfs_sb_version_hasbigtime(&ip->i_mount->m_sb))
+		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_d.di_format = XFS_DINODE_FMT_EXTENTS;
@@ -4083,6 +4083,13 @@ xfs_iflush_int(
 			__func__, ip->i_ino, ip->i_d.di_forkoff, ip);
 		goto corrupt_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 corrupt_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 a8d3c6aad1b8..168d53062fab 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -297,9 +297,16 @@ xfs_inode_item_format_attr_fork(
 
 static inline void
 xfs_from_log_timestamp(
+	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);
 }
@@ -321,9 +328,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_from_log_timestamp(&to->di_atime, &from->di_atime);
-	xfs_from_log_timestamp(&to->di_mtime, &from->di_mtime);
-	xfs_from_log_timestamp(&to->di_ctime, &from->di_ctime);
+	xfs_from_log_timestamp(from, &to->di_atime, &from->di_atime);
+	xfs_from_log_timestamp(from, &to->di_mtime, &from->di_mtime);
+	xfs_from_log_timestamp(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);
@@ -339,7 +346,7 @@ xfs_log_dinode_to_disk(
 
 	if (from->di_version == 3) {
 		to->di_changecount = cpu_to_be64(from->di_changecount);
-		xfs_from_log_timestamp(&to->di_crtime, &from->di_crtime);
+		xfs_from_log_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(from->di_ino);
@@ -354,9 +361,20 @@ xfs_log_dinode_to_disk(
 
 static inline void
 xfs_to_log_timestamp(
+	struct xfs_icdinode		*from,
 	union xfs_ictimestamp		*its,
 	const struct timespec64		*ts)
 {
+	if (from->di_version >= 3 &&
+	    (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 +399,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_to_log_timestamp(&to->di_atime, &inode->i_atime);
-	xfs_to_log_timestamp(&to->di_mtime, &inode->i_mtime);
-	xfs_to_log_timestamp(&to->di_ctime, &inode->i_ctime);
+	xfs_to_log_timestamp(from, &to->di_atime, &inode->i_atime);
+	xfs_to_log_timestamp(from, &to->di_mtime, &inode->i_mtime);
+	xfs_to_log_timestamp(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;
@@ -404,7 +422,7 @@ xfs_inode_to_log_dinode(
 
 	if (from->di_version == 3) {
 		to->di_changecount = inode_peek_iversion(inode);
-		xfs_to_log_timestamp(&to->di_crtime, &from->di_crtime);
+		xfs_to_log_timestamp(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;
@@ -412,6 +430,8 @@ xfs_inode_to_log_dinode(
 		memset(to->di_pad2, 0, sizeof(to->di_pad2));
 		uuid_copy(&to->di_uuid, &ip->i_mount->m_sb.sb_meta_uuid);
 		to->di_flushiter = 0;
+		ASSERT((from->di_flags2 & XFS_DIFLAG2_BIGTIME) ||
+		       !xfs_sb_version_hasbigtime(&ip->i_mount->m_sb));
 	} else {
 		to->di_flushiter = from->di_flushiter;
 	}
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index d11857125f45..911b71708587 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1076,7 +1076,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 5a3372fb6e6d..86b9e0e07f84 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -25,6 +25,9 @@ xfs_check_ondisk_structs(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);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 3bddf13cd8ea..0dd6bc7b82c1 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1582,8 +1582,13 @@ xfs_fc_fill_super(
 	sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
 	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);
@@ -1592,6 +1597,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) {
 		bool rtdev_is_dax = false, datadev_is_dax;
 

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

[Amir Goldstein]

> @@ -265,17 +278,35 @@ xfs_inode_from_disk(
>         if (to->di_version == 3) {
>                 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);
> +               /*
> +                * Convert this inode's timestamps to bigtime format the next
> +                * time we write it out to disk.
> +                */
> +               if (xfs_sb_version_hasbigtime(&mp->m_sb))
> +                       to->di_flags2 |= XFS_DIFLAG2_BIGTIME;
>         }

This feels wrong.
incore inode has a union for timestamp.
This flag should indicate how the union should be interpreted
otherwise it is going to be very easy to stumble on that in future code.

So either convert incore timestamp now or check hasbigtime when
we write to disk.

Thanks,
Amir.

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

[Darrick J. Wong]

On Sun, May 31, 2020 at 03:30:42PM +0300, Amir Goldstein wrote:
> > @@ -265,17 +278,35 @@ xfs_inode_from_disk(
> >         if (to->di_version == 3) {
> >                 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);
> > +               /*
> > +                * Convert this inode's timestamps to bigtime format the next
> > +                * time we write it out to disk.
> > +                */
> > +               if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > +                       to->di_flags2 |= XFS_DIFLAG2_BIGTIME;
> >         }
> 
> This feels wrong.
> incore inode has a union for timestamp.
> This flag should indicate how the union should be interpreted
> otherwise it is going to be very easy to stumble on that in future code.

Hm?  I think you've gotten it backwards.

The incore inode (struct xfs_icdinode) has a struct timespec64.

The ondisk inode (struct xfs_dinode) has a union xfs_timestamp.

xfs_inode_from_disk_timestamp uses the ondisk inode (from) to convert
the ondisk timestamp (&from->di_crtime) to the incore inode
(&to->di_crtime).

In other words, we use the ondisk flag and union to load the ondisk
timestamp into a format-agnostic incore structure.  Then we set BIGTIME
in the incore inode (to->di_flags2).

If and when we write the inode back out to disk, we'll see that BIGTIME
is set in the incore inode, and convert the incore structure into the
bigtime encoding on disk.

> So either convert incore timestamp now or check hasbigtime when
> we write to disk.

That's more or less what this is doing.  We read the timestamp in from
disk in whatever format it's in, and then set ourselves up to write it
back out in bigtime format.

--D

> Thanks,
> Amir.

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

[Amir Goldstein]

On Tue, Jun 2, 2020 at 2:17 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Sun, May 31, 2020 at 03:30:42PM +0300, Amir Goldstein wrote:
> > > @@ -265,17 +278,35 @@ xfs_inode_from_disk(
> > >         if (to->di_version == 3) {
> > >                 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);
> > > +               /*
> > > +                * Convert this inode's timestamps to bigtime format the next
> > > +                * time we write it out to disk.
> > > +                */
> > > +               if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > > +                       to->di_flags2 |= XFS_DIFLAG2_BIGTIME;
> > >         }
> >
> > This feels wrong.
> > incore inode has a union for timestamp.
> > This flag should indicate how the union should be interpreted
> > otherwise it is going to be very easy to stumble on that in future code.
>
> Hm?  I think you've gotten it backwards.
>
> The incore inode (struct xfs_icdinode) has a struct timespec64.
>
> The ondisk inode (struct xfs_dinode) has a union xfs_timestamp.
>
> xfs_inode_from_disk_timestamp uses the ondisk inode (from) to convert
> the ondisk timestamp (&from->di_crtime) to the incore inode
> (&to->di_crtime).
>
> In other words, we use the ondisk flag and union to load the ondisk
> timestamp into a format-agnostic incore structure.  Then we set BIGTIME
> in the incore inode (to->di_flags2).
>
> If and when we write the inode back out to disk, we'll see that BIGTIME
> is set in the incore inode, and convert the incore structure into the
> bigtime encoding on disk.
>
> > So either convert incore timestamp now or check hasbigtime when
> > we write to disk.
>
> That's more or less what this is doing.  We read the timestamp in from
> disk in whatever format it's in, and then set ourselves up to write it
> back out in bigtime format.
>

I stand corrected. xfs_ictimestamp got me confused.
Everything does look consistent.

Thanks,
Amir.

[PATCH 12/14] xfs: cache quota grace period expiration times incore

[Darrick J. Wong]

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

Create an in-core timestamp that will tell us when a quota's grace
period expires.  In the subsequent bigtime patchset we will sacrifice
precision in the on-disk grace period timestamp to enable larger
timestamps across the filesystem, but we'll maintain an incore copy so
that we can maintain precision so long as the filesystem isn't
unmounted.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c  |   17 +++++++++++++++++
 fs/xfs/libxfs/xfs_quota_defs.h |    2 ++
 fs/xfs/xfs_dquot.c             |   40 +++++++++++++++++++++++++++-------------
 fs/xfs/xfs_dquot.h             |    8 ++++++++
 fs/xfs/xfs_qm.c                |   38 ++++++++++++++++++++++++++------------
 fs/xfs/xfs_qm_syscalls.c       |   26 +++++++++++++++-----------
 fs/xfs/xfs_trans_dquot.c       |    8 ++++----
 7 files changed, 99 insertions(+), 40 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index bedc1e752b60..72e0fcfef580 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -287,3 +287,20 @@ const struct xfs_buf_ops xfs_dquot_buf_ra_ops = {
 	.verify_read = xfs_dquot_buf_readahead_verify,
 	.verify_write = xfs_dquot_buf_write_verify,
 };
+
+void
+xfs_dquot_from_disk_timestamp(
+	struct timespec64	*tv,
+	__be32			dtimer)
+{
+	tv->tv_nsec = 0;
+	tv->tv_sec = be32_to_cpu(dtimer);
+}
+
+void
+xfs_dquot_to_disk_timestamp(
+	__be32			*dtimer,
+	const struct timespec64	*tv)
+{
+	*dtimer = cpu_to_be32(tv->tv_sec);
+}
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index b2113b17e53c..c453611ade3b 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -144,5 +144,7 @@ extern xfs_failaddr_t xfs_dqblk_verify(struct xfs_mount *mp,
 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, uint type);
+void xfs_dquot_from_disk_timestamp(struct timespec64 *tv, __be32 dtimer);
+void xfs_dquot_to_disk_timestamp(__be32 *dtimer, const struct timespec64 *tv);
 
 #endif	/* __XFS_QUOTA_H__ */
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 44bae5f16b55..763e974f7aad 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -126,21 +126,27 @@ xfs_dquot_clamp_timer(
 /* Set a quota grace period expiration timer. */
 static inline void
 xfs_quota_set_timer(
+	time64_t		*itimer,
 	__be32			*dtimer,
 	time_t			limit)
 {
-	time64_t		new_timeout;
+	struct timespec64	tv = { 0 };
 
-	new_timeout = xfs_dquot_clamp_timer(get_seconds() + limit);
-	*dtimer = cpu_to_be32(new_timeout);
+	tv.tv_sec = xfs_dquot_clamp_timer(ktime_get_real_seconds() + limit);
+	*itimer = tv.tv_sec;
+	xfs_dquot_to_disk_timestamp(dtimer, &tv);
 }
 
 /* Clear a quota grace period expiration timer. */
 static inline void
 xfs_quota_clear_timer(
+	time64_t		*itimer,
 	__be32			*dtimer)
 {
-	*dtimer = cpu_to_be32(0);
+	struct timespec64	tv = { 0 };
+
+	*itimer = tv.tv_sec;
+	xfs_dquot_to_disk_timestamp(dtimer, &tv);
 }
 
 /*
@@ -180,46 +186,46 @@ xfs_qm_adjust_dqtimers(
 
 	over = xfs_quota_exceeded(&d->d_bcount, dqp->q_ina_bcount,
 			&d->d_blk_softlimit, &d->d_blk_hardlimit);
-	if (!d->d_btimer) {
+	if (!dqp->q_btimer) {
 		if (over) {
-			xfs_quota_set_timer(&d->d_btimer,
+			xfs_quota_set_timer(&dqp->q_btimer, &d->d_btimer,
 					mp->m_quotainfo->qi_btimelimit);
 		} else {
 			d->d_bwarns = 0;
 		}
 	} else {
 		if (!over) {
-			xfs_quota_clear_timer(&d->d_btimer);
+			xfs_quota_clear_timer(&dqp->q_btimer, &d->d_btimer);
 		}
 	}
 
 	over = xfs_quota_exceeded(&d->d_icount, dqp->q_ina_icount,
 			&d->d_ino_softlimit, &d->d_ino_hardlimit);
-	if (!d->d_itimer) {
+	if (!dqp->q_itimer) {
 		if (over) {
-			xfs_quota_set_timer(&d->d_itimer,
+			xfs_quota_set_timer(&dqp->q_itimer, &d->d_itimer,
 					mp->m_quotainfo->qi_itimelimit);
 		} else {
 			d->d_iwarns = 0;
 		}
 	} else {
 		if (!over) {
-			xfs_quota_clear_timer(&d->d_itimer);
+			xfs_quota_clear_timer(&dqp->q_itimer, &d->d_itimer);
 		}
 	}
 
 	over = xfs_quota_exceeded(&d->d_rtbcount, dqp->q_ina_rtbcount,
 			&d->d_rtb_softlimit, &d->d_rtb_hardlimit);
-	if (!d->d_rtbtimer) {
+	if (!dqp->q_rtbtimer) {
 		if (over) {
-			xfs_quota_set_timer(&d->d_rtbtimer,
+			xfs_quota_set_timer(&dqp->q_rtbtimer, &d->d_rtbtimer,
 					mp->m_quotainfo->qi_rtbtimelimit);
 		} else {
 			d->d_rtbwarns = 0;
 		}
 	} else {
 		if (!over) {
-			xfs_quota_clear_timer(&d->d_rtbtimer);
+			xfs_quota_clear_timer(&dqp->q_rtbtimer, &d->d_rtbtimer);
 		}
 	}
 }
@@ -520,6 +526,7 @@ xfs_dquot_from_disk(
 	struct xfs_dquot	*dqp,
 	struct xfs_buf		*bp)
 {
+	struct timespec64	tv;
 	struct xfs_disk_dquot	*ddqp = bp->b_addr + dqp->q_bufoffset;
 
 	/* copy everything from disk dquot to the incore dquot */
@@ -533,6 +540,13 @@ xfs_dquot_from_disk(
 	dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
 	dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
 
+	xfs_dquot_from_disk_timestamp(&tv, ddqp->d_btimer);
+	dqp->q_btimer = tv.tv_sec;
+	xfs_dquot_from_disk_timestamp(&tv, ddqp->d_itimer);
+	dqp->q_itimer = tv.tv_sec;
+	xfs_dquot_from_disk_timestamp(&tv, ddqp->d_rtbtimer);
+	dqp->q_rtbtimer = tv.tv_sec;
+
 	/* initialize the dquot speculative prealloc thresholds */
 	xfs_dquot_set_prealloc_limits(dqp);
 }
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index d924da98f66a..99c0d6266fd8 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -60,6 +60,14 @@ struct xfs_dquot {
 	xfs_qcnt_t		q_prealloc_lo_wmark;
 	xfs_qcnt_t		q_prealloc_hi_wmark;
 	int64_t			q_low_space[XFS_QLOWSP_MAX];
+
+	/* incore block grace timeout */
+	time64_t		q_btimer;
+	/* incore inode grace timeout */
+	time64_t		q_itimer;
+	/* incore rt block grace timeout */
+	time64_t		q_rtbtimer;
+
 	struct mutex		q_qlock;
 	struct completion	q_flush;
 	atomic_t		q_pincount;
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 268e028c9ec8..9be123a0902e 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -589,6 +589,7 @@ xfs_qm_init_timelimits(
 	struct xfs_mount	*mp,
 	struct xfs_quotainfo	*qinf)
 {
+	struct timespec64	tv;
 	struct xfs_disk_dquot	*ddqp;
 	struct xfs_dquot	*dqp;
 	uint			type;
@@ -628,12 +629,18 @@ xfs_qm_init_timelimits(
 	 * a user or group before he or she can not perform any
 	 * more writing. If it is zero, a default is used.
 	 */
-	if (ddqp->d_btimer)
-		qinf->qi_btimelimit = be32_to_cpu(ddqp->d_btimer);
-	if (ddqp->d_itimer)
-		qinf->qi_itimelimit = be32_to_cpu(ddqp->d_itimer);
-	if (ddqp->d_rtbtimer)
-		qinf->qi_rtbtimelimit = be32_to_cpu(ddqp->d_rtbtimer);
+	if (ddqp->d_btimer) {
+		xfs_dquot_from_disk_timestamp(&tv, ddqp->d_btimer);
+		qinf->qi_btimelimit = tv.tv_sec;
+	}
+	if (ddqp->d_itimer) {
+		xfs_dquot_from_disk_timestamp(&tv, ddqp->d_itimer);
+		qinf->qi_itimelimit = tv.tv_sec;
+	}
+	if (ddqp->d_rtbtimer) {
+		xfs_dquot_from_disk_timestamp(&tv, ddqp->d_rtbtimer);
+		qinf->qi_rtbtimelimit = tv.tv_sec;
+	}
 	if (ddqp->d_bwarns)
 		qinf->qi_bwarnlimit = be16_to_cpu(ddqp->d_bwarns);
 	if (ddqp->d_iwarns)
@@ -848,16 +855,23 @@ xfs_qm_reset_dqintervals(
 	struct xfs_mount	*mp,
 	struct xfs_disk_dquot	*ddq)
 {
+	struct timespec64	tv = { 0 };
 	struct xfs_quotainfo	*qinf = mp->m_quotainfo;
 
-	if (qinf->qi_btimelimit != XFS_QM_BTIMELIMIT)
-		ddq->d_btimer = cpu_to_be32(qinf->qi_btimelimit);
+	if (qinf->qi_btimelimit != XFS_QM_BTIMELIMIT) {
+		tv.tv_sec = qinf->qi_btimelimit;
+		xfs_dquot_to_disk_timestamp(&ddq->d_btimer, &tv);
+	}
 
-	if (qinf->qi_itimelimit != XFS_QM_ITIMELIMIT)
-		ddq->d_itimer = cpu_to_be32(qinf->qi_itimelimit);
+	if (qinf->qi_itimelimit != XFS_QM_ITIMELIMIT) {
+		tv.tv_sec = qinf->qi_itimelimit;
+		xfs_dquot_to_disk_timestamp(&ddq->d_itimer, &tv);
+	}
 
-	if (qinf->qi_rtbtimelimit != XFS_QM_RTBTIMELIMIT)
-		ddq->d_rtbtimer = cpu_to_be32(qinf->qi_rtbtimelimit);
+	if (qinf->qi_rtbtimelimit != XFS_QM_RTBTIMELIMIT) {
+		tv.tv_sec = qinf->qi_rtbtimelimit;
+		xfs_dquot_to_disk_timestamp(&ddq->d_rtbtimer, &tv);
+	}
 }
 
 STATIC void
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 20a6d304d1be..bd9db42b89b9 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -442,14 +442,17 @@ xfs_qm_scall_quotaon(
 static inline void
 xfs_qm_set_grace(
 	time_t			*qi_limit,
+	time64_t		*itimer,
 	__be32			*dtimer,
 	const s64		grace)
 {
-	time64_t		new_grace;
+	struct timespec64	tv = { 0 };
 
-	new_grace = clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN,
+	tv.tv_sec = clamp_t(time64_t, grace, XFS_DQ_GRACE_MIN,
 					     XFS_DQ_GRACE_MAX);
-	*dtimer = cpu_to_be32(new_grace);
+	*qi_limit = tv.tv_sec;
+	*itimer = tv.tv_sec;
+	xfs_dquot_to_disk_timestamp(dtimer, &tv);
 }
 
 #define XFS_QC_MASK \
@@ -582,13 +585,14 @@ xfs_qm_scall_setqlim(
 		 * for warnings.
 		 */
 		if (newlim->d_fieldmask & QC_SPC_TIMER)
-			xfs_qm_set_grace(&q->qi_btimelimit, &ddq->d_btimer,
-					newlim->d_spc_timer);
+			xfs_qm_set_grace(&q->qi_btimelimit, &dqp->q_btimer,
+					&ddq->d_btimer, newlim->d_spc_timer);
 		if (newlim->d_fieldmask & QC_INO_TIMER)
-			xfs_qm_set_grace(&q->qi_itimelimit, &ddq->d_itimer,
-					newlim->d_ino_timer);
+			xfs_qm_set_grace(&q->qi_itimelimit, &dqp->q_itimer,
+					&ddq->d_itimer, newlim->d_ino_timer);
 		if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
-			xfs_qm_set_grace(&q->qi_rtbtimelimit, &ddq->d_rtbtimer,
+			xfs_qm_set_grace(&q->qi_rtbtimelimit, &dqp->q_rtbtimer,
+					&ddq->d_rtbtimer,
 					newlim->d_rt_spc_timer);
 		if (newlim->d_fieldmask & QC_SPC_WARNS)
 			q->qi_bwarnlimit = newlim->d_spc_warns;
@@ -635,8 +639,8 @@ xfs_qm_scall_getquota_fill_qc(
 	dst->d_ino_softlimit = be64_to_cpu(dqp->q_core.d_ino_softlimit);
 	dst->d_space = XFS_FSB_TO_B(mp, dqp->q_res_bcount - dqp->q_ina_bcount);
 	dst->d_ino_count = dqp->q_res_icount - dqp->q_ina_icount;
-	dst->d_spc_timer = be32_to_cpu(dqp->q_core.d_btimer);
-	dst->d_ino_timer = be32_to_cpu(dqp->q_core.d_itimer);
+	dst->d_spc_timer = dqp->q_btimer;
+	dst->d_ino_timer = dqp->q_itimer;
 	dst->d_ino_warns = be16_to_cpu(dqp->q_core.d_iwarns);
 	dst->d_spc_warns = be16_to_cpu(dqp->q_core.d_bwarns);
 	dst->d_rt_spc_hardlimit =
@@ -645,7 +649,7 @@ xfs_qm_scall_getquota_fill_qc(
 		XFS_FSB_TO_B(mp, be64_to_cpu(dqp->q_core.d_rtb_softlimit));
 	dst->d_rt_space =
 		XFS_FSB_TO_B(mp, dqp->q_res_rtbcount - dqp->q_ina_rtbcount);
-	dst->d_rt_spc_timer = be32_to_cpu(dqp->q_core.d_rtbtimer);
+	dst->d_rt_spc_timer = dqp->q_rtbtimer;
 	dst->d_rt_spc_warns = be16_to_cpu(dqp->q_core.d_rtbwarns);
 
 	/*
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 7a2a3bd11db9..62ef99f705df 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -568,7 +568,7 @@ static inline bool
 xfs_quota_timer_exceeded(
 	time64_t		timer)
 {
-	return timer != 0 && get_seconds() > timer;
+	return timer != 0 && ktime_get_real_seconds() > timer;
 }
 
 /*
@@ -608,7 +608,7 @@ xfs_trans_dqresv(
 		softlimit = be64_to_cpu(dqp->q_core.d_blk_softlimit);
 		if (!softlimit)
 			softlimit = defq->bsoftlimit;
-		timer = be32_to_cpu(dqp->q_core.d_btimer);
+		timer = dqp->q_btimer;
 		warns = be16_to_cpu(dqp->q_core.d_bwarns);
 		warnlimit = dqp->q_mount->m_quotainfo->qi_bwarnlimit;
 		resbcountp = &dqp->q_res_bcount;
@@ -620,7 +620,7 @@ xfs_trans_dqresv(
 		softlimit = be64_to_cpu(dqp->q_core.d_rtb_softlimit);
 		if (!softlimit)
 			softlimit = defq->rtbsoftlimit;
-		timer = be32_to_cpu(dqp->q_core.d_rtbtimer);
+		timer = dqp->q_rtbtimer;
 		warns = be16_to_cpu(dqp->q_core.d_rtbwarns);
 		warnlimit = dqp->q_mount->m_quotainfo->qi_rtbwarnlimit;
 		resbcountp = &dqp->q_res_rtbcount;
@@ -655,7 +655,7 @@ xfs_trans_dqresv(
 		}
 		if (ninos > 0) {
 			total_count = be64_to_cpu(dqp->q_core.d_icount) + ninos;
-			timer = be32_to_cpu(dqp->q_core.d_itimer);
+			timer = dqp->q_itimer;
 			warns = be16_to_cpu(dqp->q_core.d_iwarns);
 			warnlimit = dqp->q_mount->m_quotainfo->qi_iwarnlimit;
 			hardlimit = be64_to_cpu(dqp->q_core.d_ino_hardlimit);

[PATCH 13/14] 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>
---
 fs/xfs/libxfs/xfs_dquot_buf.c  |   72 ++++++++++++++++++++++++++++++++++++++--
 fs/xfs/libxfs/xfs_format.h     |   22 ++++++++++++
 fs/xfs/libxfs/xfs_quota_defs.h |   11 ++++--
 fs/xfs/scrub/quota.c           |    5 +++
 fs/xfs/xfs_dquot.c             |   71 +++++++++++++++++++++++++++++++--------
 fs/xfs/xfs_ondisk.h            |    6 +++
 fs/xfs/xfs_qm.c                |   13 ++++---
 fs/xfs/xfs_qm.h                |    8 ++--
 fs/xfs/xfs_qm_syscalls.c       |   19 ++++++-----
 9 files changed, 186 insertions(+), 41 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 72e0fcfef580..2b5d51a6d64b 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -40,6 +40,8 @@ xfs_dquot_verify(
 	xfs_dqid_t		id,
 	uint			type)	/* used only during quotacheck */
 {
+	uint8_t			dtype;
+
 	/*
 	 * We can encounter an uninitialized dquot buffer for 2 reasons:
 	 * 1. If we crash while deleting the quotainode(s), and those blks got
@@ -60,11 +62,22 @@ xfs_dquot_verify(
 	if (ddq->d_version != XFS_DQUOT_VERSION)
 		return __this_address;
 
-	if (type && ddq->d_flags != type)
+	dtype = ddq->d_flags & XFS_DQ_ALLTYPES;
+	if (type && dtype != type)
+		return __this_address;
+	if (dtype != XFS_DQ_USER &&
+	    dtype != XFS_DQ_PROJ &&
+	    dtype != XFS_DQ_GROUP)
+		return __this_address;
+
+	if (ddq->d_flags & ~(XFS_DQ_ALLTYPES | XFS_DQ_BIGTIME))
 		return __this_address;
-	if (ddq->d_flags != XFS_DQ_USER &&
-	    ddq->d_flags != XFS_DQ_PROJ &&
-	    ddq->d_flags != XFS_DQ_GROUP)
+
+	if ((ddq->d_flags & XFS_DQ_BIGTIME) &&
+	    !xfs_sb_version_hasbigtime(&mp->m_sb))
+		return __this_address;
+
+	if ((ddq->d_flags & XFS_DQ_BIGTIME) && !ddq->d_id)
 		return __this_address;
 
 	if (id != -1 && id != be32_to_cpu(ddq->d_id))
@@ -290,17 +303,68 @@ const struct xfs_buf_ops xfs_dquot_buf_ra_ops = {
 
 void
 xfs_dquot_from_disk_timestamp(
+	struct xfs_disk_dquot	*ddq,
 	struct timespec64	*tv,
 	__be32			dtimer)
 {
 	tv->tv_nsec = 0;
+
+	/* Zero always means zero, regardless of encoding. */
+	if (!dtimer) {
+		tv->tv_sec = 0;
+		return;
+	}
+
+	if (ddq->d_flags & XFS_DQ_BIGTIME) {
+		uint64_t	t;
+
+		t = be32_to_cpu(dtimer);
+		tv->tv_sec = t << XFS_DQ_BIGTIME_SHIFT;
+		return;
+	}
+
 	tv->tv_sec = be32_to_cpu(dtimer);
 }
 
 void
 xfs_dquot_to_disk_timestamp(
+	struct xfs_disk_dquot	*ddq,
 	__be32			*dtimer,
 	const struct timespec64	*tv)
 {
+	/* Zero always means zero, regardless of encoding. */
+	if ((ddq->d_flags & XFS_DQ_BIGTIME) && tv->tv_sec != 0) {
+		uint64_t	t = tv->tv_sec;
+
+		/*
+		 * 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 = roundup_64(t, 1U << XFS_DQ_BIGTIME_SHIFT);
+		*dtimer = cpu_to_be32(t >> XFS_DQ_BIGTIME_SHIFT);
+		return;
+	}
+
 	*dtimer = cpu_to_be32(tv->tv_sec);
 }
+
+/*
+ * Convert the dquot to bigtime format incore so that we'll write out the new
+ * values the next time we flush the dquot to disk.  Skip this for d_id == 0
+ * because that dquot stores the grace period intervals.  Returns true if we
+ * upgraded the format, false otherwise.
+ */
+bool
+xfs_dquot_add_bigtime(
+	struct xfs_mount	*mp,
+	struct xfs_disk_dquot	*ddq)
+{
+	if (xfs_sb_version_hasbigtime(&mp->m_sb) && ddq->d_id &&
+	    !(ddq->d_flags & XFS_DQ_BIGTIME)) {
+		ddq->d_flags |= XFS_DQ_BIGTIME;
+		return true;
+	}
+
+	return false;
+}
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 9b127e4f4077..bab0b23720bb 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1248,6 +1248,28 @@ 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)
+
+/*
+ * 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. This is the q_core of the struct xfs_dquot that
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index c453611ade3b..4ac486fd20a8 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -26,6 +26,7 @@ typedef uint16_t	xfs_qwarncnt_t;
 #define XFS_DQ_GROUP		0x0004		/* a group quota */
 #define XFS_DQ_DIRTY		0x0008		/* dquot is dirty */
 #define XFS_DQ_FREEING		0x0010		/* dquot is being torn down */
+#define XFS_DQ_BIGTIME		0x0020		/* big timestamp format */
 
 #define XFS_DQ_ALLTYPES		(XFS_DQ_USER|XFS_DQ_PROJ|XFS_DQ_GROUP)
 
@@ -34,7 +35,8 @@ typedef uint16_t	xfs_qwarncnt_t;
 	{ XFS_DQ_PROJ,		"PROJ" }, \
 	{ XFS_DQ_GROUP,		"GROUP" }, \
 	{ XFS_DQ_DIRTY,		"DIRTY" }, \
-	{ XFS_DQ_FREEING,	"FREEING" }
+	{ XFS_DQ_FREEING,	"FREEING" }, \
+	{ XFS_DQ_BIGTIME,	"BIGTIME" }
 
 /*
  * We have the possibility of all three quota types being active at once, and
@@ -144,7 +146,10 @@ extern xfs_failaddr_t xfs_dqblk_verify(struct xfs_mount *mp,
 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, uint type);
-void xfs_dquot_from_disk_timestamp(struct timespec64 *tv, __be32 dtimer);
-void xfs_dquot_to_disk_timestamp(__be32 *dtimer, const struct timespec64 *tv);
+void xfs_dquot_from_disk_timestamp(struct xfs_disk_dquot *ddq,
+		struct timespec64 *tv, __be32 dtimer);
+void xfs_dquot_to_disk_timestamp(struct xfs_disk_dquot *ddq,
+		__be32 *dtimer, const struct timespec64 *tv);
+bool xfs_dquot_add_bigtime(struct xfs_mount *mp, struct xfs_disk_dquot *ddq);
 
 #endif	/* __XFS_QUOTA_H__ */
diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
index 64e24fe5dcb2..6ff402bc0a3e 100644
--- a/fs/xfs/scrub/quota.c
+++ b/fs/xfs/scrub/quota.c
@@ -135,6 +135,11 @@ xchk_quota_item(
 	if (d->d_pad0 != cpu_to_be32(0) || d->d_pad != cpu_to_be16(0))
 		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, offset);
 
+	/* incore should always have bigtime iflag set except for root */
+	if (xfs_sb_version_hasbigtime(&mp->m_sb) && d->d_id &&
+	    !(d->d_flags & XFS_DQ_BIGTIME))
+		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, offset);
+
 	/* Check the limits. */
 	bhard = be64_to_cpu(d->d_blk_hardlimit);
 	ihard = be64_to_cpu(d->d_ino_hardlimit);
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 763e974f7aad..88b37098c6f4 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -118,35 +118,42 @@ xfs_quota_exceeded(
  */
 static inline time64_t
 xfs_dquot_clamp_timer(
-	time64_t			timer)
+	struct xfs_disk_dquot	*ddq,
+	time64_t		timer)
 {
+	if (ddq->d_flags & XFS_DQ_BIGTIME)
+		return clamp_t(time64_t, timer, XFS_DQ_BIGTIMEOUT_MIN,
+						XFS_DQ_BIGTIMEOUT_MAX);
 	return clamp_t(time64_t, timer, XFS_DQ_TIMEOUT_MIN, XFS_DQ_TIMEOUT_MAX);
 }
 
 /* Set a quota grace period expiration timer. */
 static inline void
 xfs_quota_set_timer(
+	struct xfs_disk_dquot	*ddq,
 	time64_t		*itimer,
 	__be32			*dtimer,
-	time_t			limit)
+	time64_t		limit)
 {
 	struct timespec64	tv = { 0 };
 
-	tv.tv_sec = xfs_dquot_clamp_timer(ktime_get_real_seconds() + limit);
+	tv.tv_sec = xfs_dquot_clamp_timer(ddq,
+			ktime_get_real_seconds() + limit);
 	*itimer = tv.tv_sec;
-	xfs_dquot_to_disk_timestamp(dtimer, &tv);
+	xfs_dquot_to_disk_timestamp(ddq, dtimer, &tv);
 }
 
 /* Clear a quota grace period expiration timer. */
 static inline void
 xfs_quota_clear_timer(
+	struct xfs_disk_dquot	*ddq,
 	time64_t		*itimer,
 	__be32			*dtimer)
 {
 	struct timespec64	tv = { 0 };
 
 	*itimer = tv.tv_sec;
-	xfs_dquot_to_disk_timestamp(dtimer, &tv);
+	xfs_dquot_to_disk_timestamp(ddq, dtimer, &tv);
 }
 
 /*
@@ -188,14 +195,14 @@ xfs_qm_adjust_dqtimers(
 			&d->d_blk_softlimit, &d->d_blk_hardlimit);
 	if (!dqp->q_btimer) {
 		if (over) {
-			xfs_quota_set_timer(&dqp->q_btimer, &d->d_btimer,
+			xfs_quota_set_timer(d, &dqp->q_btimer, &d->d_btimer,
 					mp->m_quotainfo->qi_btimelimit);
 		} else {
 			d->d_bwarns = 0;
 		}
 	} else {
 		if (!over) {
-			xfs_quota_clear_timer(&dqp->q_btimer, &d->d_btimer);
+			xfs_quota_clear_timer(d, &dqp->q_btimer, &d->d_btimer);
 		}
 	}
 
@@ -203,14 +210,14 @@ xfs_qm_adjust_dqtimers(
 			&d->d_ino_softlimit, &d->d_ino_hardlimit);
 	if (!dqp->q_itimer) {
 		if (over) {
-			xfs_quota_set_timer(&dqp->q_itimer, &d->d_itimer,
+			xfs_quota_set_timer(d, &dqp->q_itimer, &d->d_itimer,
 					mp->m_quotainfo->qi_itimelimit);
 		} else {
 			d->d_iwarns = 0;
 		}
 	} else {
 		if (!over) {
-			xfs_quota_clear_timer(&dqp->q_itimer, &d->d_itimer);
+			xfs_quota_clear_timer(d, &dqp->q_itimer, &d->d_itimer);
 		}
 	}
 
@@ -218,14 +225,16 @@ xfs_qm_adjust_dqtimers(
 			&d->d_rtb_softlimit, &d->d_rtb_hardlimit);
 	if (!dqp->q_rtbtimer) {
 		if (over) {
-			xfs_quota_set_timer(&dqp->q_rtbtimer, &d->d_rtbtimer,
+			xfs_quota_set_timer(d, &dqp->q_rtbtimer,
+					&d->d_rtbtimer,
 					mp->m_quotainfo->qi_rtbtimelimit);
 		} else {
 			d->d_rtbwarns = 0;
 		}
 	} else {
 		if (!over) {
-			xfs_quota_clear_timer(&dqp->q_rtbtimer, &d->d_rtbtimer);
+			xfs_quota_clear_timer(d, &dqp->q_rtbtimer,
+					&d->d_rtbtimer);
 		}
 	}
 }
@@ -261,6 +270,7 @@ 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_flags = type;
+		xfs_dquot_add_bigtime(mp, &d->dd_diskdq);
 		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),
@@ -528,9 +538,10 @@ xfs_dquot_from_disk(
 {
 	struct timespec64	tv;
 	struct xfs_disk_dquot	*ddqp = bp->b_addr + dqp->q_bufoffset;
+	struct xfs_disk_dquot	*iddq = &dqp->q_core;
 
 	/* copy everything from disk dquot to the incore dquot */
-	memcpy(&dqp->q_core, ddqp, sizeof(struct xfs_disk_dquot));
+	memcpy(iddq, ddqp, sizeof(struct xfs_disk_dquot));
 
 	/*
 	 * Reservation counters are defined as reservation plus current usage
@@ -540,13 +551,28 @@ xfs_dquot_from_disk(
 	dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
 	dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
 
-	xfs_dquot_from_disk_timestamp(&tv, ddqp->d_btimer);
+	xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_btimer);
 	dqp->q_btimer = tv.tv_sec;
-	xfs_dquot_from_disk_timestamp(&tv, ddqp->d_itimer);
+	xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_itimer);
 	dqp->q_itimer = tv.tv_sec;
-	xfs_dquot_from_disk_timestamp(&tv, ddqp->d_rtbtimer);
+	xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_rtbtimer);
 	dqp->q_rtbtimer = tv.tv_sec;
 
+	/* Upgrade to bigtime if possible. */
+	if (xfs_dquot_add_bigtime(dqp->q_mount, iddq)) {
+		tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_btimer);
+		xfs_dquot_to_disk_timestamp(iddq, &iddq->d_btimer, &tv);
+		dqp->q_btimer = tv.tv_sec;
+
+		tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_itimer);
+		xfs_dquot_to_disk_timestamp(iddq, &iddq->d_itimer, &tv);
+		dqp->q_itimer = tv.tv_sec;
+
+		tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_rtbtimer);
+		xfs_dquot_to_disk_timestamp(iddq, &iddq->d_rtbtimer, &tv);
+		dqp->q_rtbtimer = tv.tv_sec;
+	}
+
 	/* initialize the dquot speculative prealloc thresholds */
 	xfs_dquot_set_prealloc_limits(dqp);
 }
@@ -1175,6 +1201,21 @@ xfs_qm_dqflush(
 	/* This is the only portion of data that needs to persist */
 	memcpy(ddqp, &dqp->q_core, sizeof(struct xfs_disk_dquot));
 
+	/*
+	 * We should never write non-bigtime dquots to a bigtime fs, except for
+	 * the root dquot.
+	 */
+	if (!(dqp->q_core.d_flags & XFS_DQ_BIGTIME) && dqp->q_core.d_id &&
+	    xfs_sb_version_hasbigtime(&mp->m_sb)) {
+		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
+				be32_to_cpu(ddqp->d_id), __this_address);
+		xfs_buf_relse(bp);
+		xfs_dqfunlock(dqp);
+		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+		xfs_quota_mark_sick(mp, dqp->dq_flags);
+		return -EFSCORRUPTED;
+	}
+
 	/*
 	 * Clear the dirty field and remember the flush lsn for later use.
 	 */
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 86b9e0e07f84..940522f78a37 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -32,6 +32,12 @@ xfs_check_ondisk_structs(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");
 
 	/* ag/file structures */
 	XFS_CHECK_STRUCT_SIZE(struct xfs_acl,			4);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 9be123a0902e..fb671be5ca25 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -630,15 +630,15 @@ xfs_qm_init_timelimits(
 	 * more writing. If it is zero, a default is used.
 	 */
 	if (ddqp->d_btimer) {
-		xfs_dquot_from_disk_timestamp(&tv, ddqp->d_btimer);
+		xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_btimer);
 		qinf->qi_btimelimit = tv.tv_sec;
 	}
 	if (ddqp->d_itimer) {
-		xfs_dquot_from_disk_timestamp(&tv, ddqp->d_itimer);
+		xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_itimer);
 		qinf->qi_itimelimit = tv.tv_sec;
 	}
 	if (ddqp->d_rtbtimer) {
-		xfs_dquot_from_disk_timestamp(&tv, ddqp->d_rtbtimer);
+		xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_rtbtimer);
 		qinf->qi_rtbtimelimit = tv.tv_sec;
 	}
 	if (ddqp->d_bwarns)
@@ -860,17 +860,17 @@ xfs_qm_reset_dqintervals(
 
 	if (qinf->qi_btimelimit != XFS_QM_BTIMELIMIT) {
 		tv.tv_sec = qinf->qi_btimelimit;
-		xfs_dquot_to_disk_timestamp(&ddq->d_btimer, &tv);
+		xfs_dquot_to_disk_timestamp(ddq, &ddq->d_btimer, &tv);
 	}
 
 	if (qinf->qi_itimelimit != XFS_QM_ITIMELIMIT) {
 		tv.tv_sec = qinf->qi_itimelimit;
-		xfs_dquot_to_disk_timestamp(&ddq->d_itimer, &tv);
+		xfs_dquot_to_disk_timestamp(ddq, &ddq->d_itimer, &tv);
 	}
 
 	if (qinf->qi_rtbtimelimit != XFS_QM_RTBTIMELIMIT) {
 		tv.tv_sec = qinf->qi_rtbtimelimit;
-		xfs_dquot_to_disk_timestamp(&ddq->d_rtbtimer, &tv);
+		xfs_dquot_to_disk_timestamp(ddq, &ddq->d_rtbtimer, &tv);
 	}
 }
 
@@ -928,6 +928,7 @@ xfs_qm_reset_dqcounts(
 		ddq->d_rtbwarns = 0;
 		if (!ddq->d_id)
 			xfs_qm_reset_dqintervals(mp, ddq);
+		xfs_dquot_add_bigtime(mp, ddq);
 
 		if (xfs_sb_version_hascrc(&mp->m_sb)) {
 			xfs_update_cksum((char *)&dqb[j],
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index a3d9932f2e65..d3b69524ca18 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -64,9 +64,9 @@ struct xfs_quotainfo {
 	struct xfs_inode	*qi_pquotaip;	/* project quota inode */
 	struct list_lru	 qi_lru;
 	int		 qi_dquots;
-	time_t		 qi_btimelimit;	 /* limit for blks timer */
-	time_t		 qi_itimelimit;	 /* limit for inodes timer */
-	time_t		 qi_rtbtimelimit;/* limit for rt blks timer */
+	time64_t	 qi_btimelimit;	 /* limit for blks timer */
+	time64_t	 qi_itimelimit;	 /* limit for inodes timer */
+	time64_t	 qi_rtbtimelimit;/* limit for rt blks timer */
 	xfs_qwarncnt_t	 qi_bwarnlimit;	 /* limit for blks warnings */
 	xfs_qwarncnt_t	 qi_iwarnlimit;	 /* limit for inodes warnings */
 	xfs_qwarncnt_t	 qi_rtbwarnlimit;/* limit for rt blks warnings */
@@ -84,7 +84,7 @@ xfs_dquot_tree(
 	struct xfs_quotainfo	*qi,
 	int			type)
 {
-	switch (type) {
+	switch (type & XFS_DQ_ALLTYPES) {
 	case XFS_DQ_USER:
 		return &qi->qi_uquota_tree;
 	case XFS_DQ_GROUP:
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index bd9db42b89b9..47b63f820f50 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -441,7 +441,8 @@ xfs_qm_scall_quotaon(
 /* Set a new quota grace period. */
 static inline void
 xfs_qm_set_grace(
-	time_t			*qi_limit,
+	struct xfs_disk_dquot	*ddq,
+	time64_t		*qi_limit,
 	time64_t		*itimer,
 	__be32			*dtimer,
 	const s64		grace)
@@ -452,7 +453,7 @@ xfs_qm_set_grace(
 					     XFS_DQ_GRACE_MAX);
 	*qi_limit = tv.tv_sec;
 	*itimer = tv.tv_sec;
-	xfs_dquot_to_disk_timestamp(dtimer, &tv);
+	xfs_dquot_to_disk_timestamp(ddq, dtimer, &tv);
 }
 
 #define XFS_QC_MASK \
@@ -585,14 +586,14 @@ xfs_qm_scall_setqlim(
 		 * for warnings.
 		 */
 		if (newlim->d_fieldmask & QC_SPC_TIMER)
-			xfs_qm_set_grace(&q->qi_btimelimit, &dqp->q_btimer,
+			xfs_qm_set_grace(ddq, &q->qi_btimelimit, &dqp->q_btimer,
 					&ddq->d_btimer, newlim->d_spc_timer);
 		if (newlim->d_fieldmask & QC_INO_TIMER)
-			xfs_qm_set_grace(&q->qi_itimelimit, &dqp->q_itimer,
+			xfs_qm_set_grace(ddq, &q->qi_itimelimit, &dqp->q_itimer,
 					&ddq->d_itimer, newlim->d_ino_timer);
 		if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
-			xfs_qm_set_grace(&q->qi_rtbtimelimit, &dqp->q_rtbtimer,
-					&ddq->d_rtbtimer,
+			xfs_qm_set_grace(ddq, &q->qi_rtbtimelimit,
+					&dqp->q_rtbtimer, &ddq->d_rtbtimer,
 					newlim->d_rt_spc_timer);
 		if (newlim->d_fieldmask & QC_SPC_WARNS)
 			q->qi_bwarnlimit = newlim->d_spc_warns;
@@ -658,11 +659,11 @@ xfs_qm_scall_getquota_fill_qc(
 	 * so return zeroes in that case.
 	 */
 	if ((!XFS_IS_UQUOTA_ENFORCED(mp) &&
-	     dqp->q_core.d_flags == XFS_DQ_USER) ||
+	     (dqp->q_core.d_flags & XFS_DQ_ALLTYPES) == XFS_DQ_USER) ||
 	    (!XFS_IS_GQUOTA_ENFORCED(mp) &&
-	     dqp->q_core.d_flags == XFS_DQ_GROUP) ||
+	     (dqp->q_core.d_flags & XFS_DQ_ALLTYPES) == XFS_DQ_GROUP) ||
 	    (!XFS_IS_PQUOTA_ENFORCED(mp) &&
-	     dqp->q_core.d_flags == XFS_DQ_PROJ)) {
+	     (dqp->q_core.d_flags & XFS_DQ_ALLTYPES) == XFS_DQ_PROJ)) {
 		dst->d_spc_timer = 0;
 		dst->d_ino_timer = 0;
 		dst->d_rt_spc_timer = 0;

Re: [PATCH 13/14] xfs: enable bigtime for quota timers

[Amir Goldstein]

On Wed, Jan 1, 2020 at 3:12 AM Darrick J. Wong <darrick.wong@oracle.com> 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>
> ---
>  fs/xfs/libxfs/xfs_dquot_buf.c  |   72 ++++++++++++++++++++++++++++++++++++++--
>  fs/xfs/libxfs/xfs_format.h     |   22 ++++++++++++
>  fs/xfs/libxfs/xfs_quota_defs.h |   11 ++++--
>  fs/xfs/scrub/quota.c           |    5 +++
>  fs/xfs/xfs_dquot.c             |   71 +++++++++++++++++++++++++++++++--------
>  fs/xfs/xfs_ondisk.h            |    6 +++
>  fs/xfs/xfs_qm.c                |   13 ++++---
>  fs/xfs/xfs_qm.h                |    8 ++--
>  fs/xfs/xfs_qm_syscalls.c       |   19 ++++++-----
>  9 files changed, 186 insertions(+), 41 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index 72e0fcfef580..2b5d51a6d64b 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -40,6 +40,8 @@ xfs_dquot_verify(
>         xfs_dqid_t              id,
>         uint                    type)   /* used only during quotacheck */
>  {
> +       uint8_t                 dtype;
> +
>         /*
>          * We can encounter an uninitialized dquot buffer for 2 reasons:
>          * 1. If we crash while deleting the quotainode(s), and those blks got
> @@ -60,11 +62,22 @@ xfs_dquot_verify(
>         if (ddq->d_version != XFS_DQUOT_VERSION)
>                 return __this_address;
>
> -       if (type && ddq->d_flags != type)
> +       dtype = ddq->d_flags & XFS_DQ_ALLTYPES;

Suggest dtype = XFS_DQ_TYPE(ddq->d_flags);

[...]

> @@ -540,13 +551,28 @@ xfs_dquot_from_disk(
>         dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
>         dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
>
> -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_btimer);
> +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_btimer);
>         dqp->q_btimer = tv.tv_sec;
> -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_itimer);
> +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_itimer);
>         dqp->q_itimer = tv.tv_sec;
> -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_rtbtimer);
> +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_rtbtimer);
>         dqp->q_rtbtimer = tv.tv_sec;
>
> +       /* Upgrade to bigtime if possible. */
> +       if (xfs_dquot_add_bigtime(dqp->q_mount, iddq)) {
> +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_btimer);
> +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_btimer, &tv);
> +               dqp->q_btimer = tv.tv_sec;
> +
> +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_itimer);
> +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_itimer, &tv);
> +               dqp->q_itimer = tv.tv_sec;
> +
> +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_rtbtimer);
> +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_rtbtimer, &tv);
> +               dqp->q_rtbtimer = tv.tv_sec;
> +       }
> +

This is better than the inode timestamp conversion because at
least the bigtime flag incore is always consistent with the incore values.
But I think it would be safer if the conversion happened inside the helper.

Thanks,
Amir.

Re: [PATCH 13/14] xfs: enable bigtime for quota timers

[Darrick J. Wong]

On Sun, May 31, 2020 at 08:07:59PM +0300, Amir Goldstein wrote:
> On Wed, Jan 1, 2020 at 3:12 AM Darrick J. Wong <darrick.wong@oracle.com> 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>
> > ---
> >  fs/xfs/libxfs/xfs_dquot_buf.c  |   72 ++++++++++++++++++++++++++++++++++++++--
> >  fs/xfs/libxfs/xfs_format.h     |   22 ++++++++++++
> >  fs/xfs/libxfs/xfs_quota_defs.h |   11 ++++--
> >  fs/xfs/scrub/quota.c           |    5 +++
> >  fs/xfs/xfs_dquot.c             |   71 +++++++++++++++++++++++++++++++--------
> >  fs/xfs/xfs_ondisk.h            |    6 +++
> >  fs/xfs/xfs_qm.c                |   13 ++++---
> >  fs/xfs/xfs_qm.h                |    8 ++--
> >  fs/xfs/xfs_qm_syscalls.c       |   19 ++++++-----
> >  9 files changed, 186 insertions(+), 41 deletions(-)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> > index 72e0fcfef580..2b5d51a6d64b 100644
> > --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> > +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> > @@ -40,6 +40,8 @@ xfs_dquot_verify(
> >         xfs_dqid_t              id,
> >         uint                    type)   /* used only during quotacheck */
> >  {
> > +       uint8_t                 dtype;
> > +
> >         /*
> >          * We can encounter an uninitialized dquot buffer for 2 reasons:
> >          * 1. If we crash while deleting the quotainode(s), and those blks got
> > @@ -60,11 +62,22 @@ xfs_dquot_verify(
> >         if (ddq->d_version != XFS_DQUOT_VERSION)
> >                 return __this_address;
> >
> > -       if (type && ddq->d_flags != type)
> > +       dtype = ddq->d_flags & XFS_DQ_ALLTYPES;
> 
> Suggest dtype = XFS_DQ_TYPE(ddq->d_flags);
> 
> [...]
> 
> > @@ -540,13 +551,28 @@ xfs_dquot_from_disk(
> >         dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
> >         dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
> >
> > -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_btimer);
> > +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_btimer);
> >         dqp->q_btimer = tv.tv_sec;
> > -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_itimer);
> > +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_itimer);
> >         dqp->q_itimer = tv.tv_sec;
> > -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_rtbtimer);
> > +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_rtbtimer);
> >         dqp->q_rtbtimer = tv.tv_sec;
> >
> > +       /* Upgrade to bigtime if possible. */
> > +       if (xfs_dquot_add_bigtime(dqp->q_mount, iddq)) {
> > +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_btimer);
> > +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_btimer, &tv);
> > +               dqp->q_btimer = tv.tv_sec;
> > +
> > +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_itimer);
> > +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_itimer, &tv);
> > +               dqp->q_itimer = tv.tv_sec;
> > +
> > +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_rtbtimer);
> > +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_rtbtimer, &tv);
> > +               dqp->q_rtbtimer = tv.tv_sec;
> > +       }
> > +
> 
> This is better than the inode timestamp conversion because at
> least the bigtime flag incore is always consistent with the incore values.
> But I think it would be safer if the conversion happened inside the helper.

This code, like the inode timestamp handling code, reads the ondisk
timer value into the incore dquot, and sets the bigtime flag on the
incore dquot.  We then wait until someone dirties the dquot to transform
the ondisk dquot to be in bigtime format with the flag set.

The dquot conversion is /much/ messier than the inode, because the dquot
infrastructure maintains an incore copy of the ondisk dquot as well as
an incore time64_t value, and we have to keep that whole mess
consistent.

TBH someone should probably go fix the incore dquots to have only the
incore values (instead of the embedded xfs_disk_dquot) but I'd rather
that go in as a cleanup after adding this series, because this series is
already 13 patches, and if you combine cleaning up dquots with hch's
incore inode removal now we have Yet Another Friggin 40-patch series.

--D

> Thanks,
> Amir.

Re: [PATCH 13/14] xfs: enable bigtime for quota timers

[Amir Goldstein]

On Tue, Jun 2, 2020 at 3:09 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Sun, May 31, 2020 at 08:07:59PM +0300, Amir Goldstein wrote:
> > On Wed, Jan 1, 2020 at 3:12 AM Darrick J. Wong <darrick.wong@oracle.com> 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>
> > > ---
> > >  fs/xfs/libxfs/xfs_dquot_buf.c  |   72 ++++++++++++++++++++++++++++++++++++++--
> > >  fs/xfs/libxfs/xfs_format.h     |   22 ++++++++++++
> > >  fs/xfs/libxfs/xfs_quota_defs.h |   11 ++++--
> > >  fs/xfs/scrub/quota.c           |    5 +++
> > >  fs/xfs/xfs_dquot.c             |   71 +++++++++++++++++++++++++++++++--------
> > >  fs/xfs/xfs_ondisk.h            |    6 +++
> > >  fs/xfs/xfs_qm.c                |   13 ++++---
> > >  fs/xfs/xfs_qm.h                |    8 ++--
> > >  fs/xfs/xfs_qm_syscalls.c       |   19 ++++++-----
> > >  9 files changed, 186 insertions(+), 41 deletions(-)
> > >
> > >
> > > diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> > > index 72e0fcfef580..2b5d51a6d64b 100644
> > > --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> > > +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> > > @@ -40,6 +40,8 @@ xfs_dquot_verify(
> > >         xfs_dqid_t              id,
> > >         uint                    type)   /* used only during quotacheck */
> > >  {
> > > +       uint8_t                 dtype;
> > > +
> > >         /*
> > >          * We can encounter an uninitialized dquot buffer for 2 reasons:
> > >          * 1. If we crash while deleting the quotainode(s), and those blks got
> > > @@ -60,11 +62,22 @@ xfs_dquot_verify(
> > >         if (ddq->d_version != XFS_DQUOT_VERSION)
> > >                 return __this_address;
> > >
> > > -       if (type && ddq->d_flags != type)
> > > +       dtype = ddq->d_flags & XFS_DQ_ALLTYPES;
> >
> > Suggest dtype = XFS_DQ_TYPE(ddq->d_flags);
> >
> > [...]
> >
> > > @@ -540,13 +551,28 @@ xfs_dquot_from_disk(
> > >         dqp->q_res_icount = be64_to_cpu(ddqp->d_icount);
> > >         dqp->q_res_rtbcount = be64_to_cpu(ddqp->d_rtbcount);
> > >
> > > -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_btimer);
> > > +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_btimer);
> > >         dqp->q_btimer = tv.tv_sec;
> > > -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_itimer);
> > > +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_itimer);
> > >         dqp->q_itimer = tv.tv_sec;
> > > -       xfs_dquot_from_disk_timestamp(&tv, ddqp->d_rtbtimer);
> > > +       xfs_dquot_from_disk_timestamp(ddqp, &tv, ddqp->d_rtbtimer);
> > >         dqp->q_rtbtimer = tv.tv_sec;
> > >
> > > +       /* Upgrade to bigtime if possible. */
> > > +       if (xfs_dquot_add_bigtime(dqp->q_mount, iddq)) {
> > > +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_btimer);
> > > +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_btimer, &tv);
> > > +               dqp->q_btimer = tv.tv_sec;
> > > +
> > > +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_itimer);
> > > +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_itimer, &tv);
> > > +               dqp->q_itimer = tv.tv_sec;
> > > +
> > > +               tv.tv_sec = xfs_dquot_clamp_timer(iddq, dqp->q_rtbtimer);
> > > +               xfs_dquot_to_disk_timestamp(iddq, &iddq->d_rtbtimer, &tv);
> > > +               dqp->q_rtbtimer = tv.tv_sec;
> > > +       }
> > > +
> >
> > This is better than the inode timestamp conversion because at
> > least the bigtime flag incore is always consistent with the incore values.
> > But I think it would be safer if the conversion happened inside the helper.
>
> This code, like the inode timestamp handling code, reads the ondisk
> timer value into the incore dquot, and sets the bigtime flag on the
> incore dquot.  We then wait until someone dirties the dquot to transform
> the ondisk dquot to be in bigtime format with the flag set.
>
> The dquot conversion is /much/ messier than the inode, because the dquot
> infrastructure maintains an incore copy of the ondisk dquot as well as
> an incore time64_t value, and we have to keep that whole mess
> consistent.
>

I see. That's what got me confused about how the inode timestamp incore
values are stored.

> TBH someone should probably go fix the incore dquots to have only the
> incore values (instead of the embedded xfs_disk_dquot) but I'd rather
> that go in as a cleanup after adding this series, because this series is
> already 13 patches, and if you combine cleaning up dquots with hch's
> incore inode removal now we have Yet Another Friggin 40-patch series.
>

Fare enough, but until someone does go and fix this code, my comment
still stands.

This patch calls xfs_dquot_add_bigtime() in three call sites.

Two of them are correct because the values of timers is 0.
The one above is correct because it shifts the embedded timer
values right after the call.

IMO it would be a better practice to keep this conversion
containers in a helper xfs_dquot_upgrade_bigtime() and
explicitly check for the 0 value case.

Thanks,
Amir.

[PATCH 14/14] 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>
---
 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 bab0b23720bb..f2f4f07b9357 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 00/14] xfs: widen timestamps to deal with y2038

[Amir Goldstein]

On Wed, Jan 1, 2020 at 3:11 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> 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 refactors the quota
> grace period expiration timer code a bit before implementing bit
> shifting to widen the effective counter size to 34 bits.  This enables
> correct time handling on XFS through the year 2486.
>

I intend to start review of this series.
I intend to focus on correctness of conversions and on backward
forward compatibility.

I saw Eric's comments on quota patches and that you mentioned
you addressed some of them. I do not intend to review correctness
of existing quota code anyway ;-)

I see that you updated the branch 3 days ago and that patch 2/14
was dropped. I assume the rest of the series is mostly unchanged,
but I can verify that before reviewing each patch.

As far as you are concerned, should I wait for v2 or can I continue
to review this series?

Thanks,
Amir.



> 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

Re: [PATCH 00/14] xfs: widen timestamps to deal with y2038

[Darrick J. Wong]

On Tue, May 26, 2020 at 12:20:59PM +0300, Amir Goldstein wrote:
> On Wed, Jan 1, 2020 at 3:11 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> > 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 refactors the quota
> > grace period expiration timer code a bit before implementing bit
> > shifting to widen the effective counter size to 34 bits.  This enables
> > correct time handling on XFS through the year 2486.
> >
> 
> I intend to start review of this series.
> I intend to focus on correctness of conversions and on backward
> forward compatibility.
> 
> I saw Eric's comments on quota patches and that you mentioned
> you addressed some of them. I do not intend to review correctness
> of existing quota code anyway ;-)
> 
> I see that you updated the branch 3 days ago and that patch 2/14
> was dropped. I assume the rest of the series is mostly unchanged,
> but I can verify that before reviewing each patch.
> 
> As far as you are concerned, should I wait for v2 or can I continue
> to review this series?

I plan to rebase the whole series after 5.8-rc1, but if you'd like to
look at the higher level details (particularly in the quota code, which
is a bit murky) sooner than later, I don't mind emailing out what I have
now.

--D

> Thanks,
> Amir.
> 
> 
> 
> > 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

Re: [PATCH 00/14] xfs: widen timestamps to deal with y2038

[Amir Goldstein]

On Tue, May 26, 2020 at 6:59 PM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Tue, May 26, 2020 at 12:20:59PM +0300, Amir Goldstein wrote:
> > On Wed, Jan 1, 2020 at 3:11 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > >
> > > 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 refactors the quota
> > > grace period expiration timer code a bit before implementing bit
> > > shifting to widen the effective counter size to 34 bits.  This enables
> > > correct time handling on XFS through the year 2486.
> > >
> >
> > I intend to start review of this series.
> > I intend to focus on correctness of conversions and on backward
> > forward compatibility.
> >
> > I saw Eric's comments on quota patches and that you mentioned
> > you addressed some of them. I do not intend to review correctness
> > of existing quota code anyway ;-)
> >
> > I see that you updated the branch 3 days ago and that patch 2/14
> > was dropped. I assume the rest of the series is mostly unchanged,
> > but I can verify that before reviewing each patch.
> >
> > As far as you are concerned, should I wait for v2 or can I continue
> > to review this series?
>
> I plan to rebase the whole series after 5.8-rc1, but if you'd like to
> look at the higher level details (particularly in the quota code, which
> is a bit murky) sooner than later, I don't mind emailing out what I have
> now.
>

No need. I can look at high level details on your branch.
Will hold off review comments until rebase unless I find something
that calls for your attention.

Thanks,
Amir.

Re: [PATCH 00/14] xfs: widen timestamps to deal with y2038

[Amir Goldstein]

> > I plan to rebase the whole series after 5.8-rc1, but if you'd like to
> > look at the higher level details (particularly in the quota code, which
> > is a bit murky) sooner than later, I don't mind emailing out what I have
> > now.
> >
>
> No need. I can look at high level details on your branch.
> Will hold off review comments until rebase unless I find something
> that calls for your attention.
>

I went over the code in:
a7091aa0d632 xfs: enable big timestamps

I did not find any correctness issues.
I commented on what seems to me like a trap for future bugs
with how the incore timestamps are converted.

So besides "widen ondisk timestamps" and "enable bigtime for quota timers",
feel free to add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

For patches that do not change on rebase.

Thanks,
Amir.

Re: [PATCH 00/14] xfs: widen timestamps to deal with y2038

[Darrick J. Wong]

On Sun, May 31, 2020 at 08:31:54PM +0300, Amir Goldstein wrote:
> > > I plan to rebase the whole series after 5.8-rc1, but if you'd like to
> > > look at the higher level details (particularly in the quota code, which
> > > is a bit murky) sooner than later, I don't mind emailing out what I have
> > > now.
> > >
> >
> > No need. I can look at high level details on your branch.
> > Will hold off review comments until rebase unless I find something
> > that calls for your attention.
> >
> 
> I went over the code in:
> a7091aa0d632 xfs: enable big timestamps
> 
> I did not find any correctness issues.
> I commented on what seems to me like a trap for future bugs
> with how the incore timestamps are converted.
> 
> So besides "widen ondisk timestamps" and "enable bigtime for quota timers",
> feel free to add:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> For patches that do not change on rebase.

Ok, thanks for the first round of review!  See you after -rc1. :)

--D

> Thanks,
> Amir.