* [PATCH RESEND 2/2] xfs: Bail out if transaction can cause extent count to overflow
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
1 sibling, 0 replies; 3+ messages in thread
From: Chandan Babu R @ 2020-08-01 8:28 UTC (permalink / raw)
To: linux-xfs; +Cc: Chandan Babu R, darrick.wong, david
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
^ permalink raw reply related [flat|nested] 3+ messages in thread