All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chandan Babu R <chandanrlinux@gmail.com>
To: linux-xfs@vger.kernel.org
Cc: Chandan Babu R <chandanrlinux@gmail.com>,
	darrick.wong@oracle.com, david@fromorbit.com
Subject: [PATCH RESEND 2/2] xfs: Bail out if transaction can cause extent count to overflow
Date: Sat,  1 Aug 2020 13:58:03 +0530	[thread overview]
Message-ID: <20200801082803.12109-3-chandanrlinux@gmail.com> (raw)
In-Reply-To: <20200801082803.12109-1-chandanrlinux@gmail.com>

This commit computes the maximum number of new extents that can be added
to per-inode forks (data and xattr) at the beginning of a
transaction. It then uses the helper function xfs_trans_resv_ext_cnt()
to verify that the resulting sum does not overflow the corresponding
on-disk inode extent counter.

Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 fs/xfs/libxfs/xfs_attr.c | 33 ++++++++++++--
 fs/xfs/libxfs/xfs_bmap.c |  7 +++
 fs/xfs/xfs_bmap_item.c   | 12 +++++
 fs/xfs/xfs_bmap_util.c   | 40 +++++++++++++++++
 fs/xfs/xfs_dquot.c       |  7 ++-
 fs/xfs/xfs_inode.c       | 96 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_iomap.c       | 19 ++++++++
 fs/xfs/xfs_reflink.c     | 35 +++++++++++++++
 fs/xfs/xfs_rtalloc.c     |  4 ++
 fs/xfs/xfs_symlink.c     | 18 ++++++++
 10 files changed, 267 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index d4583a0d1b3f..745bf9293a17 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -142,7 +142,8 @@ xfs_attr_get(
 STATIC int
 xfs_attr_calc_size(
 	struct xfs_da_args	*args,
-	int			*local)
+	int			*local,
+	int			*dsplit)
 {
 	struct xfs_mount	*mp = args->dp->i_mount;
 	int			size;
@@ -157,7 +158,10 @@ xfs_attr_calc_size(
 	if (*local) {
 		if (size > (args->geo->blksize / 2)) {
 			/* Double split possible */
+			*dsplit = 1;
 			nblks *= 2;
+		} else {
+			*dsplit = 0;
 		}
 	} else {
 		/*
@@ -395,7 +399,8 @@ xfs_attr_set(
 	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_trans_res	tres;
 	bool			rsvd = (args->attr_filter & XFS_ATTR_ROOT);
-	int			error, local;
+	int			error, local, dsplit;
+	int			rsv_exts = 0;
 	unsigned int		total;
 
 	if (XFS_FORCED_SHUTDOWN(dp->i_mount))
@@ -420,7 +425,7 @@ xfs_attr_set(
 		XFS_STATS_INC(mp, xs_attr_set);
 
 		args->op_flags |= XFS_DA_OP_ADDNAME;
-		args->total = xfs_attr_calc_size(args, &local);
+		args->total = xfs_attr_calc_size(args, &local, &dsplit);
 
 		/*
 		 * If the inode doesn't have an attribute fork, add one.
@@ -442,11 +447,19 @@ xfs_attr_set(
 		tres.tr_logcount = XFS_ATTRSET_LOG_COUNT;
 		tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
 		total = args->total;
+
+		if (local) {
+			if (dsplit)
+				++rsv_exts;
+		} else {
+			rsv_exts += xfs_attr3_rmt_blocks(mp, args->valuelen);
+		}
 	} else {
 		XFS_STATS_INC(mp, xs_attr_remove);
 
 		tres = M_RES(mp)->tr_attrrm;
 		total = XFS_ATTRRM_SPACE_RES(mp);
+		rsv_exts += xfs_attr3_rmt_blocks(mp, XFS_XATTR_SIZE_MAX);
 	}
 
 	/*
@@ -460,6 +473,20 @@ xfs_attr_set(
 
 	xfs_ilock(dp, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(args->trans, dp, 0);
+
+	rsv_exts += XFS_DAENTER_BLOCKS(mp, XFS_ATTR_FORK);
+
+	if (args->value || xfs_inode_hasattr(dp)) {
+		/*
+		 * XFS_DA_NODE_MAXDEPTH blocks for dabtree.
+		 * One extra block for dabtree in case of a double split.
+		 * Extents for remote attributes.
+		 */
+		error = xfs_trans_resv_ext_cnt(dp, XFS_ATTR_FORK, rsv_exts);
+		if (error)
+			goto out_trans_cancel;
+	}
+
 	if (args->value) {
 		unsigned int	quota_flags = XFS_QMOPT_RES_REGBLKS;
 
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9c40d5971035..462869ab26b9 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4527,6 +4527,13 @@ xfs_bmapi_convert_delalloc(
 		return error;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
+
+	if (whichfork == XFS_DATA_FORK) {
+		error = xfs_trans_resv_ext_cnt(ip, whichfork, 1);
+		if (error)
+			goto out_trans_cancel;
+	}
+
 	xfs_trans_ijoin(tp, ip, 0);
 
 	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &bma.icur, &bma.got) ||
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index ec3691372e7c..de427444cc8a 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -519,6 +519,18 @@ xfs_bui_item_recover(
 	}
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * Removing an extent from the middle of an existing extent
+	 * can cause the extent count to increase by 1.
+	 * i.e. | Old extent | Hole | Old extent |
+	 *
+	 * Mapping a new extent into a file can cause the extent
+	 * count to increase by 1.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, whichfork, 1);
+	if (error)
+		goto err_inode;
+
 	count = bmap->me_len;
 	error = xfs_trans_log_finish_bmap_update(tp, budp, type, ip, whichfork,
 			bmap->me_startoff, bmap->me_startblock, &count, state);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index afdc7f8e0e70..a8cd0f7c6005 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -822,6 +822,10 @@ xfs_alloc_file_space(
 		if (error)
 			goto error1;
 
+		error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+		if (error)
+			goto error0;
+
 		xfs_trans_ijoin(tp, ip, 0);
 
 		error = xfs_bmapi_write(tp, ip, startoffset_fsb,
@@ -886,6 +890,15 @@ xfs_unmap_extent(
 
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * One extent encompasses the complete file offset range.
+	 * Removing the file offset range causes extent count to
+	 * increase by 1.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+	if (error)
+		goto out_trans_cancel;
+
 	error = xfs_bunmapi(tp, ip, startoffset_fsb, len_fsb, 0, 2, done);
 	if (error)
 		goto out_trans_cancel;
@@ -1155,6 +1168,14 @@ xfs_insert_file_space(
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * Splitting the extent mapping containing stop_fsb will cause
+	 * extent count to increase by 1.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * The extent shifting code works on extent granularity. So, if stop_fsb
 	 * is not the starting block of extent, we need to split the extent at
@@ -1356,6 +1377,25 @@ xfs_swap_extent_rmap(
 		/* Unmap the old blocks in the source file. */
 		while (tirec.br_blockcount) {
 			ASSERT(tp->t_firstblock == NULLFSBLOCK);
+
+			/*
+			 * Removing an initial part of source file's extent and
+			 * adding a new extent (from donor file) in its place
+			 * will cause extent count to increase by 1.
+			 */
+			error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+			if (error)
+				goto out;
+
+			/*
+			 * Removing an initial part of donor file's extent and
+			 * adding a new extent (from source file) in its place
+			 * will cause extent count to increase by 1.
+			 */
+			error = xfs_trans_resv_ext_cnt(tip, XFS_DATA_FORK, 1);
+			if (error)
+				goto out;
+
 			trace_xfs_swap_extent_rmap_remap_piece(tip, &tirec);
 
 			/* Read extent from the source file */
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 04dc2be19c3a..582f050595bc 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -290,8 +290,13 @@ xfs_dquot_disk_alloc(
 		return -ESRCH;
 	}
 
-	/* Create the block mapping. */
 	xfs_trans_ijoin(tp, quotip, XFS_ILOCK_EXCL);
+
+	error = xfs_trans_resv_ext_cnt(quotip, XFS_DATA_FORK, 1);
+	if (error)
+		return error;
+
+	/* Create the block mapping. */
 	error = xfs_bmapi_write(tp, quotip, dqp->q_fileoffset,
 			XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA, 0, &map,
 			&nmaps);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 407d6299606d..007a719c2cdf 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1175,6 +1175,24 @@ xfs_create(
 	if (error)
 		goto out_trans_cancel;
 
+	/*
+	 * Directory entry addition can cause the following,
+	 * 1. Data block can be added.
+	 *    A new extent can cause extent count to increase by 1.
+	 * 2. Free disk block can be added.
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be added. Each of these
+	 *    can be new extents. Hence extent count can increase by
+	 *    XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+			mp->m_dir_geo->fsbcount);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * A newly created regular or special file just has one directory
 	 * entry pointing to them, but a directory also the "." entry
@@ -1391,6 +1409,24 @@ xfs_link(
 	xfs_trans_ijoin(tp, sip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, tdp, XFS_ILOCK_EXCL);
 
+	/*
+	 * Creating a new link can cause the following,
+	 * 1. Data block can be added.
+	 *    A new extent can cause extent count to increase by 1.
+	 * 2. Free disk block can be added.
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be added. Each of these
+	 *    can be new extents. Hence extent count can increase by
+	 *    XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(tdp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+	if (error)
+		goto error_return;
+
 	/*
 	 * If we are using project inheritance, we only allow hard link
 	 * creation in our tree when the project IDs are the same; else
@@ -2861,6 +2897,26 @@ xfs_remove(
 	xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
 
+	/*
+	 * Directory entry removal can cause the following,
+	 * 1. Data block can be freed.
+	 *    3 data blocks can be contiguous. Deletion of a single
+	 *    data block can cause this single extent to be split into
+	 *    two. Hence extent count can increase by 1.
+	 * 2. Free disk block
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be freed. Each of these
+	 *    blocks can cause a single extent to be split into two.
+	 *    Hence extent count can increase by XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * If we're removing a directory perform some additional validation.
 	 */
@@ -3221,6 +3277,46 @@ xfs_rename(
 	if (wip)
 		xfs_trans_ijoin(tp, wip, XFS_ILOCK_EXCL);
 
+	/*
+	 * Directory entry removal can cause the following,
+	 * 1. Data block can be freed.
+	 *    3 data blocks can be contiguous. Deletion of a single
+	 *    data block can cause this single extent to be split into
+	 *    two. Hence extent count can increase by 1.
+	 * 2. Free disk block
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be freed. Each of these
+	 *    blocks can cause a single extent to be split into two.
+	 *    Hence extent count can increase by XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(src_dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+	if (error)
+		goto out_trans_cancel;
+
+	/*
+	 * Directory entry addition can cause the following,
+	 * 1. Data block can be added.
+	 *    A new extent can cause extent count to increase by 1.
+	 * 2. Free disk block can be added.
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be added. Each of these
+	 *    can be new extents. Hence extent count can increase by
+	 *    XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	if (target_ip == NULL) {
+		error = xfs_trans_resv_ext_cnt(target_dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+		if (error)
+			goto out_trans_cancel;
+	}
+
 	/*
 	 * If we are using project inheritance, we only allow renames
 	 * into our tree when the project IDs are the same; else the
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 0e3f62cde375..e7ea3b7bbb0d 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -250,6 +250,14 @@ xfs_iomap_write_direct(
 	if (error)
 		goto out_trans_cancel;
 
+	/*
+	 * Writing to a hole or extending a file can cause
+	 * the extent count to increase by 1.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+	if (error)
+		goto out_trans_cancel;
+
 	xfs_trans_ijoin(tp, ip, 0);
 
 	/*
@@ -561,6 +569,17 @@ xfs_iomap_write_unwritten(
 		if (error)
 			goto error_on_bmapi_transaction;
 
+		/*
+		 * We might be writing to the middle region of an
+		 * existing unwritten extent. This causes the original
+		 * extent to be split into 3 extents
+		 * i.e. | Unwritten | Real | Unwritten |
+		 * Hence extent count can increase by 2.
+		 */
+		error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 2);
+		if (error)
+			goto error_on_bmapi_transaction;
+
 		/*
 		 * Modify the unwritten extent state of the buffer.
 		 */
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index aac83f9d6107..45fa89558cdb 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -29,6 +29,7 @@
 #include "xfs_iomap.h"
 #include "xfs_sb.h"
 #include "xfs_ag_resv.h"
+#include "xfs_trans_resv.h"
 
 /*
  * Copy on Write of Shared Blocks
@@ -628,6 +629,17 @@ xfs_reflink_end_cow_extent(
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * Extents are unmapped starting from "end_fsb - 1" and moves
+	 * towards offset_fsb.  A data fork extent containing
+	 * "end_fsb - 1" can be split into three parts i.e.
+	 * | Old extent | New extent | Old extent |
+	 * Hence number of extents increases by 2.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 2);
+	if (error)
+		goto out_cancel;
+
 	/*
 	 * In case of racing, overlapping AIO writes no COW extents might be
 	 * left by the time I/O completes for the loser of the race.  In that
@@ -1002,6 +1014,7 @@ xfs_reflink_remap_extent(
 	bool			smap_real;
 	bool			dmap_written = xfs_bmap_is_written_extent(dmap);
 	int			nimaps;
+	int			resv_exts = 0;
 	int			error;
 
 	/* Start a rolling transaction to switch the mappings */
@@ -1094,6 +1107,28 @@ xfs_reflink_remap_extent(
 			goto out_cancel;
 	}
 
+	/*
+	 * When unmapping, an extent containing the entire unmap
+	 * range can be split into two extents,
+	 * i.e. | old extent | hole | old extent |
+	 * Hence extent count increases by 1.
+	 */
+	if (smap_real)
+		++resv_exts;
+
+	/*
+	 * Mapping in the new extent into the destination file can
+	 * increase the extent count by 1.
+	 */
+	if (dmap_written)
+		++resv_exts;
+
+	if (resv_exts) {
+		error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, resv_exts);
+		if (error)
+			goto out_cancel;
+	}
+
 	if (smap_real) {
 		/*
 		 * If the extent we're unmapping is backed by storage (written
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 6209e7b6b895..a2e640f43f1e 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -787,6 +787,10 @@ xfs_growfs_rt_alloc(
 		xfs_ilock(ip, XFS_ILOCK_EXCL);
 		xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
 
+		error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+		if (error)
+			goto out_trans_cancel;
+
 		/*
 		 * Allocate blocks to the bitmap file.
 		 */
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 8e88a7ca387e..1bc71576289d 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -220,6 +220,24 @@ xfs_symlink(
 	if (error)
 		goto out_trans_cancel;
 
+	/*
+	 * Creating a new symlink can cause the following,
+	 * 1. Data block can be added.
+	 *    A new extent can cause extent count to increase by 1.
+	 * 2. Free disk block can be added.
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be added. Each of these
+	 *    can be new extents. Hence extent count can increase by
+	 *    XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * Allocate an inode for the symlink.
 	 */
-- 
2.20.1


      parent reply	other threads:[~2020-08-01  8:28 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-01  8:28 [PATCH RESEND 0/2] Bail out if transaction can cause extent count to overflow Chandan Babu R
2020-08-01  8:28 ` [PATCH RESEND 1/2] xfs: Add helper for checking per-inode extent count overflow Chandan Babu R
2020-08-01  8:28 ` Chandan Babu R [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200801082803.12109-3-chandanrlinux@gmail.com \
    --to=chandanrlinux@gmail.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.