[PATCH 0/2] xfs: prevent overflow of delalloc block counters

[PATCH 0/2] xfs: prevent overflow of delalloc block counters

[Darrick J. Wong]

Hi all,

This series corrects an integer overflow problem in the inode
i_delayed_blks counter that tracks the number of blocks assigned to the
in-core inode but not reflected in the on-disk metadata.

The first patch widens the in-core dquot to handle block reservations
counts that are 64 bits in size.  This is a requirement for the second
patch, which widens i_delayed_blks to 64 bits.

This rework is required to handle large amounts of COW staging blocks.
Right now the only way to trigger it is to set the maximum cow extent
size hint, reflink a very large (> 16T on a 4k block fs) sparse file,
and then touch enough blocks to cause the cow reservations to exceed
2^32 blocks.

This happens because i_delayed_blks tracks both actual delalloc
reservations in data and cow forks as well as allocated but not yet
remapped real extents in the cow fork.  The overflow will become more
practical when the atomic writes patch lands, whenever that is.

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=widen-idelayed

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

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=widen-idelayed

[PATCH 1/2] xfs: widen quota block counters to 64-bit integers

[Darrick J. Wong]

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

Widen the incore quota transaction delta structure to treat block
counters as 64-bit integers.  This is a necessary addition so that we
can widen the i_delayed_blks counter to be a 64-bit integer.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_qm.h          |    8 ++------
 fs/xfs/xfs_quota.h       |   31 +++++++++++++++++--------------
 fs/xfs/xfs_trans_dquot.c |   28 ++++++++++++++--------------
 3 files changed, 33 insertions(+), 34 deletions(-)


diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 3ccf0fbc9071..b41b75089548 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -113,12 +113,8 @@ xfs_quota_inode(xfs_mount_t *mp, uint dq_flags)
 	return NULL;
 }
 
-extern void	xfs_trans_mod_dquot(struct xfs_trans *,
-					struct xfs_dquot *, uint, long);
-extern int	xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
-			struct xfs_mount *, struct xfs_dquot *,
-			struct xfs_dquot *, struct xfs_dquot *,
-			long, long, uint);
+extern void	xfs_trans_mod_dquot(struct xfs_trans *tp, struct xfs_dquot *dqp,
+				    uint field, int64_t delta);
 extern void	xfs_trans_dqjoin(struct xfs_trans *, struct xfs_dquot *);
 extern void	xfs_trans_log_dquot(struct xfs_trans *, struct xfs_dquot *);
 
diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
index 55b798265ef7..215cb39e3a04 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -58,30 +58,33 @@ xfs_quota_chkd_flag(
  */
 typedef struct xfs_dqtrx {
 	struct xfs_dquot *qt_dquot;	  /* the dquot this refers to */
-	ulong		qt_blk_res;	  /* blks reserved on a dquot */
-	ulong		qt_ino_res;	  /* inode reserved on a dquot */
-	ulong		qt_ino_res_used;  /* inodes used from the reservation */
-	long		qt_bcount_delta;  /* dquot blk count changes */
-	long		qt_delbcnt_delta; /* delayed dquot blk count changes */
+
+	uint64_t	qt_blk_res;	  /* blks reserved on a dquot */
+	int64_t		qt_bcount_delta;  /* dquot blk count changes */
+	int64_t		qt_delbcnt_delta; /* delayed dquot blk count changes */
+
+	uint64_t	qt_rtblk_res;	  /* # blks reserved on a dquot */
+	uint64_t	qt_rtblk_res_used;/* # blks used from reservation */
+	int64_t		qt_rtbcount_delta;/* dquot realtime blk changes */
+	int64_t		qt_delrtb_delta;  /* delayed RT blk count changes */
+
+	unsigned long	qt_ino_res;	  /* inode reserved on a dquot */
+	unsigned long	qt_ino_res_used;  /* inodes used from the reservation */
 	long		qt_icount_delta;  /* dquot inode count changes */
-	ulong		qt_rtblk_res;	  /* # blks reserved on a dquot */
-	ulong		qt_rtblk_res_used;/* # blks used from reservation */
-	long		qt_rtbcount_delta;/* dquot realtime blk changes */
-	long		qt_delrtb_delta;  /* delayed RT blk count changes */
 } xfs_dqtrx_t;
 
 #ifdef CONFIG_XFS_QUOTA
 extern void xfs_trans_dup_dqinfo(struct xfs_trans *, struct xfs_trans *);
 extern void xfs_trans_free_dqinfo(struct xfs_trans *);
 extern void xfs_trans_mod_dquot_byino(struct xfs_trans *, struct xfs_inode *,
-		uint, long);
+		uint, int64_t);
 extern void xfs_trans_apply_dquot_deltas(struct xfs_trans *);
 extern void xfs_trans_unreserve_and_mod_dquots(struct xfs_trans *);
 extern int xfs_trans_reserve_quota_nblks(struct xfs_trans *,
-		struct xfs_inode *, long, long, uint);
+		struct xfs_inode *, int64_t, long, uint);
 extern int xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
 		struct xfs_mount *, struct xfs_dquot *,
-		struct xfs_dquot *, struct xfs_dquot *, long, long, uint);
+		struct xfs_dquot *, struct xfs_dquot *, int64_t, long, uint);
 
 extern int xfs_qm_vop_dqalloc(struct xfs_inode *, xfs_dqid_t, xfs_dqid_t,
 		prid_t, uint, struct xfs_dquot **, struct xfs_dquot **,
@@ -121,14 +124,14 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip, xfs_dqid_t uid, xfs_dqid_t gid,
 #define xfs_trans_apply_dquot_deltas(tp)
 #define xfs_trans_unreserve_and_mod_dquots(tp)
 static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *tp,
-		struct xfs_inode *ip, long nblks, long ninos, uint flags)
+		struct xfs_inode *ip, int64_t nblks, long ninos, uint flags)
 {
 	return 0;
 }
 static inline int xfs_trans_reserve_quota_bydquots(struct xfs_trans *tp,
 		struct xfs_mount *mp, struct xfs_dquot *udqp,
 		struct xfs_dquot *gdqp, struct xfs_dquot *pdqp,
-		long nblks, long nions, uint flags)
+		int64_t nblks, long nions, uint flags)
 {
 	return 0;
 }
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index c23257a26c2b..840ec456c9a8 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -80,7 +80,7 @@ xfs_trans_dup_dqinfo(
 	xfs_dqtrx_t	*oq, *nq;
 	int		i, j;
 	xfs_dqtrx_t	*oqa, *nqa;
-	ulong		blk_res_used;
+	uint64_t	blk_res_used;
 
 	if (!otp->t_dqinfo)
 		return;
@@ -137,7 +137,7 @@ xfs_trans_mod_dquot_byino(
 	xfs_trans_t	*tp,
 	xfs_inode_t	*ip,
 	uint		field,
-	long		delta)
+	int64_t		delta)
 {
 	xfs_mount_t	*mp = tp->t_mountp;
 
@@ -194,7 +194,7 @@ xfs_trans_mod_dquot(
 	xfs_trans_t	*tp,
 	xfs_dquot_t	*dqp,
 	uint		field,
-	long		delta)
+	int64_t		delta)
 {
 	xfs_dqtrx_t	*qtrx;
 
@@ -219,14 +219,14 @@ xfs_trans_mod_dquot(
 		 * regular disk blk reservation
 		 */
 	      case XFS_TRANS_DQ_RES_BLKS:
-		qtrx->qt_blk_res += (ulong)delta;
+		qtrx->qt_blk_res += delta;
 		break;
 
 		/*
 		 * inode reservation
 		 */
 	      case XFS_TRANS_DQ_RES_INOS:
-		qtrx->qt_ino_res += (ulong)delta;
+		qtrx->qt_ino_res += (unsigned long)delta;
 		break;
 
 		/*
@@ -245,7 +245,7 @@ xfs_trans_mod_dquot(
 		 */
 	      case XFS_TRANS_DQ_ICOUNT:
 		if (qtrx->qt_ino_res && delta > 0) {
-			qtrx->qt_ino_res_used += (ulong)delta;
+			qtrx->qt_ino_res_used += (unsigned long)delta;
 			ASSERT(qtrx->qt_ino_res >= qtrx->qt_ino_res_used);
 		}
 		qtrx->qt_icount_delta += delta;
@@ -255,7 +255,7 @@ xfs_trans_mod_dquot(
 		 * rtblk reservation
 		 */
 	      case XFS_TRANS_DQ_RES_RTBLKS:
-		qtrx->qt_rtblk_res += (ulong)delta;
+		qtrx->qt_rtblk_res += delta;
 		break;
 
 		/*
@@ -263,7 +263,7 @@ xfs_trans_mod_dquot(
 		 */
 	      case XFS_TRANS_DQ_RTBCOUNT:
 		if (qtrx->qt_rtblk_res && delta > 0) {
-			qtrx->qt_rtblk_res_used += (ulong)delta;
+			qtrx->qt_rtblk_res_used += delta;
 			ASSERT(qtrx->qt_rtblk_res >= qtrx->qt_rtblk_res_used);
 		}
 		qtrx->qt_rtbcount_delta += delta;
@@ -320,8 +320,8 @@ xfs_trans_apply_dquot_deltas(
 	struct xfs_dquot	*dqp;
 	struct xfs_dqtrx	*qtrx, *qa;
 	struct xfs_disk_dquot	*d;
-	long			totalbdelta;
-	long			totalrtbdelta;
+	int64_t			totalbdelta;
+	int64_t			totalrtbdelta;
 
 	if (!(tp->t_flags & XFS_TRANS_DQ_DIRTY))
 		return;
@@ -413,7 +413,7 @@ xfs_trans_apply_dquot_deltas(
 			 * reservation that a transaction structure knows of.
 			 */
 			if (qtrx->qt_blk_res != 0) {
-				ulong blk_res_used = 0;
+				uint64_t	blk_res_used = 0;
 
 				if (qtrx->qt_bcount_delta > 0)
 					blk_res_used = qtrx->qt_bcount_delta;
@@ -585,7 +585,7 @@ xfs_trans_dqresv(
 	xfs_trans_t	*tp,
 	xfs_mount_t	*mp,
 	xfs_dquot_t	*dqp,
-	long		nblks,
+	int64_t		nblks,
 	long		ninos,
 	uint		flags)
 {
@@ -745,7 +745,7 @@ xfs_trans_reserve_quota_bydquots(
 	struct xfs_dquot	*udqp,
 	struct xfs_dquot	*gdqp,
 	struct xfs_dquot	*pdqp,
-	long			nblks,
+	int64_t			nblks,
 	long			ninos,
 	uint			flags)
 {
@@ -804,7 +804,7 @@ int
 xfs_trans_reserve_quota_nblks(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
-	long			nblks,
+	int64_t			nblks,
 	long			ninos,
 	uint			flags)
 {

[PATCH 2/2] xfs: widen inode delalloc block counter to 64-bits

[Darrick J. Wong]

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

Widen the incore inode's i_delayed_blks counter to be a 64-bit integer.
This is necessary to fix an integer overflow problem that can be
reproduced easily now that we use the counter to track blocks that are
assigned to the inode in memory but not on disk.  This includes actual
delalloc reservations as well as real extents in the COW fork that
are waiting to be remapped into the data fork.

These 'delayed mapping' blocks can easily exceed 2^32 blocks if one
creates a very large sparse file of size approximately 2^33 bytes with
one byte written every 2^23 bytes, sets a very large COW extent size
hint of 2^23 blocks, reflinks the first file into a second file, and
then writes a single byte every 2^23 blocks in the original file.

When this happens, we'll try to create approximately 1024 2^23 extent
reservations in the COW fork, which will overflow the counter and cause
problems.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_inode.h |    2 +-
 fs/xfs/xfs_qm.c    |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 87e701b638ae..558173f95a03 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -56,7 +56,7 @@ typedef struct xfs_inode {
 	spinlock_t		i_flags_lock;	/* inode i_flags lock */
 	/* Miscellaneous state. */
 	unsigned long		i_flags;	/* see defined flags below */
-	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
+	uint64_t		i_delayed_blks;	/* count of delay alloc blks */
 
 	struct xfs_icdinode	i_d;		/* most of ondisk inode */
 
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 52ed7904df10..aa6b6db3db0e 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1812,7 +1812,8 @@ xfs_qm_vop_chown_reserve(
 	uint			flags)
 {
 	struct xfs_mount	*mp = ip->i_mount;
-	uint			delblks, blkflags, prjflags = 0;
+	uint64_t		delblks;
+	unsigned int		blkflags, prjflags = 0;
 	struct xfs_dquot	*udq_unres = NULL;
 	struct xfs_dquot	*gdq_unres = NULL;
 	struct xfs_dquot	*pdq_unres = NULL;

Re: [PATCH 1/2] xfs: widen quota block counters to 64-bit integers

[Allison Henderson]

On 4/16/19 6:39 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Widen the incore quota transaction delta structure to treat block
> counters as 64-bit integers.  This is a necessary addition so that we
> can widen the i_delayed_blks counter to be a 64-bit integer.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>   fs/xfs/xfs_qm.h          |    8 ++------
>   fs/xfs/xfs_quota.h       |   31 +++++++++++++++++--------------
>   fs/xfs/xfs_trans_dquot.c |   28 ++++++++++++++--------------
>   3 files changed, 33 insertions(+), 34 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 3ccf0fbc9071..b41b75089548 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -113,12 +113,8 @@ xfs_quota_inode(xfs_mount_t *mp, uint dq_flags)
>   	return NULL;
>   }
>   
> -extern void	xfs_trans_mod_dquot(struct xfs_trans *,
> -					struct xfs_dquot *, uint, long);
> -extern int	xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
> -			struct xfs_mount *, struct xfs_dquot *,
> -			struct xfs_dquot *, struct xfs_dquot *,
> -			long, long, uint);
Did you mean to remove this last one?  I don't see it redefined, but I'm 
guessing maybe it wasn't needed in this scope.

Other than that it looks ok though.  You can add my review:
Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> +extern void	xfs_trans_mod_dquot(struct xfs_trans *tp, struct xfs_dquot *dqp,
> +				    uint field, int64_t delta);
>   extern void	xfs_trans_dqjoin(struct xfs_trans *, struct xfs_dquot *);
>   extern void	xfs_trans_log_dquot(struct xfs_trans *, struct xfs_dquot *);
>   
> diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
> index 55b798265ef7..215cb39e3a04 100644
> --- a/fs/xfs/xfs_quota.h
> +++ b/fs/xfs/xfs_quota.h
> @@ -58,30 +58,33 @@ xfs_quota_chkd_flag(
>    */
>   typedef struct xfs_dqtrx {
>   	struct xfs_dquot *qt_dquot;	  /* the dquot this refers to */
> -	ulong		qt_blk_res;	  /* blks reserved on a dquot */
> -	ulong		qt_ino_res;	  /* inode reserved on a dquot */
> -	ulong		qt_ino_res_used;  /* inodes used from the reservation */
> -	long		qt_bcount_delta;  /* dquot blk count changes */
> -	long		qt_delbcnt_delta; /* delayed dquot blk count changes */
> +
> +	uint64_t	qt_blk_res;	  /* blks reserved on a dquot */
> +	int64_t		qt_bcount_delta;  /* dquot blk count changes */
> +	int64_t		qt_delbcnt_delta; /* delayed dquot blk count changes */
> +
> +	uint64_t	qt_rtblk_res;	  /* # blks reserved on a dquot */
> +	uint64_t	qt_rtblk_res_used;/* # blks used from reservation */
> +	int64_t		qt_rtbcount_delta;/* dquot realtime blk changes */
> +	int64_t		qt_delrtb_delta;  /* delayed RT blk count changes */
> +
> +	unsigned long	qt_ino_res;	  /* inode reserved on a dquot */
> +	unsigned long	qt_ino_res_used;  /* inodes used from the reservation */
>   	long		qt_icount_delta;  /* dquot inode count changes */
> -	ulong		qt_rtblk_res;	  /* # blks reserved on a dquot */
> -	ulong		qt_rtblk_res_used;/* # blks used from reservation */
> -	long		qt_rtbcount_delta;/* dquot realtime blk changes */
> -	long		qt_delrtb_delta;  /* delayed RT blk count changes */
>   } xfs_dqtrx_t;
>   
>   #ifdef CONFIG_XFS_QUOTA
>   extern void xfs_trans_dup_dqinfo(struct xfs_trans *, struct xfs_trans *);
>   extern void xfs_trans_free_dqinfo(struct xfs_trans *);
>   extern void xfs_trans_mod_dquot_byino(struct xfs_trans *, struct xfs_inode *,
> -		uint, long);
> +		uint, int64_t);
>   extern void xfs_trans_apply_dquot_deltas(struct xfs_trans *);
>   extern void xfs_trans_unreserve_and_mod_dquots(struct xfs_trans *);
>   extern int xfs_trans_reserve_quota_nblks(struct xfs_trans *,
> -		struct xfs_inode *, long, long, uint);
> +		struct xfs_inode *, int64_t, long, uint);
>   extern int xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
>   		struct xfs_mount *, struct xfs_dquot *,
> -		struct xfs_dquot *, struct xfs_dquot *, long, long, uint);
> +		struct xfs_dquot *, struct xfs_dquot *, int64_t, long, uint);
>   
>   extern int xfs_qm_vop_dqalloc(struct xfs_inode *, xfs_dqid_t, xfs_dqid_t,
>   		prid_t, uint, struct xfs_dquot **, struct xfs_dquot **,
> @@ -121,14 +124,14 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip, xfs_dqid_t uid, xfs_dqid_t gid,
>   #define xfs_trans_apply_dquot_deltas(tp)
>   #define xfs_trans_unreserve_and_mod_dquots(tp)
>   static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *tp,
> -		struct xfs_inode *ip, long nblks, long ninos, uint flags)
> +		struct xfs_inode *ip, int64_t nblks, long ninos, uint flags)
>   {
>   	return 0;
>   }
>   static inline int xfs_trans_reserve_quota_bydquots(struct xfs_trans *tp,
>   		struct xfs_mount *mp, struct xfs_dquot *udqp,
>   		struct xfs_dquot *gdqp, struct xfs_dquot *pdqp,
> -		long nblks, long nions, uint flags)
> +		int64_t nblks, long nions, uint flags)
>   {
>   	return 0;
>   }
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index c23257a26c2b..840ec456c9a8 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -80,7 +80,7 @@ xfs_trans_dup_dqinfo(
>   	xfs_dqtrx_t	*oq, *nq;
>   	int		i, j;
>   	xfs_dqtrx_t	*oqa, *nqa;
> -	ulong		blk_res_used;
> +	uint64_t	blk_res_used;
>   
>   	if (!otp->t_dqinfo)
>   		return;
> @@ -137,7 +137,7 @@ xfs_trans_mod_dquot_byino(
>   	xfs_trans_t	*tp,
>   	xfs_inode_t	*ip,
>   	uint		field,
> -	long		delta)
> +	int64_t		delta)
>   {
>   	xfs_mount_t	*mp = tp->t_mountp;
>   
> @@ -194,7 +194,7 @@ xfs_trans_mod_dquot(
>   	xfs_trans_t	*tp,
>   	xfs_dquot_t	*dqp,
>   	uint		field,
> -	long		delta)
> +	int64_t		delta)
>   {
>   	xfs_dqtrx_t	*qtrx;
>   
> @@ -219,14 +219,14 @@ xfs_trans_mod_dquot(
>   		 * regular disk blk reservation
>   		 */
>   	      case XFS_TRANS_DQ_RES_BLKS:
> -		qtrx->qt_blk_res += (ulong)delta;
> +		qtrx->qt_blk_res += delta;
>   		break;
>   
>   		/*
>   		 * inode reservation
>   		 */
>   	      case XFS_TRANS_DQ_RES_INOS:
> -		qtrx->qt_ino_res += (ulong)delta;
> +		qtrx->qt_ino_res += (unsigned long)delta;
>   		break;
>   
>   		/*
> @@ -245,7 +245,7 @@ xfs_trans_mod_dquot(
>   		 */
>   	      case XFS_TRANS_DQ_ICOUNT:
>   		if (qtrx->qt_ino_res && delta > 0) {
> -			qtrx->qt_ino_res_used += (ulong)delta;
> +			qtrx->qt_ino_res_used += (unsigned long)delta;
>   			ASSERT(qtrx->qt_ino_res >= qtrx->qt_ino_res_used);
>   		}
>   		qtrx->qt_icount_delta += delta;
> @@ -255,7 +255,7 @@ xfs_trans_mod_dquot(
>   		 * rtblk reservation
>   		 */
>   	      case XFS_TRANS_DQ_RES_RTBLKS:
> -		qtrx->qt_rtblk_res += (ulong)delta;
> +		qtrx->qt_rtblk_res += delta;
>   		break;
>   
>   		/*
> @@ -263,7 +263,7 @@ xfs_trans_mod_dquot(
>   		 */
>   	      case XFS_TRANS_DQ_RTBCOUNT:
>   		if (qtrx->qt_rtblk_res && delta > 0) {
> -			qtrx->qt_rtblk_res_used += (ulong)delta;
> +			qtrx->qt_rtblk_res_used += delta;
>   			ASSERT(qtrx->qt_rtblk_res >= qtrx->qt_rtblk_res_used);
>   		}
>   		qtrx->qt_rtbcount_delta += delta;
> @@ -320,8 +320,8 @@ xfs_trans_apply_dquot_deltas(
>   	struct xfs_dquot	*dqp;
>   	struct xfs_dqtrx	*qtrx, *qa;
>   	struct xfs_disk_dquot	*d;
> -	long			totalbdelta;
> -	long			totalrtbdelta;
> +	int64_t			totalbdelta;
> +	int64_t			totalrtbdelta;
>   
>   	if (!(tp->t_flags & XFS_TRANS_DQ_DIRTY))
>   		return;
> @@ -413,7 +413,7 @@ xfs_trans_apply_dquot_deltas(
>   			 * reservation that a transaction structure knows of.
>   			 */
>   			if (qtrx->qt_blk_res != 0) {
> -				ulong blk_res_used = 0;
> +				uint64_t	blk_res_used = 0;
>   
>   				if (qtrx->qt_bcount_delta > 0)
>   					blk_res_used = qtrx->qt_bcount_delta;
> @@ -585,7 +585,7 @@ xfs_trans_dqresv(
>   	xfs_trans_t	*tp,
>   	xfs_mount_t	*mp,
>   	xfs_dquot_t	*dqp,
> -	long		nblks,
> +	int64_t		nblks,
>   	long		ninos,
>   	uint		flags)
>   {
> @@ -745,7 +745,7 @@ xfs_trans_reserve_quota_bydquots(
>   	struct xfs_dquot	*udqp,
>   	struct xfs_dquot	*gdqp,
>   	struct xfs_dquot	*pdqp,
> -	long			nblks,
> +	int64_t			nblks,
>   	long			ninos,
>   	uint			flags)
>   {
> @@ -804,7 +804,7 @@ int
>   xfs_trans_reserve_quota_nblks(
>   	struct xfs_trans	*tp,
>   	struct xfs_inode	*ip,
> -	long			nblks,
> +	int64_t			nblks,
>   	long			ninos,
>   	uint			flags)
>   {
> 

Re: [PATCH 2/2] xfs: widen inode delalloc block counter to 64-bits

[Allison Henderson]

Alrighty, looks good.

Reviewed-by: Allison Collins <allison.henderson@oracle.com>

On 4/16/19 6:39 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Widen the incore inode's i_delayed_blks counter to be a 64-bit integer.
> This is necessary to fix an integer overflow problem that can be
> reproduced easily now that we use the counter to track blocks that are
> assigned to the inode in memory but not on disk.  This includes actual
> delalloc reservations as well as real extents in the COW fork that
> are waiting to be remapped into the data fork.
> 
> These 'delayed mapping' blocks can easily exceed 2^32 blocks if one
> creates a very large sparse file of size approximately 2^33 bytes with
> one byte written every 2^23 bytes, sets a very large COW extent size
> hint of 2^23 blocks, reflinks the first file into a second file, and
> then writes a single byte every 2^23 blocks in the original file.
> 
> When this happens, we'll try to create approximately 1024 2^23 extent
> reservations in the COW fork, which will overflow the counter and cause
> problems.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>   fs/xfs/xfs_inode.h |    2 +-
>   fs/xfs/xfs_qm.c    |    3 ++-
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 87e701b638ae..558173f95a03 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -56,7 +56,7 @@ typedef struct xfs_inode {
>   	spinlock_t		i_flags_lock;	/* inode i_flags lock */
>   	/* Miscellaneous state. */
>   	unsigned long		i_flags;	/* see defined flags below */
> -	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
> +	uint64_t		i_delayed_blks;	/* count of delay alloc blks */
>   
>   	struct xfs_icdinode	i_d;		/* most of ondisk inode */
>   
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 52ed7904df10..aa6b6db3db0e 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1812,7 +1812,8 @@ xfs_qm_vop_chown_reserve(
>   	uint			flags)
>   {
>   	struct xfs_mount	*mp = ip->i_mount;
> -	uint			delblks, blkflags, prjflags = 0;
> +	uint64_t		delblks;
> +	unsigned int		blkflags, prjflags = 0;
>   	struct xfs_dquot	*udq_unres = NULL;
>   	struct xfs_dquot	*gdq_unres = NULL;
>   	struct xfs_dquot	*pdq_unres = NULL;
> 

[PATCH 3/2] xfstests: check for COW overflows in i_delayed_blks

[Darrick J. Wong]

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

With the new copy on write functionality it's possible to reserve so
much COW space for a file that we end up overflowing i_delayed_blks.
The only user-visible effect of this is to cause totally wrong i_blocks
output in stat, so check for that.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/907     |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/907.out |    7 +++
 tests/xfs/group   |    1 
 3 files changed, 136 insertions(+)
 create mode 100755 tests/xfs/907
 create mode 100644 tests/xfs/907.out

diff --git a/tests/xfs/907 b/tests/xfs/907
new file mode 100755
index 00000000..5791f835
--- /dev/null
+++ b/tests/xfs/907
@@ -0,0 +1,128 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2019 Oracle, Inc.  All Rights Reserved.
+#
+# FS QA Test No. 907
+#
+# Try to overflow i_delayed_blks by setting the largest cowextsize hint
+# possible, creating a sparse file with a single byte every cowextsize bytes,
+# reflinking it, and retouching every written byte to see if we can create
+# enough speculative COW reservations to overflow i_delayed_blks.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 7 15
+
+_cleanup()
+{
+	cd /
+	umount $loop_mount > /dev/null 2>&1
+	rm -rf $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/reflink
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs xfs
+_require_scratch_reflink
+_require_loop
+_require_xfs_debug
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+_require_fs_space $SCRATCH_MNT 2400000	# 100T fs requires ~2.4GB of space
+
+loop_file=$SCRATCH_MNT/a.img
+loop_mount=$SCRATCH_MNT/a
+truncate -s 100T $loop_file
+$MKFS_XFS_PROG $MKFS_OPTIONS -f $loop_file >> $seqres.full
+mkdir $loop_mount
+mount -o loop -t xfs $loop_file $loop_mount
+
+echo "Create crazy huge file"
+touch "${loop_mount}/a"
+blksz="$(stat -f -c '%S' "${loop_mount}")"
+MAXEXTLEN=2097151	# cowextsize can't be more than MAXEXTLEN
+extsize="$(( ((2 ** 32) - 1) / blksz ))"
+test "${extsize}" -gt "${MAXEXTLEN}" && extsize="${MAXEXTLEN}"
+extsize_bytes="$(( extsize * blksz ))"
+
+# Set the largest cowextsize we can
+$XFS_IO_PROG -c "cowextsize ${extsize_bytes}" "${loop_mount}/a"
+set_cowextsize="$($XFS_IO_PROG -c 'cowextsize' "${loop_mount}/a" | sed -e 's/^.\([0-9]*\).*$/\1/g')"
+test "${set_cowextsize}" -eq 0 && _fail "could not set cowextsize?"
+
+statB="$(stat -c '%B' "${loop_mount}/a")"
+
+# Write a single byte every cowextsize bytes so that we minimize the space
+# required to create maximally sized cow reservations
+nr="$(( ((2 ** 32) / extsize) + 100 ))"
+seq 0 "${nr}" | tac | while read n; do
+	off="$((n * extsize * blksz))"
+	$XFS_IO_PROG -c "pwrite ${off} 1" "${loop_mount}/a" > /dev/null
+done
+
+echo "Reflink crazy huge file"
+cp --reflink=always "${loop_mount}/a" "${loop_mount}/b"
+
+echo "COW crazy huge file"
+# Try to create enough maximally sized cow reservations to overflow
+# i_delayed_blks
+seq 0 "${nr}" | tac | while read n; do
+	off="$((n * extsize * blksz))"
+	$XFS_IO_PROG -c "pwrite ${off} 1" "${loop_mount}/a" > /dev/null
+done
+
+echo "Check crazy huge file"
+blocks="$(stat -c '%b' "${loop_mount}/a")"
+fsblocks="$((blocks * statB / blksz))"
+
+# Make sure we got enough COW reservations to overflow a 32-bit counter.
+$XFS_IO_PROG -c 'bmap -clpv' "${loop_mount}/a" > $tmp.extents
+echo "COW EXTENT STATE" >> $seqres.full
+cat $tmp.extents >> $seqres.full
+cat > $tmp.awk << ENDL
+{
+	if (\$3 == "delalloc") {
+		x += \$4;
+	} else if (\$3 == "hole") {
+		;
+	} else {
+		x += \$6;
+	}
+}
+END {
+	printf("%d\\n", x / ($blksz / 512));
+}
+ENDL
+cat $tmp.awk >> $seqres.full
+cowblocks="$(awk -f $tmp.awk $tmp.extents)"
+echo "cowblocks is ${cowblocks}" >> $seqres.full
+if [ "${cowblocks}" -lt "$((2 ** 32))" ]; then
+	echo "cowblocks (${cowblocks}) should be more than 2^32!"
+fi
+
+# And finally, see if i_delayed_blks overflowed.
+echo "stat blocks is ${fsblocks}" >> $seqres.full
+if [ "${fsblocks}" -lt "$((2 ** 32))" ]; then
+	echo "stat blocks (${fsblocks}) should be more than 2^32!"
+	if [ "${cowblocks}" -lt "$((2 ** 32))" ]; then
+		echo "cowblocks (${cowblocks}) is more than 2^32, your system has overflowed!!!"
+	fi
+fi
+
+echo "Test done"
+umount $loop_mount
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/907.out b/tests/xfs/907.out
new file mode 100644
index 00000000..9778d5ed
--- /dev/null
+++ b/tests/xfs/907.out
@@ -0,0 +1,7 @@
+QA output created by 907
+Format and mount
+Create crazy huge file
+Reflink crazy huge file
+COW crazy huge file
+Check crazy huge file
+Test done
diff --git a/tests/xfs/group b/tests/xfs/group
index 5a4ef4bf..e0c7fc97 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -504,3 +504,4 @@
 739 auto quick mkfs label
 742 auto quick spaceman
 743 auto quick health
+907 clone

Re: [PATCH 1/2] xfs: widen quota block counters to 64-bit integers

[Darrick J. Wong]

On Tue, Apr 16, 2019 at 08:06:20PM -0700, Allison Henderson wrote:
> On 4/16/19 6:39 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Widen the incore quota transaction delta structure to treat block
> > counters as 64-bit integers.  This is a necessary addition so that we
> > can widen the i_delayed_blks counter to be a 64-bit integer.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >   fs/xfs/xfs_qm.h          |    8 ++------
> >   fs/xfs/xfs_quota.h       |   31 +++++++++++++++++--------------
> >   fs/xfs/xfs_trans_dquot.c |   28 ++++++++++++++--------------
> >   3 files changed, 33 insertions(+), 34 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> > index 3ccf0fbc9071..b41b75089548 100644
> > --- a/fs/xfs/xfs_qm.h
> > +++ b/fs/xfs/xfs_qm.h
> > @@ -113,12 +113,8 @@ xfs_quota_inode(xfs_mount_t *mp, uint dq_flags)
> >   	return NULL;
> >   }
> > -extern void	xfs_trans_mod_dquot(struct xfs_trans *,
> > -					struct xfs_dquot *, uint, long);
> > -extern int	xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
> > -			struct xfs_mount *, struct xfs_dquot *,
> > -			struct xfs_dquot *, struct xfs_dquot *,
> > -			long, long, uint);
> Did you mean to remove this last one?  I don't see it redefined, but I'm
> guessing maybe it wasn't needed in this scope.

Nah, it was a duplicate function declaration.

--D

> Other than that it looks ok though.  You can add my review:
> Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> 
> > +extern void	xfs_trans_mod_dquot(struct xfs_trans *tp, struct xfs_dquot *dqp,
> > +				    uint field, int64_t delta);
> >   extern void	xfs_trans_dqjoin(struct xfs_trans *, struct xfs_dquot *);
> >   extern void	xfs_trans_log_dquot(struct xfs_trans *, struct xfs_dquot *);
> > diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
> > index 55b798265ef7..215cb39e3a04 100644
> > --- a/fs/xfs/xfs_quota.h
> > +++ b/fs/xfs/xfs_quota.h
> > @@ -58,30 +58,33 @@ xfs_quota_chkd_flag(
> >    */
> >   typedef struct xfs_dqtrx {
> >   	struct xfs_dquot *qt_dquot;	  /* the dquot this refers to */
> > -	ulong		qt_blk_res;	  /* blks reserved on a dquot */
> > -	ulong		qt_ino_res;	  /* inode reserved on a dquot */
> > -	ulong		qt_ino_res_used;  /* inodes used from the reservation */
> > -	long		qt_bcount_delta;  /* dquot blk count changes */
> > -	long		qt_delbcnt_delta; /* delayed dquot blk count changes */
> > +
> > +	uint64_t	qt_blk_res;	  /* blks reserved on a dquot */
> > +	int64_t		qt_bcount_delta;  /* dquot blk count changes */
> > +	int64_t		qt_delbcnt_delta; /* delayed dquot blk count changes */
> > +
> > +	uint64_t	qt_rtblk_res;	  /* # blks reserved on a dquot */
> > +	uint64_t	qt_rtblk_res_used;/* # blks used from reservation */
> > +	int64_t		qt_rtbcount_delta;/* dquot realtime blk changes */
> > +	int64_t		qt_delrtb_delta;  /* delayed RT blk count changes */
> > +
> > +	unsigned long	qt_ino_res;	  /* inode reserved on a dquot */
> > +	unsigned long	qt_ino_res_used;  /* inodes used from the reservation */
> >   	long		qt_icount_delta;  /* dquot inode count changes */
> > -	ulong		qt_rtblk_res;	  /* # blks reserved on a dquot */
> > -	ulong		qt_rtblk_res_used;/* # blks used from reservation */
> > -	long		qt_rtbcount_delta;/* dquot realtime blk changes */
> > -	long		qt_delrtb_delta;  /* delayed RT blk count changes */
> >   } xfs_dqtrx_t;
> >   #ifdef CONFIG_XFS_QUOTA
> >   extern void xfs_trans_dup_dqinfo(struct xfs_trans *, struct xfs_trans *);
> >   extern void xfs_trans_free_dqinfo(struct xfs_trans *);
> >   extern void xfs_trans_mod_dquot_byino(struct xfs_trans *, struct xfs_inode *,
> > -		uint, long);
> > +		uint, int64_t);
> >   extern void xfs_trans_apply_dquot_deltas(struct xfs_trans *);
> >   extern void xfs_trans_unreserve_and_mod_dquots(struct xfs_trans *);
> >   extern int xfs_trans_reserve_quota_nblks(struct xfs_trans *,
> > -		struct xfs_inode *, long, long, uint);
> > +		struct xfs_inode *, int64_t, long, uint);
> >   extern int xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
> >   		struct xfs_mount *, struct xfs_dquot *,
> > -		struct xfs_dquot *, struct xfs_dquot *, long, long, uint);
> > +		struct xfs_dquot *, struct xfs_dquot *, int64_t, long, uint);
> >   extern int xfs_qm_vop_dqalloc(struct xfs_inode *, xfs_dqid_t, xfs_dqid_t,
> >   		prid_t, uint, struct xfs_dquot **, struct xfs_dquot **,
> > @@ -121,14 +124,14 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip, xfs_dqid_t uid, xfs_dqid_t gid,
> >   #define xfs_trans_apply_dquot_deltas(tp)
> >   #define xfs_trans_unreserve_and_mod_dquots(tp)
> >   static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *tp,
> > -		struct xfs_inode *ip, long nblks, long ninos, uint flags)
> > +		struct xfs_inode *ip, int64_t nblks, long ninos, uint flags)
> >   {
> >   	return 0;
> >   }
> >   static inline int xfs_trans_reserve_quota_bydquots(struct xfs_trans *tp,
> >   		struct xfs_mount *mp, struct xfs_dquot *udqp,
> >   		struct xfs_dquot *gdqp, struct xfs_dquot *pdqp,
> > -		long nblks, long nions, uint flags)
> > +		int64_t nblks, long nions, uint flags)
> >   {
> >   	return 0;
> >   }
> > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> > index c23257a26c2b..840ec456c9a8 100644
> > --- a/fs/xfs/xfs_trans_dquot.c
> > +++ b/fs/xfs/xfs_trans_dquot.c
> > @@ -80,7 +80,7 @@ xfs_trans_dup_dqinfo(
> >   	xfs_dqtrx_t	*oq, *nq;
> >   	int		i, j;
> >   	xfs_dqtrx_t	*oqa, *nqa;
> > -	ulong		blk_res_used;
> > +	uint64_t	blk_res_used;
> >   	if (!otp->t_dqinfo)
> >   		return;
> > @@ -137,7 +137,7 @@ xfs_trans_mod_dquot_byino(
> >   	xfs_trans_t	*tp,
> >   	xfs_inode_t	*ip,
> >   	uint		field,
> > -	long		delta)
> > +	int64_t		delta)
> >   {
> >   	xfs_mount_t	*mp = tp->t_mountp;
> > @@ -194,7 +194,7 @@ xfs_trans_mod_dquot(
> >   	xfs_trans_t	*tp,
> >   	xfs_dquot_t	*dqp,
> >   	uint		field,
> > -	long		delta)
> > +	int64_t		delta)
> >   {
> >   	xfs_dqtrx_t	*qtrx;
> > @@ -219,14 +219,14 @@ xfs_trans_mod_dquot(
> >   		 * regular disk blk reservation
> >   		 */
> >   	      case XFS_TRANS_DQ_RES_BLKS:
> > -		qtrx->qt_blk_res += (ulong)delta;
> > +		qtrx->qt_blk_res += delta;
> >   		break;
> >   		/*
> >   		 * inode reservation
> >   		 */
> >   	      case XFS_TRANS_DQ_RES_INOS:
> > -		qtrx->qt_ino_res += (ulong)delta;
> > +		qtrx->qt_ino_res += (unsigned long)delta;
> >   		break;
> >   		/*
> > @@ -245,7 +245,7 @@ xfs_trans_mod_dquot(
> >   		 */
> >   	      case XFS_TRANS_DQ_ICOUNT:
> >   		if (qtrx->qt_ino_res && delta > 0) {
> > -			qtrx->qt_ino_res_used += (ulong)delta;
> > +			qtrx->qt_ino_res_used += (unsigned long)delta;
> >   			ASSERT(qtrx->qt_ino_res >= qtrx->qt_ino_res_used);
> >   		}
> >   		qtrx->qt_icount_delta += delta;
> > @@ -255,7 +255,7 @@ xfs_trans_mod_dquot(
> >   		 * rtblk reservation
> >   		 */
> >   	      case XFS_TRANS_DQ_RES_RTBLKS:
> > -		qtrx->qt_rtblk_res += (ulong)delta;
> > +		qtrx->qt_rtblk_res += delta;
> >   		break;
> >   		/*
> > @@ -263,7 +263,7 @@ xfs_trans_mod_dquot(
> >   		 */
> >   	      case XFS_TRANS_DQ_RTBCOUNT:
> >   		if (qtrx->qt_rtblk_res && delta > 0) {
> > -			qtrx->qt_rtblk_res_used += (ulong)delta;
> > +			qtrx->qt_rtblk_res_used += delta;
> >   			ASSERT(qtrx->qt_rtblk_res >= qtrx->qt_rtblk_res_used);
> >   		}
> >   		qtrx->qt_rtbcount_delta += delta;
> > @@ -320,8 +320,8 @@ xfs_trans_apply_dquot_deltas(
> >   	struct xfs_dquot	*dqp;
> >   	struct xfs_dqtrx	*qtrx, *qa;
> >   	struct xfs_disk_dquot	*d;
> > -	long			totalbdelta;
> > -	long			totalrtbdelta;
> > +	int64_t			totalbdelta;
> > +	int64_t			totalrtbdelta;
> >   	if (!(tp->t_flags & XFS_TRANS_DQ_DIRTY))
> >   		return;
> > @@ -413,7 +413,7 @@ xfs_trans_apply_dquot_deltas(
> >   			 * reservation that a transaction structure knows of.
> >   			 */
> >   			if (qtrx->qt_blk_res != 0) {
> > -				ulong blk_res_used = 0;
> > +				uint64_t	blk_res_used = 0;
> >   				if (qtrx->qt_bcount_delta > 0)
> >   					blk_res_used = qtrx->qt_bcount_delta;
> > @@ -585,7 +585,7 @@ xfs_trans_dqresv(
> >   	xfs_trans_t	*tp,
> >   	xfs_mount_t	*mp,
> >   	xfs_dquot_t	*dqp,
> > -	long		nblks,
> > +	int64_t		nblks,
> >   	long		ninos,
> >   	uint		flags)
> >   {
> > @@ -745,7 +745,7 @@ xfs_trans_reserve_quota_bydquots(
> >   	struct xfs_dquot	*udqp,
> >   	struct xfs_dquot	*gdqp,
> >   	struct xfs_dquot	*pdqp,
> > -	long			nblks,
> > +	int64_t			nblks,
> >   	long			ninos,
> >   	uint			flags)
> >   {
> > @@ -804,7 +804,7 @@ int
> >   xfs_trans_reserve_quota_nblks(
> >   	struct xfs_trans	*tp,
> >   	struct xfs_inode	*ip,
> > -	long			nblks,
> > +	int64_t			nblks,
> >   	long			ninos,
> >   	uint			flags)
> >   {
> > 

Re: [PATCH 1/2] xfs: widen quota block counters to 64-bit integers

[Dave Chinner]

On Tue, Apr 16, 2019 at 06:39:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Widen the incore quota transaction delta structure to treat block
> counters as 64-bit integers.  This is a necessary addition so that we
> can widen the i_delayed_blks counter to be a 64-bit integer.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_qm.h          |    8 ++------
>  fs/xfs/xfs_quota.h       |   31 +++++++++++++++++--------------
>  fs/xfs/xfs_trans_dquot.c |   28 ++++++++++++++--------------
>  3 files changed, 33 insertions(+), 34 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 3ccf0fbc9071..b41b75089548 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -113,12 +113,8 @@ xfs_quota_inode(xfs_mount_t *mp, uint dq_flags)
>  	return NULL;
>  }
>  
> -extern void	xfs_trans_mod_dquot(struct xfs_trans *,
> -					struct xfs_dquot *, uint, long);
> -extern int	xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
> -			struct xfs_mount *, struct xfs_dquot *,
> -			struct xfs_dquot *, struct xfs_dquot *,
> -			long, long, uint);
> +extern void	xfs_trans_mod_dquot(struct xfs_trans *tp, struct xfs_dquot *dqp,
> +				    uint field, int64_t delta);
>  extern void	xfs_trans_dqjoin(struct xfs_trans *, struct xfs_dquot *);
>  extern void	xfs_trans_log_dquot(struct xfs_trans *, struct xfs_dquot *);
>  
> diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
> index 55b798265ef7..215cb39e3a04 100644
> --- a/fs/xfs/xfs_quota.h
> +++ b/fs/xfs/xfs_quota.h
> @@ -58,30 +58,33 @@ xfs_quota_chkd_flag(
>   */
>  typedef struct xfs_dqtrx {
>  	struct xfs_dquot *qt_dquot;	  /* the dquot this refers to */
> -	ulong		qt_blk_res;	  /* blks reserved on a dquot */
> -	ulong		qt_ino_res;	  /* inode reserved on a dquot */
> -	ulong		qt_ino_res_used;  /* inodes used from the reservation */
> -	long		qt_bcount_delta;  /* dquot blk count changes */
> -	long		qt_delbcnt_delta; /* delayed dquot blk count changes */
> +
> +	uint64_t	qt_blk_res;	  /* blks reserved on a dquot */
> +	int64_t		qt_bcount_delta;  /* dquot blk count changes */
> +	int64_t		qt_delbcnt_delta; /* delayed dquot blk count changes */
> +
> +	uint64_t	qt_rtblk_res;	  /* # blks reserved on a dquot */
> +	uint64_t	qt_rtblk_res_used;/* # blks used from reservation */
> +	int64_t		qt_rtbcount_delta;/* dquot realtime blk changes */
> +	int64_t		qt_delrtb_delta;  /* delayed RT blk count changes */
> +
> +	unsigned long	qt_ino_res;	  /* inode reserved on a dquot */
> +	unsigned long	qt_ino_res_used;  /* inodes used from the reservation */

Canyou get rid of the ulongs here and make them explicit? uint32_t
should be large enough for them, but it might just be simpler to
make everything 64 bit variables.

ANd that....

>  		/*
>  		 * inode reservation
>  		 */
>  	      case XFS_TRANS_DQ_RES_INOS:
> -		qtrx->qt_ino_res += (ulong)delta;
> +		qtrx->qt_ino_res += (unsigned long)delta;
>  		break;
>  
>  		/*
> @@ -245,7 +245,7 @@ xfs_trans_mod_dquot(
>  		 */
>  	      case XFS_TRANS_DQ_ICOUNT:
>  		if (qtrx->qt_ino_res && delta > 0) {
> -			qtrx->qt_ino_res_used += (ulong)delta;
> +			qtrx->qt_ino_res_used += (unsigned long)delta;
>  			ASSERT(qtrx->qt_ino_res >= qtrx->qt_ino_res_used);

would get rid of these casts, too.

But, otherwise, this is a long overdue cleanup :)

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

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] xfs: widen inode delalloc block counter to 64-bits

[Dave Chinner]

On Tue, Apr 16, 2019 at 06:39:54PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Widen the incore inode's i_delayed_blks counter to be a 64-bit integer.
> This is necessary to fix an integer overflow problem that can be
> reproduced easily now that we use the counter to track blocks that are
> assigned to the inode in memory but not on disk.  This includes actual
> delalloc reservations as well as real extents in the COW fork that
> are waiting to be remapped into the data fork.
> 
> These 'delayed mapping' blocks can easily exceed 2^32 blocks if one
> creates a very large sparse file of size approximately 2^33 bytes with
> one byte written every 2^23 bytes, sets a very large COW extent size
> hint of 2^23 blocks, reflinks the first file into a second file, and
> then writes a single byte every 2^23 blocks in the original file.
> 
> When this happens, we'll try to create approximately 1024 2^23 extent
> reservations in the COW fork, which will overflow the counter and cause
> problems.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_inode.h |    2 +-
>  fs/xfs/xfs_qm.c    |    3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 87e701b638ae..558173f95a03 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -56,7 +56,7 @@ typedef struct xfs_inode {
>  	spinlock_t		i_flags_lock;	/* inode i_flags lock */
>  	/* Miscellaneous state. */
>  	unsigned long		i_flags;	/* see defined flags below */
> -	unsigned int		i_delayed_blks;	/* count of delay alloc blks */
> +	uint64_t		i_delayed_blks;	/* count of delay alloc blks */
>  
>  	struct xfs_icdinode	i_d;		/* most of ondisk inode */

This fills a 4 byte hole in the structure, yes?  Might be worth
mentioning in the commit log that it doesn't increase the size of
the struct inode, then.

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

Re: [PATCH 3/2] xfstests: check for COW overflows in i_delayed_blks

[Dave Chinner]

On Tue, Apr 16, 2019 at 11:24:50PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> With the new copy on write functionality it's possible to reserve so
> much COW space for a file that we end up overflowing i_delayed_blks.
> The only user-visible effect of this is to cause totally wrong i_blocks
> output in stat, so check for that.
....
> 
> +	umount $loop_mount > /dev/null 2>&1
> +	rm -rf $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/reflink
> +
> +# real QA test starts here
> +_supported_os Linux
> +_supported_fs xfs
> +_require_scratch_reflink
> +_require_loop
> +_require_xfs_debug
> +
> +echo "Format and mount"
> +_scratch_mkfs > "$seqres.full" 2>&1
> +_scratch_mount
> +_require_fs_space $SCRATCH_MNT 2400000	# 100T fs requires ~2.4GB of space
> +
> +loop_file=$SCRATCH_MNT/a.img
> +loop_mount=$SCRATCH_MNT/a
> +truncate -s 100T $loop_file
> +$MKFS_XFS_PROG $MKFS_OPTIONS -f $loop_file >> $seqres.full

Hmm - that's going to create a 2GB log and zero it, meaning on slow
devices this is going to take some time.

lodev=$(_create_loop_device $file)
_mkfs_dev -l size=128m $lodev


> +mkdir $loop_mount
> +mount -o loop -t xfs $loop_file $loop_mount
> +
> +echo "Create crazy huge file"
> +touch "${loop_mount}/a"
> +blksz="$(stat -f -c '%S' "${loop_mount}")"
> +MAXEXTLEN=2097151	# cowextsize can't be more than MAXEXTLEN
> +extsize="$(( ((2 ** 32) - 1) / blksz ))"
> +test "${extsize}" -gt "${MAXEXTLEN}" && extsize="${MAXEXTLEN}"
> +extsize_bytes="$(( extsize * blksz ))"

This is overkill, yes? When is extsize_bytes not equal to MAXEXTLEN
* blksz on this 100TB filesystem?


> +# Set the largest cowextsize we can
> +$XFS_IO_PROG -c "cowextsize ${extsize_bytes}" "${loop_mount}/a"
> +set_cowextsize="$($XFS_IO_PROG -c 'cowextsize' "${loop_mount}/a" | sed -e 's/^.\([0-9]*\).*$/\1/g')"
> +test "${set_cowextsize}" -eq 0 && _fail "could not set cowextsize?"

Run the test anyway, even if the cowextsize setting fails. WHo knows
what random crap will fall out....

> +statB="$(stat -c '%B' "${loop_mount}/a")"
> +
> +# Write a single byte every cowextsize bytes so that we minimize the space
> +# required to create maximally sized cow reservations
> +nr="$(( ((2 ** 32) / extsize) + 100 ))"

What's the magic 2^32 here?

> +seq 0 "${nr}" | tac | while read n; do

seq ${nr} -1 0 | while read n; do

> +	off="$((n * extsize * blksz))"
> +	$XFS_IO_PROG -c "pwrite ${off} 1" "${loop_mount}/a" > /dev/null
> +done
> +
> +echo "Reflink crazy huge file"
> +cp --reflink=always "${loop_mount}/a" "${loop_mount}/b"
> +
> +echo "COW crazy huge file"
> +# Try to create enough maximally sized cow reservations to overflow
> +# i_delayed_blks
> +seq 0 "${nr}" | tac | while read n; do
> +	off="$((n * extsize * blksz))"
> +	$XFS_IO_PROG -c "pwrite ${off} 1" "${loop_mount}/a" > /dev/null
> +done
> +
> +echo "Check crazy huge file"
> +blocks="$(stat -c '%b' "${loop_mount}/a")"
> +fsblocks="$((blocks * statB / blksz))"
> +
> +# Make sure we got enough COW reservations to overflow a 32-bit counter.
> +$XFS_IO_PROG -c 'bmap -clpv' "${loop_mount}/a" > $tmp.extents
> +echo "COW EXTENT STATE" >> $seqres.full
> +cat $tmp.extents >> $seqres.full
> +cat > $tmp.awk << ENDL
> +{
> +	if (\$3 == "delalloc") {
> +		x += \$4;
> +	} else if (\$3 == "hole") {
> +		;
> +	} else {
> +		x += \$6;
> +	}
> +}
> +END {
> +	printf("%d\\n", x / ($blksz / 512));
> +}
> +ENDL

Write that as a filter function and use tee to direct it to
seqres.full and the filter function at the same time?

> +cat $tmp.awk >> $seqres.full
> +cowblocks="$(awk -f $tmp.awk $tmp.extents)"
> +echo "cowblocks is ${cowblocks}" >> $seqres.full
> +if [ "${cowblocks}" -lt "$((2 ** 32))" ]; then
> +	echo "cowblocks (${cowblocks}) should be more than 2^32!"
> +fi
> +
> +# And finally, see if i_delayed_blks overflowed.
> +echo "stat blocks is ${fsblocks}" >> $seqres.full
> +if [ "${fsblocks}" -lt "$((2 ** 32))" ]; then
> +	echo "stat blocks (${fsblocks}) should be more than 2^32!"
> +	if [ "${cowblocks}" -lt "$((2 ** 32))" ]; then
> +		echo "cowblocks (${cowblocks}) is more than 2^32, your system has overflowed!!!"
> +	fi
> +fi

_within_tolerance?

CHeers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/2] xfstests: check for COW overflows in i_delayed_blks

[Darrick J. Wong]

On Thu, Apr 18, 2019 at 07:29:50AM +1000, Dave Chinner wrote:
> On Tue, Apr 16, 2019 at 11:24:50PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > With the new copy on write functionality it's possible to reserve so
> > much COW space for a file that we end up overflowing i_delayed_blks.
> > The only user-visible effect of this is to cause totally wrong i_blocks
> > output in stat, so check for that.
> ....
> > 
> > +	umount $loop_mount > /dev/null 2>&1
> > +	rm -rf $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +_supported_os Linux
> > +_supported_fs xfs
> > +_require_scratch_reflink
> > +_require_loop
> > +_require_xfs_debug
> > +
> > +echo "Format and mount"
> > +_scratch_mkfs > "$seqres.full" 2>&1
> > +_scratch_mount
> > +_require_fs_space $SCRATCH_MNT 2400000	# 100T fs requires ~2.4GB of space
> > +
> > +loop_file=$SCRATCH_MNT/a.img
> > +loop_mount=$SCRATCH_MNT/a
> > +truncate -s 100T $loop_file
> > +$MKFS_XFS_PROG $MKFS_OPTIONS -f $loop_file >> $seqres.full
> 
> Hmm - that's going to create a 2GB log and zero it, meaning on slow
> devices this is going to take some time.
> 
> lodev=$(_create_loop_device $file)
> _mkfs_dev -l size=128m $lodev

<nod>

> 
> > +mkdir $loop_mount
> > +mount -o loop -t xfs $loop_file $loop_mount
> > +
> > +echo "Create crazy huge file"
> > +touch "${loop_mount}/a"
> > +blksz="$(stat -f -c '%S' "${loop_mount}")"
> > +MAXEXTLEN=2097151	# cowextsize can't be more than MAXEXTLEN
> > +extsize="$(( ((2 ** 32) - 1) / blksz ))"
> > +test "${extsize}" -gt "${MAXEXTLEN}" && extsize="${MAXEXTLEN}"
> > +extsize_bytes="$(( extsize * blksz ))"
> 
> This is overkill, yes? When is extsize_bytes not equal to MAXEXTLEN
> * blksz on this 100TB filesystem?

Most of the time.  struct fsxattr.fsx_cowextsize is a u32 field and
expects units of bytes, which means that we have to clamp the hint we
set on any filesystem with larger than 2k blocks.

> > +# Set the largest cowextsize we can
> > +$XFS_IO_PROG -c "cowextsize ${extsize_bytes}" "${loop_mount}/a"
> > +set_cowextsize="$($XFS_IO_PROG -c 'cowextsize' "${loop_mount}/a" | sed -e 's/^.\([0-9]*\).*$/\1/g')"
> > +test "${set_cowextsize}" -eq 0 && _fail "could not set cowextsize?"
> 
> Run the test anyway, even if the cowextsize setting fails. WHo knows
> what random crap will fall out....

Ok.

> > +statB="$(stat -c '%B' "${loop_mount}/a")"
> > +
> > +# Write a single byte every cowextsize bytes so that we minimize the space
> > +# required to create maximally sized cow reservations
> > +nr="$(( ((2 ** 32) / extsize) + 100 ))"
> 
> What's the magic 2^32 here?

We're relying on cowextsize hints to create oversized speculative
preallocations in the cow fork to bump up i_delayed_blks, so we only
really have to touch a block every extsize_bytes.

> 
> > +seq 0 "${nr}" | tac | while read n; do
> 
> seq ${nr} -1 0 | while read n; do

Ok

> 
> > +	off="$((n * extsize * blksz))"
> > +	$XFS_IO_PROG -c "pwrite ${off} 1" "${loop_mount}/a" > /dev/null
> > +done
> > +
> > +echo "Reflink crazy huge file"
> > +cp --reflink=always "${loop_mount}/a" "${loop_mount}/b"
> > +
> > +echo "COW crazy huge file"
> > +# Try to create enough maximally sized cow reservations to overflow
> > +# i_delayed_blks
> > +seq 0 "${nr}" | tac | while read n; do
> > +	off="$((n * extsize * blksz))"
> > +	$XFS_IO_PROG -c "pwrite ${off} 1" "${loop_mount}/a" > /dev/null
> > +done
> > +
> > +echo "Check crazy huge file"
> > +blocks="$(stat -c '%b' "${loop_mount}/a")"
> > +fsblocks="$((blocks * statB / blksz))"
> > +
> > +# Make sure we got enough COW reservations to overflow a 32-bit counter.
> > +$XFS_IO_PROG -c 'bmap -clpv' "${loop_mount}/a" > $tmp.extents
> > +echo "COW EXTENT STATE" >> $seqres.full
> > +cat $tmp.extents >> $seqres.full
> > +cat > $tmp.awk << ENDL
> > +{
> > +	if (\$3 == "delalloc") {
> > +		x += \$4;
> > +	} else if (\$3 == "hole") {
> > +		;
> > +	} else {
> > +		x += \$6;
> > +	}
> > +}
> > +END {
> > +	printf("%d\\n", x / ($blksz / 512));
> > +}
> > +ENDL
> 
> Write that as a filter function and use tee to direct it to
> seqres.full and the filter function at the same time?

Ok.

> > +cat $tmp.awk >> $seqres.full
> > +cowblocks="$(awk -f $tmp.awk $tmp.extents)"
> > +echo "cowblocks is ${cowblocks}" >> $seqres.full
> > +if [ "${cowblocks}" -lt "$((2 ** 32))" ]; then
> > +	echo "cowblocks (${cowblocks}) should be more than 2^32!"
> > +fi
> > +
> > +# And finally, see if i_delayed_blks overflowed.
> > +echo "stat blocks is ${fsblocks}" >> $seqres.full
> > +if [ "${fsblocks}" -lt "$((2 ** 32))" ]; then
> > +	echo "stat blocks (${fsblocks}) should be more than 2^32!"
> > +	if [ "${cowblocks}" -lt "$((2 ** 32))" ]; then
> > +		echo "cowblocks (${cowblocks}) is more than 2^32, your system has overflowed!!!"
> > +	fi
> > +fi
> 
> _within_tolerance?

Sure?  I only care that it's above 2^32 though, not that we have an
exact value ... but I guess we can put fairly wide thresholds on that
comparison since if we overflow then the counter will be way off.

--D

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

Re: [PATCH 1/2] xfs: widen quota block counters to 64-bit integers

[Christoph Hellwig]

Looks good,

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

(and yes, I wouldn't mind the cast removal either).

Re: [PATCH 2/2] xfs: widen inode delalloc block counter to 64-bits

[Christoph Hellwig]

Looks good,

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