* [PATCH v7 1/5] xfs: update lazy sb counters immediately for resizefs
2021-03-02 2:48 [PATCH v7 0/5] xfs: support shrinking free space in the last AG Gao Xiang
@ 2021-03-02 2:48 ` Gao Xiang
2021-03-03 18:13 ` Darrick J. Wong
2021-03-02 2:48 ` [PATCH v7 2/5] xfs: hoist out xfs_resizefs_init_new_ags() Gao Xiang
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2021-03-02 2:48 UTC (permalink / raw)
To: linux-xfs
Cc: Darrick J. Wong, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen, Gao Xiang
sb_fdblocks will be updated lazily if lazysbcount is enabled,
therefore when shrinking the filesystem sb_fdblocks could be
larger than sb_dblocks and xfs_validate_sb_write() would fail.
Even for growfs case, it'd be better to update lazy sb counters
immediately to reflect the real sb counters.
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
fs/xfs/xfs_fsops.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index a2a407039227..9f9ba8bd0213 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -128,6 +128,15 @@ xfs_growfs_data_private(
nb - mp->m_sb.sb_dblocks);
if (id.nfree)
xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree);
+
+ /*
+ * Sync sb counters now to reflect the updated values. This is
+ * particularly important for shrink because the write verifier
+ * will fail if sb_fdblocks is ever larger than sb_dblocks.
+ */
+ if (xfs_sb_version_haslazysbcount(&mp->m_sb))
+ xfs_log_sb(tp);
+
xfs_trans_set_sync(tp);
error = xfs_trans_commit(tp);
if (error)
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 1/5] xfs: update lazy sb counters immediately for resizefs
2021-03-02 2:48 ` [PATCH v7 1/5] xfs: update lazy sb counters immediately for resizefs Gao Xiang
@ 2021-03-03 18:13 ` Darrick J. Wong
0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2021-03-03 18:13 UTC (permalink / raw)
To: Gao Xiang
Cc: linux-xfs, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen
On Tue, Mar 02, 2021 at 10:48:12AM +0800, Gao Xiang wrote:
> sb_fdblocks will be updated lazily if lazysbcount is enabled,
> therefore when shrinking the filesystem sb_fdblocks could be
> larger than sb_dblocks and xfs_validate_sb_write() would fail.
>
> Even for growfs case, it'd be better to update lazy sb counters
> immediately to reflect the real sb counters.
>
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/xfs_fsops.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index a2a407039227..9f9ba8bd0213 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -128,6 +128,15 @@ xfs_growfs_data_private(
> nb - mp->m_sb.sb_dblocks);
> if (id.nfree)
> xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree);
> +
> + /*
> + * Sync sb counters now to reflect the updated values. This is
> + * particularly important for shrink because the write verifier
> + * will fail if sb_fdblocks is ever larger than sb_dblocks.
> + */
> + if (xfs_sb_version_haslazysbcount(&mp->m_sb))
> + xfs_log_sb(tp);
> +
> xfs_trans_set_sync(tp);
> error = xfs_trans_commit(tp);
> if (error)
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 2/5] xfs: hoist out xfs_resizefs_init_new_ags()
2021-03-02 2:48 [PATCH v7 0/5] xfs: support shrinking free space in the last AG Gao Xiang
2021-03-02 2:48 ` [PATCH v7 1/5] xfs: update lazy sb counters immediately for resizefs Gao Xiang
@ 2021-03-02 2:48 ` Gao Xiang
2021-03-02 2:48 ` [PATCH v7 3/5] xfs: introduce xfs_ag_shrink_space() Gao Xiang
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2021-03-02 2:48 UTC (permalink / raw)
To: linux-xfs
Cc: Darrick J. Wong, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen, Gao Xiang, Darrick J . Wong
Move out related logic for initializing new added AGs to a new helper
in preparation for shrinking. No logic changes.
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
fs/xfs/xfs_fsops.c | 110 ++++++++++++++++++++++++++-------------------
1 file changed, 65 insertions(+), 45 deletions(-)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 9f9ba8bd0213..494f9354e3fb 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -20,6 +20,64 @@
#include "xfs_ag.h"
#include "xfs_ag_resv.h"
+/*
+ * Write new AG headers to disk. Non-transactional, but need to be
+ * written and completed prior to the growfs transaction being logged.
+ * To do this, we use a delayed write buffer list and wait for
+ * submission and IO completion of the list as a whole. This allows the
+ * IO subsystem to merge all the AG headers in a single AG into a single
+ * IO and hide most of the latency of the IO from us.
+ *
+ * This also means that if we get an error whilst building the buffer
+ * list to write, we can cancel the entire list without having written
+ * anything.
+ */
+static int
+xfs_resizefs_init_new_ags(
+ struct xfs_trans *tp,
+ struct aghdr_init_data *id,
+ xfs_agnumber_t oagcount,
+ xfs_agnumber_t nagcount,
+ xfs_rfsblock_t delta,
+ bool *lastag_resetagres)
+{
+ struct xfs_mount *mp = tp->t_mountp;
+ xfs_rfsblock_t nb = mp->m_sb.sb_dblocks + delta;
+ int error;
+
+ *lastag_resetagres = false;
+
+ INIT_LIST_HEAD(&id->buffer_list);
+ for (id->agno = nagcount - 1;
+ id->agno >= oagcount;
+ id->agno--, delta -= id->agsize) {
+
+ if (id->agno == nagcount - 1)
+ id->agsize = nb - (id->agno *
+ (xfs_rfsblock_t)mp->m_sb.sb_agblocks);
+ else
+ id->agsize = mp->m_sb.sb_agblocks;
+
+ error = xfs_ag_init_headers(mp, id);
+ if (error) {
+ xfs_buf_delwri_cancel(&id->buffer_list);
+ return error;
+ }
+ }
+
+ error = xfs_buf_delwri_submit(&id->buffer_list);
+ if (error)
+ return error;
+
+ xfs_trans_agblocks_delta(tp, id->nfree);
+
+ if (delta) {
+ *lastag_resetagres = true;
+ error = xfs_ag_extend_space(mp, tp, id, delta);
+ }
+ return error;
+}
+
/*
* growfs operations
*/
@@ -32,8 +90,8 @@ xfs_growfs_data_private(
int error;
xfs_agnumber_t nagcount;
xfs_agnumber_t nagimax = 0;
- xfs_rfsblock_t nb, nb_div, nb_mod;
- xfs_rfsblock_t delta;
+ xfs_rfsblock_t nb, nb_div, nb_mod, delta;
+ bool lastag_resetagres;
xfs_agnumber_t oagcount;
struct xfs_trans *tp;
struct aghdr_init_data id = {};
@@ -74,48 +132,11 @@ xfs_growfs_data_private(
if (error)
return error;
- /*
- * Write new AG headers to disk. Non-transactional, but need to be
- * written and completed prior to the growfs transaction being logged.
- * To do this, we use a delayed write buffer list and wait for
- * submission and IO completion of the list as a whole. This allows the
- * IO subsystem to merge all the AG headers in a single AG into a single
- * IO and hide most of the latency of the IO from us.
- *
- * This also means that if we get an error whilst building the buffer
- * list to write, we can cancel the entire list without having written
- * anything.
- */
- INIT_LIST_HEAD(&id.buffer_list);
- for (id.agno = nagcount - 1;
- id.agno >= oagcount;
- id.agno--, delta -= id.agsize) {
-
- if (id.agno == nagcount - 1)
- id.agsize = nb -
- (id.agno * (xfs_rfsblock_t)mp->m_sb.sb_agblocks);
- else
- id.agsize = mp->m_sb.sb_agblocks;
-
- error = xfs_ag_init_headers(mp, &id);
- if (error) {
- xfs_buf_delwri_cancel(&id.buffer_list);
- goto out_trans_cancel;
- }
- }
- error = xfs_buf_delwri_submit(&id.buffer_list);
+ error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount,
+ delta, &lastag_resetagres);
if (error)
goto out_trans_cancel;
- xfs_trans_agblocks_delta(tp, id.nfree);
-
- /* If there are new blocks in the old last AG, extend it. */
- if (delta) {
- error = xfs_ag_extend_space(mp, tp, &id, delta);
- if (error)
- goto out_trans_cancel;
- }
-
/*
* Update changed superblock fields transactionally. These are not
* seen by the rest of the world until the transaction commit applies
@@ -123,9 +144,8 @@ xfs_growfs_data_private(
*/
if (nagcount > oagcount)
xfs_trans_mod_sb(tp, XFS_TRANS_SB_AGCOUNT, nagcount - oagcount);
- if (nb > mp->m_sb.sb_dblocks)
- xfs_trans_mod_sb(tp, XFS_TRANS_SB_DBLOCKS,
- nb - mp->m_sb.sb_dblocks);
+ if (delta > 0)
+ xfs_trans_mod_sb(tp, XFS_TRANS_SB_DBLOCKS, delta);
if (id.nfree)
xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree);
@@ -152,7 +172,7 @@ xfs_growfs_data_private(
* If we expanded the last AG, free the per-AG reservation
* so we can reinitialize it with the new size.
*/
- if (delta) {
+ if (lastag_resetagres) {
struct xfs_perag *pag;
pag = xfs_perag_get(mp, id.agno);
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v7 3/5] xfs: introduce xfs_ag_shrink_space()
2021-03-02 2:48 [PATCH v7 0/5] xfs: support shrinking free space in the last AG Gao Xiang
2021-03-02 2:48 ` [PATCH v7 1/5] xfs: update lazy sb counters immediately for resizefs Gao Xiang
2021-03-02 2:48 ` [PATCH v7 2/5] xfs: hoist out xfs_resizefs_init_new_ags() Gao Xiang
@ 2021-03-02 2:48 ` Gao Xiang
2021-03-03 18:19 ` Darrick J. Wong
2021-03-02 2:48 ` [PATCH v7 4/5] xfs: support shrinking unused space in the last AG Gao Xiang
2021-03-02 2:48 ` [PATCH v7 5/5] xfs: add error injection for per-AG resv failure Gao Xiang
4 siblings, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2021-03-02 2:48 UTC (permalink / raw)
To: linux-xfs
Cc: Darrick J. Wong, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen, Gao Xiang
This patch introduces a helper to shrink unused space in the last AG
by fixing up the freespace btree.
Also make sure that the per-AG reservation works under the new AG
size. If such per-AG reservation or extent allocation fails, roll
the transaction so the new transaction could cancel without any side
effects.
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
fs/xfs/libxfs/xfs_ag.c | 108 +++++++++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_ag.h | 2 +
2 files changed, 110 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 9331f3516afa..a1128814630a 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -22,6 +22,11 @@
#include "xfs_ag.h"
#include "xfs_ag_resv.h"
#include "xfs_health.h"
+#include "xfs_error.h"
+#include "xfs_bmap.h"
+#include "xfs_defer.h"
+#include "xfs_log_format.h"
+#include "xfs_trans.h"
static int
xfs_get_aghdr_buf(
@@ -485,6 +490,109 @@ xfs_ag_init_headers(
return error;
}
+int
+xfs_ag_shrink_space(
+ struct xfs_mount *mp,
+ struct xfs_trans **tpp,
+ xfs_agnumber_t agno,
+ xfs_extlen_t len)
+{
+ struct xfs_alloc_arg args = {
+ .tp = *tpp,
+ .mp = mp,
+ .type = XFS_ALLOCTYPE_THIS_BNO,
+ .minlen = len,
+ .maxlen = len,
+ .oinfo = XFS_RMAP_OINFO_SKIP_UPDATE,
+ .resv = XFS_AG_RESV_NONE,
+ .prod = 1
+ };
+ struct xfs_buf *agibp, *agfbp;
+ struct xfs_agi *agi;
+ struct xfs_agf *agf;
+ int error, err2;
+
+ ASSERT(agno == mp->m_sb.sb_agcount - 1);
+ error = xfs_ialloc_read_agi(mp, *tpp, agno, &agibp);
+ if (error)
+ return error;
+
+ agi = agibp->b_addr;
+
+ error = xfs_alloc_read_agf(mp, *tpp, agno, 0, &agfbp);
+ if (error)
+ return error;
+
+ agf = agfbp->b_addr;
+ if (XFS_IS_CORRUPT(mp, agf->agf_length != agi->agi_length))
+ return -EFSCORRUPTED;
+
+ args.fsbno = XFS_AGB_TO_FSB(mp, agno,
+ be32_to_cpu(agi->agi_length) - len);
+
+ /* remove the preallocations before allocation and re-establish then */
+ error = xfs_ag_resv_free(agibp->b_pag);
+ if (error)
+ return error;
+
+ /* internal log shouldn't also show up in the free space btrees */
+ error = xfs_alloc_vextent(&args);
+ if (!error && args.agbno == NULLAGBLOCK)
+ error = -ENOSPC;
+
+ if (error) {
+ /*
+ * if extent allocation fails, need to roll the transaction to
+ * ensure that the AGFL fixup has been committed anyway.
+ */
+ err2 = xfs_trans_roll(tpp);
+ if (err2)
+ return err2;
+ goto resv_init_out;
+ }
+
+ /*
+ * if successfully deleted from freespace btrees, need to confirm
+ * per-AG reservation works as expected.
+ */
+ be32_add_cpu(&agi->agi_length, -len);
+ be32_add_cpu(&agf->agf_length, -len);
+
+ err2 = xfs_ag_resv_init(agibp->b_pag, *tpp);
+ if (err2) {
+ be32_add_cpu(&agi->agi_length, len);
+ be32_add_cpu(&agf->agf_length, len);
+ if (err2 != -ENOSPC)
+ goto resv_err;
+
+ __xfs_bmap_add_free(*tpp, args.fsbno, len, NULL, true);
+
+ /*
+ * Roll the transaction before trying to re-init the per-ag
+ * reservation. The new transaction is clean so it will cancel
+ * without any side effects.
+ */
+ error = xfs_defer_finish(tpp);
+ if (error)
+ return error;
+
+ error = -ENOSPC;
+ goto resv_init_out;
+ }
+ xfs_ialloc_log_agi(*tpp, agibp, XFS_AGI_LENGTH);
+ xfs_alloc_log_agf(*tpp, agfbp, XFS_AGF_LENGTH);
+ return 0;
+
+resv_init_out:
+ err2 = xfs_ag_resv_init(agibp->b_pag, *tpp);
+ if (!err2)
+ return error;
+resv_err:
+ xfs_warn(mp, "Error %d reserving per-AG metadata reserve pool.", err2);
+ xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+ return err2;
+}
+
/*
* Extent the AG indicated by the @id by the length passed in
*/
diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 5166322807e7..f33388eb130a 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -24,6 +24,8 @@ struct aghdr_init_data {
};
int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
+int xfs_ag_shrink_space(struct xfs_mount *mp, struct xfs_trans **tpp,
+ xfs_agnumber_t agno, xfs_extlen_t len);
int xfs_ag_extend_space(struct xfs_mount *mp, struct xfs_trans *tp,
struct aghdr_init_data *id, xfs_extlen_t len);
int xfs_ag_get_geometry(struct xfs_mount *mp, xfs_agnumber_t agno,
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 3/5] xfs: introduce xfs_ag_shrink_space()
2021-03-02 2:48 ` [PATCH v7 3/5] xfs: introduce xfs_ag_shrink_space() Gao Xiang
@ 2021-03-03 18:19 ` Darrick J. Wong
2021-03-03 23:16 ` Gao Xiang
0 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2021-03-03 18:19 UTC (permalink / raw)
To: Gao Xiang
Cc: linux-xfs, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen
On Tue, Mar 02, 2021 at 10:48:14AM +0800, Gao Xiang wrote:
> This patch introduces a helper to shrink unused space in the last AG
> by fixing up the freespace btree.
>
> Also make sure that the per-AG reservation works under the new AG
> size. If such per-AG reservation or extent allocation fails, roll
> the transaction so the new transaction could cancel without any side
> effects.
>
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
> fs/xfs/libxfs/xfs_ag.c | 108 +++++++++++++++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_ag.h | 2 +
> 2 files changed, 110 insertions(+)
>
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 9331f3516afa..a1128814630a 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -22,6 +22,11 @@
> #include "xfs_ag.h"
> #include "xfs_ag_resv.h"
> #include "xfs_health.h"
> +#include "xfs_error.h"
> +#include "xfs_bmap.h"
> +#include "xfs_defer.h"
> +#include "xfs_log_format.h"
> +#include "xfs_trans.h"
>
> static int
> xfs_get_aghdr_buf(
> @@ -485,6 +490,109 @@ xfs_ag_init_headers(
> return error;
> }
>
> +int
> +xfs_ag_shrink_space(
> + struct xfs_mount *mp,
> + struct xfs_trans **tpp,
> + xfs_agnumber_t agno,
> + xfs_extlen_t len)
> +{
> + struct xfs_alloc_arg args = {
> + .tp = *tpp,
> + .mp = mp,
> + .type = XFS_ALLOCTYPE_THIS_BNO,
> + .minlen = len,
> + .maxlen = len,
> + .oinfo = XFS_RMAP_OINFO_SKIP_UPDATE,
> + .resv = XFS_AG_RESV_NONE,
> + .prod = 1
> + };
> + struct xfs_buf *agibp, *agfbp;
> + struct xfs_agi *agi;
> + struct xfs_agf *agf;
> + int error, err2;
> +
> + ASSERT(agno == mp->m_sb.sb_agcount - 1);
> + error = xfs_ialloc_read_agi(mp, *tpp, agno, &agibp);
> + if (error)
> + return error;
> +
> + agi = agibp->b_addr;
> +
> + error = xfs_alloc_read_agf(mp, *tpp, agno, 0, &agfbp);
> + if (error)
> + return error;
> +
> + agf = agfbp->b_addr;
> + if (XFS_IS_CORRUPT(mp, agf->agf_length != agi->agi_length))
> + return -EFSCORRUPTED;
> +
> + args.fsbno = XFS_AGB_TO_FSB(mp, agno,
> + be32_to_cpu(agi->agi_length) - len);
Paranoia nit: Should we check that len < agi_length?
> +
> + /* remove the preallocations before allocation and re-establish then */
> + error = xfs_ag_resv_free(agibp->b_pag);
> + if (error)
> + return error;
> +
> + /* internal log shouldn't also show up in the free space btrees */
> + error = xfs_alloc_vextent(&args);
I forget, does xfs_alloc_vextent ever roll args.tp?
Other than those two things this looks good to me.
--D
> + if (!error && args.agbno == NULLAGBLOCK)
> + error = -ENOSPC;
> +
> + if (error) {
> + /*
> + * if extent allocation fails, need to roll the transaction to
> + * ensure that the AGFL fixup has been committed anyway.
> + */
> + err2 = xfs_trans_roll(tpp);
> + if (err2)
> + return err2;
> + goto resv_init_out;
> + }
> +
> + /*
> + * if successfully deleted from freespace btrees, need to confirm
> + * per-AG reservation works as expected.
> + */
> + be32_add_cpu(&agi->agi_length, -len);
> + be32_add_cpu(&agf->agf_length, -len);
> +
> + err2 = xfs_ag_resv_init(agibp->b_pag, *tpp);
> + if (err2) {
> + be32_add_cpu(&agi->agi_length, len);
> + be32_add_cpu(&agf->agf_length, len);
> + if (err2 != -ENOSPC)
> + goto resv_err;
> +
> + __xfs_bmap_add_free(*tpp, args.fsbno, len, NULL, true);
> +
> + /*
> + * Roll the transaction before trying to re-init the per-ag
> + * reservation. The new transaction is clean so it will cancel
> + * without any side effects.
> + */
> + error = xfs_defer_finish(tpp);
> + if (error)
> + return error;
> +
> + error = -ENOSPC;
> + goto resv_init_out;
> + }
> + xfs_ialloc_log_agi(*tpp, agibp, XFS_AGI_LENGTH);
> + xfs_alloc_log_agf(*tpp, agfbp, XFS_AGF_LENGTH);
> + return 0;
> +
> +resv_init_out:
> + err2 = xfs_ag_resv_init(agibp->b_pag, *tpp);
> + if (!err2)
> + return error;
> +resv_err:
> + xfs_warn(mp, "Error %d reserving per-AG metadata reserve pool.", err2);
> + xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> + return err2;
> +}
> +
> /*
> * Extent the AG indicated by the @id by the length passed in
> */
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 5166322807e7..f33388eb130a 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -24,6 +24,8 @@ struct aghdr_init_data {
> };
>
> int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
> +int xfs_ag_shrink_space(struct xfs_mount *mp, struct xfs_trans **tpp,
> + xfs_agnumber_t agno, xfs_extlen_t len);
> int xfs_ag_extend_space(struct xfs_mount *mp, struct xfs_trans *tp,
> struct aghdr_init_data *id, xfs_extlen_t len);
> int xfs_ag_get_geometry(struct xfs_mount *mp, xfs_agnumber_t agno,
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 3/5] xfs: introduce xfs_ag_shrink_space()
2021-03-03 18:19 ` Darrick J. Wong
@ 2021-03-03 23:16 ` Gao Xiang
0 siblings, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2021-03-03 23:16 UTC (permalink / raw)
To: Darrick J. Wong
Cc: linux-xfs, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen
On Wed, Mar 03, 2021 at 10:19:31AM -0800, Darrick J. Wong wrote:
> On Tue, Mar 02, 2021 at 10:48:14AM +0800, Gao Xiang wrote:
> > This patch introduces a helper to shrink unused space in the last AG
> > by fixing up the freespace btree.
> >
> > Also make sure that the per-AG reservation works under the new AG
> > size. If such per-AG reservation or extent allocation fails, roll
> > the transaction so the new transaction could cancel without any side
> > effects.
> >
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > ---
> > fs/xfs/libxfs/xfs_ag.c | 108 +++++++++++++++++++++++++++++++++++++++++
> > fs/xfs/libxfs/xfs_ag.h | 2 +
> > 2 files changed, 110 insertions(+)
> >
> > diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> > index 9331f3516afa..a1128814630a 100644
> > --- a/fs/xfs/libxfs/xfs_ag.c
> > +++ b/fs/xfs/libxfs/xfs_ag.c
> > @@ -22,6 +22,11 @@
> > #include "xfs_ag.h"
> > #include "xfs_ag_resv.h"
> > #include "xfs_health.h"
> > +#include "xfs_error.h"
> > +#include "xfs_bmap.h"
> > +#include "xfs_defer.h"
> > +#include "xfs_log_format.h"
> > +#include "xfs_trans.h"
> >
> > static int
> > xfs_get_aghdr_buf(
> > @@ -485,6 +490,109 @@ xfs_ag_init_headers(
> > return error;
> > }
> >
> > +int
> > +xfs_ag_shrink_space(
> > + struct xfs_mount *mp,
> > + struct xfs_trans **tpp,
> > + xfs_agnumber_t agno,
> > + xfs_extlen_t len)
> > +{
> > + struct xfs_alloc_arg args = {
> > + .tp = *tpp,
> > + .mp = mp,
> > + .type = XFS_ALLOCTYPE_THIS_BNO,
> > + .minlen = len,
> > + .maxlen = len,
> > + .oinfo = XFS_RMAP_OINFO_SKIP_UPDATE,
> > + .resv = XFS_AG_RESV_NONE,
> > + .prod = 1
> > + };
> > + struct xfs_buf *agibp, *agfbp;
> > + struct xfs_agi *agi;
> > + struct xfs_agf *agf;
> > + int error, err2;
> > +
> > + ASSERT(agno == mp->m_sb.sb_agcount - 1);
> > + error = xfs_ialloc_read_agi(mp, *tpp, agno, &agibp);
> > + if (error)
> > + return error;
> > +
> > + agi = agibp->b_addr;
> > +
> > + error = xfs_alloc_read_agf(mp, *tpp, agno, 0, &agfbp);
> > + if (error)
> > + return error;
> > +
> > + agf = agfbp->b_addr;
> > + if (XFS_IS_CORRUPT(mp, agf->agf_length != agi->agi_length))
> > + return -EFSCORRUPTED;
> > +
> > + args.fsbno = XFS_AGB_TO_FSB(mp, agno,
> > + be32_to_cpu(agi->agi_length) - len);
>
> Paranoia nit: Should we check that len < agi_length?
Ok, although (I think) that's impossible for the current only caller,
I could add some check in the next version.
>
> > +
> > + /* remove the preallocations before allocation and re-establish then */
> > + error = xfs_ag_resv_free(agibp->b_pag);
> > + if (error)
> > + return error;
> > +
> > + /* internal log shouldn't also show up in the free space btrees */
> > + error = xfs_alloc_vextent(&args);
>
> I forget, does xfs_alloc_vextent ever roll args.tp?
I think xfs_alloc_vextent will return a dirty transaction without
rolling instead.
Thanks,
Gao Xiang
>
> Other than those two things this looks good to me.
>
> --D
>
> > + if (!error && args.agbno == NULLAGBLOCK)
> > + error = -ENOSPC;
> > +
> > + if (error) {
> > + /*
> > + * if extent allocation fails, need to roll the transaction to
> > + * ensure that the AGFL fixup has been committed anyway.
> > + */
> > + err2 = xfs_trans_roll(tpp);
> > + if (err2)
> > + return err2;
> > + goto resv_init_out;
> > + }
> > +
> > + /*
> > + * if successfully deleted from freespace btrees, need to confirm
> > + * per-AG reservation works as expected.
> > + */
> > + be32_add_cpu(&agi->agi_length, -len);
> > + be32_add_cpu(&agf->agf_length, -len);
> > +
> > + err2 = xfs_ag_resv_init(agibp->b_pag, *tpp);
> > + if (err2) {
> > + be32_add_cpu(&agi->agi_length, len);
> > + be32_add_cpu(&agf->agf_length, len);
> > + if (err2 != -ENOSPC)
> > + goto resv_err;
> > +
> > + __xfs_bmap_add_free(*tpp, args.fsbno, len, NULL, true);
> > +
> > + /*
> > + * Roll the transaction before trying to re-init the per-ag
> > + * reservation. The new transaction is clean so it will cancel
> > + * without any side effects.
> > + */
> > + error = xfs_defer_finish(tpp);
> > + if (error)
> > + return error;
> > +
> > + error = -ENOSPC;
> > + goto resv_init_out;
> > + }
> > + xfs_ialloc_log_agi(*tpp, agibp, XFS_AGI_LENGTH);
> > + xfs_alloc_log_agf(*tpp, agfbp, XFS_AGF_LENGTH);
> > + return 0;
> > +
> > +resv_init_out:
> > + err2 = xfs_ag_resv_init(agibp->b_pag, *tpp);
> > + if (!err2)
> > + return error;
> > +resv_err:
> > + xfs_warn(mp, "Error %d reserving per-AG metadata reserve pool.", err2);
> > + xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> > + return err2;
> > +}
> > +
> > /*
> > * Extent the AG indicated by the @id by the length passed in
> > */
> > diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> > index 5166322807e7..f33388eb130a 100644
> > --- a/fs/xfs/libxfs/xfs_ag.h
> > +++ b/fs/xfs/libxfs/xfs_ag.h
> > @@ -24,6 +24,8 @@ struct aghdr_init_data {
> > };
> >
> > int xfs_ag_init_headers(struct xfs_mount *mp, struct aghdr_init_data *id);
> > +int xfs_ag_shrink_space(struct xfs_mount *mp, struct xfs_trans **tpp,
> > + xfs_agnumber_t agno, xfs_extlen_t len);
> > int xfs_ag_extend_space(struct xfs_mount *mp, struct xfs_trans *tp,
> > struct aghdr_init_data *id, xfs_extlen_t len);
> > int xfs_ag_get_geometry(struct xfs_mount *mp, xfs_agnumber_t agno,
> > --
> > 2.27.0
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 4/5] xfs: support shrinking unused space in the last AG
2021-03-02 2:48 [PATCH v7 0/5] xfs: support shrinking free space in the last AG Gao Xiang
` (2 preceding siblings ...)
2021-03-02 2:48 ` [PATCH v7 3/5] xfs: introduce xfs_ag_shrink_space() Gao Xiang
@ 2021-03-02 2:48 ` Gao Xiang
2021-03-03 18:25 ` Darrick J. Wong
2021-03-02 2:48 ` [PATCH v7 5/5] xfs: add error injection for per-AG resv failure Gao Xiang
4 siblings, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2021-03-02 2:48 UTC (permalink / raw)
To: linux-xfs
Cc: Darrick J. Wong, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen, Gao Xiang
As the first step of shrinking, this attempts to enable shrinking
unused space in the last allocation group by fixing up freespace
btree, agi, agf and adjusting super block and use a helper
xfs_ag_shrink_space() to fixup the last AG.
This can be all done in one transaction for now, so I think no
additional protection is needed.
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
fs/xfs/xfs_fsops.c | 90 ++++++++++++++++++++++++++++------------------
fs/xfs/xfs_trans.c | 1 -
2 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 494f9354e3fb..204c96d0010f 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -90,23 +90,29 @@ xfs_growfs_data_private(
int error;
xfs_agnumber_t nagcount;
xfs_agnumber_t nagimax = 0;
- xfs_rfsblock_t nb, nb_div, nb_mod, delta;
+ xfs_rfsblock_t nb, nb_div, nb_mod;
+ int64_t delta;
bool lastag_resetagres;
xfs_agnumber_t oagcount;
struct xfs_trans *tp;
struct aghdr_init_data id = {};
nb = in->newblocks;
- if (nb < mp->m_sb.sb_dblocks)
- return -EINVAL;
- if ((error = xfs_sb_validate_fsb_count(&mp->m_sb, nb)))
+ if (nb == mp->m_sb.sb_dblocks)
+ return 0;
+
+ error = xfs_sb_validate_fsb_count(&mp->m_sb, nb);
+ if (error)
return error;
- error = xfs_buf_read_uncached(mp->m_ddev_targp,
+
+ if (nb > mp->m_sb.sb_dblocks) {
+ error = xfs_buf_read_uncached(mp->m_ddev_targp,
XFS_FSB_TO_BB(mp, nb) - XFS_FSS_TO_BB(mp, 1),
XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
- if (error)
- return error;
- xfs_buf_relse(bp);
+ if (error)
+ return error;
+ xfs_buf_relse(bp);
+ }
nb_div = nb;
nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks);
@@ -114,10 +120,15 @@ xfs_growfs_data_private(
if (nb_mod && nb_mod < XFS_MIN_AG_BLOCKS) {
nagcount--;
nb = (xfs_rfsblock_t)nagcount * mp->m_sb.sb_agblocks;
- if (nb < mp->m_sb.sb_dblocks)
- return -EINVAL;
}
delta = nb - mp->m_sb.sb_dblocks;
+ /*
+ * XFS doesn't really support single-AG filesystems, so do not
+ * permit callers to remove the filesystem's second and last AG.
+ */
+ if (delta < 0 && nagcount < 2)
+ return -EINVAL;
+
oagcount = mp->m_sb.sb_agcount;
/* allocate the new per-ag structures */
@@ -125,15 +136,23 @@ xfs_growfs_data_private(
error = xfs_initialize_perag(mp, nagcount, &nagimax);
if (error)
return error;
+ } else if (nagcount < oagcount) {
+ /* TODO: shrinking the entire AGs hasn't yet completed */
+ return -EINVAL;
}
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata,
- XFS_GROWFS_SPACE_RES(mp), 0, XFS_TRANS_RESERVE, &tp);
+ (delta > 0 ? XFS_GROWFS_SPACE_RES(mp) : -delta), 0,
+ XFS_TRANS_RESERVE, &tp);
if (error)
return error;
- error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount,
- delta, &lastag_resetagres);
+ if (delta > 0)
+ error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount,
+ delta, &lastag_resetagres);
+ else
+ error = xfs_ag_shrink_space(mp, &tp, nagcount - 1, -delta);
+
if (error)
goto out_trans_cancel;
@@ -144,7 +163,7 @@ xfs_growfs_data_private(
*/
if (nagcount > oagcount)
xfs_trans_mod_sb(tp, XFS_TRANS_SB_AGCOUNT, nagcount - oagcount);
- if (delta > 0)
+ if (delta)
xfs_trans_mod_sb(tp, XFS_TRANS_SB_DBLOCKS, delta);
if (id.nfree)
xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree);
@@ -168,28 +187,29 @@ xfs_growfs_data_private(
xfs_set_low_space_thresholds(mp);
mp->m_alloc_set_aside = xfs_alloc_set_aside(mp);
- /*
- * If we expanded the last AG, free the per-AG reservation
- * so we can reinitialize it with the new size.
- */
- if (lastag_resetagres) {
- struct xfs_perag *pag;
-
- pag = xfs_perag_get(mp, id.agno);
- error = xfs_ag_resv_free(pag);
- xfs_perag_put(pag);
- if (error)
- return error;
+ if (delta > 0) {
+ /*
+ * If we expanded the last AG, free the per-AG reservation
+ * so we can reinitialize it with the new size.
+ */
+ if (lastag_resetagres) {
+ struct xfs_perag *pag;
+
+ pag = xfs_perag_get(mp, id.agno);
+ error = xfs_ag_resv_free(pag);
+ xfs_perag_put(pag);
+ if (error)
+ return error;
+ }
+ /*
+ * Reserve AG metadata blocks. ENOSPC here does not mean there
+ * was a growfs failure, just that there still isn't space for
+ * new user data after the grow has been run.
+ */
+ error = xfs_fs_reserve_ag_blocks(mp);
+ if (error == -ENOSPC)
+ error = 0;
}
-
- /*
- * Reserve AG metadata blocks. ENOSPC here does not mean there was a
- * growfs failure, just that there still isn't space for new user data
- * after the grow has been run.
- */
- error = xfs_fs_reserve_ag_blocks(mp);
- if (error == -ENOSPC)
- error = 0;
return error;
out_trans_cancel:
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 44f72c09c203..d047f5f26cc0 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -434,7 +434,6 @@ xfs_trans_mod_sb(
tp->t_res_frextents_delta += delta;
break;
case XFS_TRANS_SB_DBLOCKS:
- ASSERT(delta > 0);
tp->t_dblocks_delta += delta;
break;
case XFS_TRANS_SB_AGCOUNT:
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 4/5] xfs: support shrinking unused space in the last AG
2021-03-02 2:48 ` [PATCH v7 4/5] xfs: support shrinking unused space in the last AG Gao Xiang
@ 2021-03-03 18:25 ` Darrick J. Wong
2021-03-03 23:19 ` Gao Xiang
0 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2021-03-03 18:25 UTC (permalink / raw)
To: Gao Xiang
Cc: linux-xfs, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen
On Tue, Mar 02, 2021 at 10:48:15AM +0800, Gao Xiang wrote:
> As the first step of shrinking, this attempts to enable shrinking
> unused space in the last allocation group by fixing up freespace
> btree, agi, agf and adjusting super block and use a helper
> xfs_ag_shrink_space() to fixup the last AG.
>
> This can be all done in one transaction for now, so I think no
> additional protection is needed.
>
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
> fs/xfs/xfs_fsops.c | 90 ++++++++++++++++++++++++++++------------------
> fs/xfs/xfs_trans.c | 1 -
> 2 files changed, 55 insertions(+), 36 deletions(-)
>
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 494f9354e3fb..204c96d0010f 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -90,23 +90,29 @@ xfs_growfs_data_private(
> int error;
> xfs_agnumber_t nagcount;
> xfs_agnumber_t nagimax = 0;
> - xfs_rfsblock_t nb, nb_div, nb_mod, delta;
> + xfs_rfsblock_t nb, nb_div, nb_mod;
> + int64_t delta;
> bool lastag_resetagres;
> xfs_agnumber_t oagcount;
> struct xfs_trans *tp;
> struct aghdr_init_data id = {};
>
> nb = in->newblocks;
> - if (nb < mp->m_sb.sb_dblocks)
> - return -EINVAL;
> - if ((error = xfs_sb_validate_fsb_count(&mp->m_sb, nb)))
> + if (nb == mp->m_sb.sb_dblocks)
> + return 0;
> +
> + error = xfs_sb_validate_fsb_count(&mp->m_sb, nb);
> + if (error)
> return error;
> - error = xfs_buf_read_uncached(mp->m_ddev_targp,
> +
> + if (nb > mp->m_sb.sb_dblocks) {
> + error = xfs_buf_read_uncached(mp->m_ddev_targp,
> XFS_FSB_TO_BB(mp, nb) - XFS_FSS_TO_BB(mp, 1),
> XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
> - if (error)
> - return error;
> - xfs_buf_relse(bp);
> + if (error)
> + return error;
> + xfs_buf_relse(bp);
> + }
>
> nb_div = nb;
> nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks);
> @@ -114,10 +120,15 @@ xfs_growfs_data_private(
> if (nb_mod && nb_mod < XFS_MIN_AG_BLOCKS) {
> nagcount--;
> nb = (xfs_rfsblock_t)nagcount * mp->m_sb.sb_agblocks;
> - if (nb < mp->m_sb.sb_dblocks)
> - return -EINVAL;
> }
> delta = nb - mp->m_sb.sb_dblocks;
> + /*
> + * XFS doesn't really support single-AG filesystems, so do not
> + * permit callers to remove the filesystem's second and last AG.
> + */
> + if (delta < 0 && nagcount < 2)
> + return -EINVAL;
> +
> oagcount = mp->m_sb.sb_agcount;
>
> /* allocate the new per-ag structures */
> @@ -125,15 +136,23 @@ xfs_growfs_data_private(
> error = xfs_initialize_perag(mp, nagcount, &nagimax);
> if (error)
> return error;
> + } else if (nagcount < oagcount) {
> + /* TODO: shrinking the entire AGs hasn't yet completed */
> + return -EINVAL;
> }
>
> error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata,
> - XFS_GROWFS_SPACE_RES(mp), 0, XFS_TRANS_RESERVE, &tp);
> + (delta > 0 ? XFS_GROWFS_SPACE_RES(mp) : -delta), 0,
> + XFS_TRANS_RESERVE, &tp);
> if (error)
> return error;
>
> - error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount,
> - delta, &lastag_resetagres);
> + if (delta > 0)
> + error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount,
> + delta, &lastag_resetagres);
> + else
> + error = xfs_ag_shrink_space(mp, &tp, nagcount - 1, -delta);
Hm, looking back at the previous patch, I think the last argument to
this function should be named "shrink_len" (or maybe just delta?) so
that future readers don't confuse it for the new (shorter) AG length.
> +
Nit: no blank line needed here.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> if (error)
> goto out_trans_cancel;
>
> @@ -144,7 +163,7 @@ xfs_growfs_data_private(
> */
> if (nagcount > oagcount)
> xfs_trans_mod_sb(tp, XFS_TRANS_SB_AGCOUNT, nagcount - oagcount);
> - if (delta > 0)
> + if (delta)
> xfs_trans_mod_sb(tp, XFS_TRANS_SB_DBLOCKS, delta);
> if (id.nfree)
> xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree);
> @@ -168,28 +187,29 @@ xfs_growfs_data_private(
> xfs_set_low_space_thresholds(mp);
> mp->m_alloc_set_aside = xfs_alloc_set_aside(mp);
>
> - /*
> - * If we expanded the last AG, free the per-AG reservation
> - * so we can reinitialize it with the new size.
> - */
> - if (lastag_resetagres) {
> - struct xfs_perag *pag;
> -
> - pag = xfs_perag_get(mp, id.agno);
> - error = xfs_ag_resv_free(pag);
> - xfs_perag_put(pag);
> - if (error)
> - return error;
> + if (delta > 0) {
> + /*
> + * If we expanded the last AG, free the per-AG reservation
> + * so we can reinitialize it with the new size.
> + */
> + if (lastag_resetagres) {
> + struct xfs_perag *pag;
> +
> + pag = xfs_perag_get(mp, id.agno);
> + error = xfs_ag_resv_free(pag);
> + xfs_perag_put(pag);
> + if (error)
> + return error;
> + }
> + /*
> + * Reserve AG metadata blocks. ENOSPC here does not mean there
> + * was a growfs failure, just that there still isn't space for
> + * new user data after the grow has been run.
> + */
> + error = xfs_fs_reserve_ag_blocks(mp);
> + if (error == -ENOSPC)
> + error = 0;
> }
> -
> - /*
> - * Reserve AG metadata blocks. ENOSPC here does not mean there was a
> - * growfs failure, just that there still isn't space for new user data
> - * after the grow has been run.
> - */
> - error = xfs_fs_reserve_ag_blocks(mp);
> - if (error == -ENOSPC)
> - error = 0;
> return error;
>
> out_trans_cancel:
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 44f72c09c203..d047f5f26cc0 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -434,7 +434,6 @@ xfs_trans_mod_sb(
> tp->t_res_frextents_delta += delta;
> break;
> case XFS_TRANS_SB_DBLOCKS:
> - ASSERT(delta > 0);
> tp->t_dblocks_delta += delta;
> break;
> case XFS_TRANS_SB_AGCOUNT:
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 4/5] xfs: support shrinking unused space in the last AG
2021-03-03 18:25 ` Darrick J. Wong
@ 2021-03-03 23:19 ` Gao Xiang
0 siblings, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2021-03-03 23:19 UTC (permalink / raw)
To: Darrick J. Wong
Cc: linux-xfs, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen
On Wed, Mar 03, 2021 at 10:25:27AM -0800, Darrick J. Wong wrote:
> On Tue, Mar 02, 2021 at 10:48:15AM +0800, Gao Xiang wrote:
> > As the first step of shrinking, this attempts to enable shrinking
> > unused space in the last allocation group by fixing up freespace
> > btree, agi, agf and adjusting super block and use a helper
> > xfs_ag_shrink_space() to fixup the last AG.
> >
> > This can be all done in one transaction for now, so I think no
> > additional protection is needed.
> >
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > ---
> > fs/xfs/xfs_fsops.c | 90 ++++++++++++++++++++++++++++------------------
> > fs/xfs/xfs_trans.c | 1 -
> > 2 files changed, 55 insertions(+), 36 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > index 494f9354e3fb..204c96d0010f 100644
> > --- a/fs/xfs/xfs_fsops.c
> > +++ b/fs/xfs/xfs_fsops.c
> > @@ -90,23 +90,29 @@ xfs_growfs_data_private(
> > int error;
> > xfs_agnumber_t nagcount;
> > xfs_agnumber_t nagimax = 0;
> > - xfs_rfsblock_t nb, nb_div, nb_mod, delta;
> > + xfs_rfsblock_t nb, nb_div, nb_mod;
> > + int64_t delta;
> > bool lastag_resetagres;
> > xfs_agnumber_t oagcount;
> > struct xfs_trans *tp;
> > struct aghdr_init_data id = {};
> >
> > nb = in->newblocks;
> > - if (nb < mp->m_sb.sb_dblocks)
> > - return -EINVAL;
> > - if ((error = xfs_sb_validate_fsb_count(&mp->m_sb, nb)))
> > + if (nb == mp->m_sb.sb_dblocks)
> > + return 0;
> > +
> > + error = xfs_sb_validate_fsb_count(&mp->m_sb, nb);
> > + if (error)
> > return error;
> > - error = xfs_buf_read_uncached(mp->m_ddev_targp,
> > +
> > + if (nb > mp->m_sb.sb_dblocks) {
> > + error = xfs_buf_read_uncached(mp->m_ddev_targp,
> > XFS_FSB_TO_BB(mp, nb) - XFS_FSS_TO_BB(mp, 1),
> > XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
> > - if (error)
> > - return error;
> > - xfs_buf_relse(bp);
> > + if (error)
> > + return error;
> > + xfs_buf_relse(bp);
> > + }
> >
> > nb_div = nb;
> > nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks);
> > @@ -114,10 +120,15 @@ xfs_growfs_data_private(
> > if (nb_mod && nb_mod < XFS_MIN_AG_BLOCKS) {
> > nagcount--;
> > nb = (xfs_rfsblock_t)nagcount * mp->m_sb.sb_agblocks;
> > - if (nb < mp->m_sb.sb_dblocks)
> > - return -EINVAL;
> > }
> > delta = nb - mp->m_sb.sb_dblocks;
> > + /*
> > + * XFS doesn't really support single-AG filesystems, so do not
> > + * permit callers to remove the filesystem's second and last AG.
> > + */
> > + if (delta < 0 && nagcount < 2)
> > + return -EINVAL;
> > +
> > oagcount = mp->m_sb.sb_agcount;
> >
> > /* allocate the new per-ag structures */
> > @@ -125,15 +136,23 @@ xfs_growfs_data_private(
> > error = xfs_initialize_perag(mp, nagcount, &nagimax);
> > if (error)
> > return error;
> > + } else if (nagcount < oagcount) {
> > + /* TODO: shrinking the entire AGs hasn't yet completed */
> > + return -EINVAL;
> > }
> >
> > error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata,
> > - XFS_GROWFS_SPACE_RES(mp), 0, XFS_TRANS_RESERVE, &tp);
> > + (delta > 0 ? XFS_GROWFS_SPACE_RES(mp) : -delta), 0,
> > + XFS_TRANS_RESERVE, &tp);
> > if (error)
> > return error;
> >
> > - error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount,
> > - delta, &lastag_resetagres);
> > + if (delta > 0)
> > + error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount,
> > + delta, &lastag_resetagres);
> > + else
> > + error = xfs_ag_shrink_space(mp, &tp, nagcount - 1, -delta);
>
> Hm, looking back at the previous patch, I think the last argument to
> this function should be named "shrink_len" (or maybe just delta?) so
> that future readers don't confuse it for the new (shorter) AG length.
>
Ok, will update in the next version.
> > +
>
> Nit: no blank line needed here.
>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Thanks!
Thanks,
Gao Xiang
>
> --D
>
> > if (error)
> > goto out_trans_cancel;
> >
> > @@ -144,7 +163,7 @@ xfs_growfs_data_private(
> > */
> > if (nagcount > oagcount)
> > xfs_trans_mod_sb(tp, XFS_TRANS_SB_AGCOUNT, nagcount - oagcount);
> > - if (delta > 0)
> > + if (delta)
> > xfs_trans_mod_sb(tp, XFS_TRANS_SB_DBLOCKS, delta);
> > if (id.nfree)
> > xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree);
> > @@ -168,28 +187,29 @@ xfs_growfs_data_private(
> > xfs_set_low_space_thresholds(mp);
> > mp->m_alloc_set_aside = xfs_alloc_set_aside(mp);
> >
> > - /*
> > - * If we expanded the last AG, free the per-AG reservation
> > - * so we can reinitialize it with the new size.
> > - */
> > - if (lastag_resetagres) {
> > - struct xfs_perag *pag;
> > -
> > - pag = xfs_perag_get(mp, id.agno);
> > - error = xfs_ag_resv_free(pag);
> > - xfs_perag_put(pag);
> > - if (error)
> > - return error;
> > + if (delta > 0) {
> > + /*
> > + * If we expanded the last AG, free the per-AG reservation
> > + * so we can reinitialize it with the new size.
> > + */
> > + if (lastag_resetagres) {
> > + struct xfs_perag *pag;
> > +
> > + pag = xfs_perag_get(mp, id.agno);
> > + error = xfs_ag_resv_free(pag);
> > + xfs_perag_put(pag);
> > + if (error)
> > + return error;
> > + }
> > + /*
> > + * Reserve AG metadata blocks. ENOSPC here does not mean there
> > + * was a growfs failure, just that there still isn't space for
> > + * new user data after the grow has been run.
> > + */
> > + error = xfs_fs_reserve_ag_blocks(mp);
> > + if (error == -ENOSPC)
> > + error = 0;
> > }
> > -
> > - /*
> > - * Reserve AG metadata blocks. ENOSPC here does not mean there was a
> > - * growfs failure, just that there still isn't space for new user data
> > - * after the grow has been run.
> > - */
> > - error = xfs_fs_reserve_ag_blocks(mp);
> > - if (error == -ENOSPC)
> > - error = 0;
> > return error;
> >
> > out_trans_cancel:
> > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> > index 44f72c09c203..d047f5f26cc0 100644
> > --- a/fs/xfs/xfs_trans.c
> > +++ b/fs/xfs/xfs_trans.c
> > @@ -434,7 +434,6 @@ xfs_trans_mod_sb(
> > tp->t_res_frextents_delta += delta;
> > break;
> > case XFS_TRANS_SB_DBLOCKS:
> > - ASSERT(delta > 0);
> > tp->t_dblocks_delta += delta;
> > break;
> > case XFS_TRANS_SB_AGCOUNT:
> > --
> > 2.27.0
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 5/5] xfs: add error injection for per-AG resv failure
2021-03-02 2:48 [PATCH v7 0/5] xfs: support shrinking free space in the last AG Gao Xiang
` (3 preceding siblings ...)
2021-03-02 2:48 ` [PATCH v7 4/5] xfs: support shrinking unused space in the last AG Gao Xiang
@ 2021-03-02 2:48 ` Gao Xiang
2021-03-03 0:02 ` [PATCH v7.1 " Gao Xiang
2021-03-03 18:30 ` [PATCH v7 " Darrick J. Wong
4 siblings, 2 replies; 14+ messages in thread
From: Gao Xiang @ 2021-03-02 2:48 UTC (permalink / raw)
To: linux-xfs
Cc: Darrick J. Wong, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen, Gao Xiang
per-AG resv failure after fixing up freespace is hard to test in an
effective way, so directly add an error injection path to observe
such error handling path works as expected.
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
fs/xfs/libxfs/xfs_ag_resv.c | 6 +++++-
fs/xfs/libxfs/xfs_errortag.h | 4 +++-
fs/xfs/xfs_error.c | 2 ++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
index fdfe6dc0d307..6c5f8d10589c 100644
--- a/fs/xfs/libxfs/xfs_ag_resv.c
+++ b/fs/xfs/libxfs/xfs_ag_resv.c
@@ -211,7 +211,11 @@ __xfs_ag_resv_init(
ASSERT(0);
return -EINVAL;
}
- error = xfs_mod_fdblocks(mp, -(int64_t)hidden_space, true);
+
+ if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_AG_RESV_FAIL))
+ error = -ENOSPC;
+ else
+ error = xfs_mod_fdblocks(mp, -(int64_t)hidden_space, true);
if (error) {
trace_xfs_ag_resv_init_error(pag->pag_mount, pag->pag_agno,
error, _RET_IP_);
diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index 6ca9084b6934..b433ef735217 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -40,6 +40,7 @@
#define XFS_ERRTAG_REFCOUNT_FINISH_ONE 25
#define XFS_ERRTAG_BMAP_FINISH_ONE 26
#define XFS_ERRTAG_AG_RESV_CRITICAL 27
+
/*
* DEBUG mode instrumentation to test and/or trigger delayed allocation
* block killing in the event of failed writes. When enabled, all
@@ -58,7 +59,8 @@
#define XFS_ERRTAG_BUF_IOERROR 35
#define XFS_ERRTAG_REDUCE_MAX_IEXTENTS 36
#define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT 37
-#define XFS_ERRTAG_MAX 38
+#define XFS_ERRTAG_AG_RESV_FAIL 38
+#define XFS_ERRTAG_MAX 39
/*
* Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 185b4915b7bf..5192a7063d95 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -168,6 +168,7 @@ XFS_ERRORTAG_ATTR_RW(iunlink_fallback, XFS_ERRTAG_IUNLINK_FALLBACK);
XFS_ERRORTAG_ATTR_RW(buf_ioerror, XFS_ERRTAG_BUF_IOERROR);
XFS_ERRORTAG_ATTR_RW(reduce_max_iextents, XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
XFS_ERRORTAG_ATTR_RW(bmap_alloc_minlen_extent, XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT);
+XFS_ERRORTAG_ATTR_RW(ag_resv_fail, XFS_ERRTAG_AG_RESV_FAIL);
static struct attribute *xfs_errortag_attrs[] = {
XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -208,6 +209,7 @@ static struct attribute *xfs_errortag_attrs[] = {
XFS_ERRORTAG_ATTR_LIST(buf_ioerror),
XFS_ERRORTAG_ATTR_LIST(reduce_max_iextents),
XFS_ERRORTAG_ATTR_LIST(bmap_alloc_minlen_extent),
+ XFS_ERRORTAG_ATTR_LIST(ag_resv_fail),
NULL,
};
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v7.1 5/5] xfs: add error injection for per-AG resv failure
2021-03-02 2:48 ` [PATCH v7 5/5] xfs: add error injection for per-AG resv failure Gao Xiang
@ 2021-03-03 0:02 ` Gao Xiang
2021-03-03 18:30 ` [PATCH v7 " Darrick J. Wong
1 sibling, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2021-03-03 0:02 UTC (permalink / raw)
To: linux-xfs
Cc: Darrick J. Wong, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen, Gao Xiang
per-AG resv failure after fixing up freespace is hard to test in an
effective way, so directly add an error injection path to observe
such error handling path works as expected.
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
changes since v7:
- add missing random errortag reported by 0day CI and smatch.
fs/xfs/libxfs/xfs_ag_resv.c | 6 +++++-
fs/xfs/libxfs/xfs_errortag.h | 4 +++-
fs/xfs/xfs_error.c | 3 +++
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
index fdfe6dc0d307..6c5f8d10589c 100644
--- a/fs/xfs/libxfs/xfs_ag_resv.c
+++ b/fs/xfs/libxfs/xfs_ag_resv.c
@@ -211,7 +211,11 @@ __xfs_ag_resv_init(
ASSERT(0);
return -EINVAL;
}
- error = xfs_mod_fdblocks(mp, -(int64_t)hidden_space, true);
+
+ if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_AG_RESV_FAIL))
+ error = -ENOSPC;
+ else
+ error = xfs_mod_fdblocks(mp, -(int64_t)hidden_space, true);
if (error) {
trace_xfs_ag_resv_init_error(pag->pag_mount, pag->pag_agno,
error, _RET_IP_);
diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index 6ca9084b6934..a23a52e643ad 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -58,7 +58,8 @@
#define XFS_ERRTAG_BUF_IOERROR 35
#define XFS_ERRTAG_REDUCE_MAX_IEXTENTS 36
#define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT 37
-#define XFS_ERRTAG_MAX 38
+#define XFS_ERRTAG_AG_RESV_FAIL 38
+#define XFS_ERRTAG_MAX 39
/*
* Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -101,5 +102,6 @@
#define XFS_RANDOM_BUF_IOERROR XFS_RANDOM_DEFAULT
#define XFS_RANDOM_REDUCE_MAX_IEXTENTS 1
#define XFS_RANDOM_BMAP_ALLOC_MINLEN_EXTENT 1
+#define XFS_RANDOM_AG_RESV_FAIL 1
#endif /* __XFS_ERRORTAG_H_ */
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 185b4915b7bf..f70984f3174d 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -56,6 +56,7 @@ static unsigned int xfs_errortag_random_default[] = {
XFS_RANDOM_BUF_IOERROR,
XFS_RANDOM_REDUCE_MAX_IEXTENTS,
XFS_RANDOM_BMAP_ALLOC_MINLEN_EXTENT,
+ XFS_RANDOM_AG_RESV_FAIL,
};
struct xfs_errortag_attr {
@@ -168,6 +169,7 @@ XFS_ERRORTAG_ATTR_RW(iunlink_fallback, XFS_ERRTAG_IUNLINK_FALLBACK);
XFS_ERRORTAG_ATTR_RW(buf_ioerror, XFS_ERRTAG_BUF_IOERROR);
XFS_ERRORTAG_ATTR_RW(reduce_max_iextents, XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
XFS_ERRORTAG_ATTR_RW(bmap_alloc_minlen_extent, XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT);
+XFS_ERRORTAG_ATTR_RW(ag_resv_fail, XFS_ERRTAG_AG_RESV_FAIL);
static struct attribute *xfs_errortag_attrs[] = {
XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -208,6 +210,7 @@ static struct attribute *xfs_errortag_attrs[] = {
XFS_ERRORTAG_ATTR_LIST(buf_ioerror),
XFS_ERRORTAG_ATTR_LIST(reduce_max_iextents),
XFS_ERRORTAG_ATTR_LIST(bmap_alloc_minlen_extent),
+ XFS_ERRORTAG_ATTR_LIST(ag_resv_fail),
NULL,
};
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 5/5] xfs: add error injection for per-AG resv failure
2021-03-02 2:48 ` [PATCH v7 5/5] xfs: add error injection for per-AG resv failure Gao Xiang
2021-03-03 0:02 ` [PATCH v7.1 " Gao Xiang
@ 2021-03-03 18:30 ` Darrick J. Wong
2021-03-03 23:11 ` Gao Xiang
1 sibling, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2021-03-03 18:30 UTC (permalink / raw)
To: Gao Xiang
Cc: linux-xfs, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen
On Tue, Mar 02, 2021 at 10:48:16AM +0800, Gao Xiang wrote:
> per-AG resv failure after fixing up freespace is hard to test in an
> effective way, so directly add an error injection path to observe
> such error handling path works as expected.
>
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
> fs/xfs/libxfs/xfs_ag_resv.c | 6 +++++-
> fs/xfs/libxfs/xfs_errortag.h | 4 +++-
> fs/xfs/xfs_error.c | 2 ++
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
> index fdfe6dc0d307..6c5f8d10589c 100644
> --- a/fs/xfs/libxfs/xfs_ag_resv.c
> +++ b/fs/xfs/libxfs/xfs_ag_resv.c
> @@ -211,7 +211,11 @@ __xfs_ag_resv_init(
> ASSERT(0);
> return -EINVAL;
> }
> - error = xfs_mod_fdblocks(mp, -(int64_t)hidden_space, true);
> +
> + if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_AG_RESV_FAIL))
> + error = -ENOSPC;
> + else
> + error = xfs_mod_fdblocks(mp, -(int64_t)hidden_space, true);
> if (error) {
> trace_xfs_ag_resv_init_error(pag->pag_mount, pag->pag_agno,
> error, _RET_IP_);
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 6ca9084b6934..b433ef735217 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -40,6 +40,7 @@
> #define XFS_ERRTAG_REFCOUNT_FINISH_ONE 25
> #define XFS_ERRTAG_BMAP_FINISH_ONE 26
> #define XFS_ERRTAG_AG_RESV_CRITICAL 27
> +
Extra space?
> /*
> * DEBUG mode instrumentation to test and/or trigger delayed allocation
> * block killing in the event of failed writes. When enabled, all
> @@ -58,7 +59,8 @@
> #define XFS_ERRTAG_BUF_IOERROR 35
> #define XFS_ERRTAG_REDUCE_MAX_IEXTENTS 36
> #define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT 37
> -#define XFS_ERRTAG_MAX 38
> +#define XFS_ERRTAG_AG_RESV_FAIL 38
> +#define XFS_ERRTAG_MAX 39
>
> /*
> * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
This needs to define XFS_RANDOM_AG_RESV_FAIL and put it in
xfs_errortag_random_default in xfs_error.c to avoid running off the end
of the array.
Also... that _default array /really/ needs to have a BUILD_BUG_ON
somewhere to scream loudly if it isn't XFS_ERRTAG_MAX elements long.
--D
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 185b4915b7bf..5192a7063d95 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -168,6 +168,7 @@ XFS_ERRORTAG_ATTR_RW(iunlink_fallback, XFS_ERRTAG_IUNLINK_FALLBACK);
> XFS_ERRORTAG_ATTR_RW(buf_ioerror, XFS_ERRTAG_BUF_IOERROR);
> XFS_ERRORTAG_ATTR_RW(reduce_max_iextents, XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
> XFS_ERRORTAG_ATTR_RW(bmap_alloc_minlen_extent, XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT);
> +XFS_ERRORTAG_ATTR_RW(ag_resv_fail, XFS_ERRTAG_AG_RESV_FAIL);
>
> static struct attribute *xfs_errortag_attrs[] = {
> XFS_ERRORTAG_ATTR_LIST(noerror),
> @@ -208,6 +209,7 @@ static struct attribute *xfs_errortag_attrs[] = {
> XFS_ERRORTAG_ATTR_LIST(buf_ioerror),
> XFS_ERRORTAG_ATTR_LIST(reduce_max_iextents),
> XFS_ERRORTAG_ATTR_LIST(bmap_alloc_minlen_extent),
> + XFS_ERRORTAG_ATTR_LIST(ag_resv_fail),
> NULL,
> };
>
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 5/5] xfs: add error injection for per-AG resv failure
2021-03-03 18:30 ` [PATCH v7 " Darrick J. Wong
@ 2021-03-03 23:11 ` Gao Xiang
0 siblings, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2021-03-03 23:11 UTC (permalink / raw)
To: Darrick J. Wong
Cc: linux-xfs, Brian Foster, Dave Chinner, Christoph Hellwig,
Eric Sandeen
Hi Darrick,
On Wed, Mar 03, 2021 at 10:30:42AM -0800, Darrick J. Wong wrote:
> On Tue, Mar 02, 2021 at 10:48:16AM +0800, Gao Xiang wrote:
> > per-AG resv failure after fixing up freespace is hard to test in an
> > effective way, so directly add an error injection path to observe
> > such error handling path works as expected.
> >
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > ---
> > fs/xfs/libxfs/xfs_ag_resv.c | 6 +++++-
> > fs/xfs/libxfs/xfs_errortag.h | 4 +++-
> > fs/xfs/xfs_error.c | 2 ++
> > 3 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
> > index fdfe6dc0d307..6c5f8d10589c 100644
> > --- a/fs/xfs/libxfs/xfs_ag_resv.c
> > +++ b/fs/xfs/libxfs/xfs_ag_resv.c
> > @@ -211,7 +211,11 @@ __xfs_ag_resv_init(
> > ASSERT(0);
> > return -EINVAL;
> > }
> > - error = xfs_mod_fdblocks(mp, -(int64_t)hidden_space, true);
> > +
> > + if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_AG_RESV_FAIL))
> > + error = -ENOSPC;
> > + else
> > + error = xfs_mod_fdblocks(mp, -(int64_t)hidden_space, true);
> > if (error) {
> > trace_xfs_ag_resv_init_error(pag->pag_mount, pag->pag_agno,
> > error, _RET_IP_);
> > diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> > index 6ca9084b6934..b433ef735217 100644
> > --- a/fs/xfs/libxfs/xfs_errortag.h
> > +++ b/fs/xfs/libxfs/xfs_errortag.h
> > @@ -40,6 +40,7 @@
> > #define XFS_ERRTAG_REFCOUNT_FINISH_ONE 25
> > #define XFS_ERRTAG_BMAP_FINISH_ONE 26
> > #define XFS_ERRTAG_AG_RESV_CRITICAL 27
> > +
>
> Extra space?
>
> > /*
> > * DEBUG mode instrumentation to test and/or trigger delayed allocation
> > * block killing in the event of failed writes. When enabled, all
> > @@ -58,7 +59,8 @@
> > #define XFS_ERRTAG_BUF_IOERROR 35
> > #define XFS_ERRTAG_REDUCE_MAX_IEXTENTS 36
> > #define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT 37
> > -#define XFS_ERRTAG_MAX 38
> > +#define XFS_ERRTAG_AG_RESV_FAIL 38
> > +#define XFS_ERRTAG_MAX 39
> >
> > /*
> > * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
>
> This needs to define XFS_RANDOM_AG_RESV_FAIL and put it in
> xfs_errortag_random_default in xfs_error.c to avoid running off the end
> of the array.
>
> Also... that _default array /really/ needs to have a BUILD_BUG_ON
> somewhere to scream loudly if it isn't XFS_ERRTAG_MAX elements long.
I've sent out v7.1 of this patch attached to this version,
https://lore.kernel.org/linux-xfs/20210303000202.2671220-1-hsiangkao@redhat.com/
Yes, I received a 0day CI report of this, would you mind
take a look at the fix above instead?
Thanks,
Ga oXiang
>
> --D
>
> > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> > index 185b4915b7bf..5192a7063d95 100644
> > --- a/fs/xfs/xfs_error.c
> > +++ b/fs/xfs/xfs_error.c
> > @@ -168,6 +168,7 @@ XFS_ERRORTAG_ATTR_RW(iunlink_fallback, XFS_ERRTAG_IUNLINK_FALLBACK);
> > XFS_ERRORTAG_ATTR_RW(buf_ioerror, XFS_ERRTAG_BUF_IOERROR);
> > XFS_ERRORTAG_ATTR_RW(reduce_max_iextents, XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
> > XFS_ERRORTAG_ATTR_RW(bmap_alloc_minlen_extent, XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT);
> > +XFS_ERRORTAG_ATTR_RW(ag_resv_fail, XFS_ERRTAG_AG_RESV_FAIL);
> >
> > static struct attribute *xfs_errortag_attrs[] = {
> > XFS_ERRORTAG_ATTR_LIST(noerror),
> > @@ -208,6 +209,7 @@ static struct attribute *xfs_errortag_attrs[] = {
> > XFS_ERRORTAG_ATTR_LIST(buf_ioerror),
> > XFS_ERRORTAG_ATTR_LIST(reduce_max_iextents),
> > XFS_ERRORTAG_ATTR_LIST(bmap_alloc_minlen_extent),
> > + XFS_ERRORTAG_ATTR_LIST(ag_resv_fail),
> > NULL,
> > };
> >
> > --
> > 2.27.0
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread