From 82742f8c3f1a93787a05a00aca50c2a565231f84 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 14 Oct 2024 08:04:50 +0200 Subject: xfs: pass the exact range to initialize to xfs_initialize_perag Currently only the new agcount is passed to xfs_initialize_perag, which requires lookups of existing AGs to skip them and complicates error handling. Also pass the previous agcount so that the range that xfs_initialize_perag operates on is exactly defined. That way the extra lookups can be avoided, and error handling can clean up the exact range from the old count to the last added perag structure. Signed-off-by: Christoph Hellwig Reviewed-by: Darrick J. Wong Reviewed-by: Brian Foster Signed-off-by: Carlos Maiolino --- fs/xfs/libxfs/xfs_ag.c | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) (limited to 'fs/xfs/libxfs/xfs_ag.c') diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c index 5f0494702e0b..464f682eab46 100644 --- a/fs/xfs/libxfs/xfs_ag.c +++ b/fs/xfs/libxfs/xfs_ag.c @@ -296,27 +296,16 @@ xfs_free_unused_perag_range( int xfs_initialize_perag( struct xfs_mount *mp, - xfs_agnumber_t agcount, + xfs_agnumber_t old_agcount, + xfs_agnumber_t new_agcount, xfs_rfsblock_t dblocks, xfs_agnumber_t *maxagi) { struct xfs_perag *pag; xfs_agnumber_t index; - xfs_agnumber_t first_initialised = NULLAGNUMBER; int error; - /* - * Walk the current per-ag tree so we don't try to initialise AGs - * that already exist (growfs case). Allocate and insert all the - * AGs we don't find ready for initialisation. - */ - for (index = 0; index < agcount; index++) { - pag = xfs_perag_get(mp, index); - if (pag) { - xfs_perag_put(pag); - continue; - } - + for (index = old_agcount; index < new_agcount; index++) { pag = kzalloc(sizeof(*pag), GFP_KERNEL | __GFP_RETRY_MAYFAIL); if (!pag) { error = -ENOMEM; @@ -353,21 +342,17 @@ xfs_initialize_perag( /* Active ref owned by mount indicates AG is online. */ atomic_set(&pag->pag_active_ref, 1); - /* first new pag is fully initialized */ - if (first_initialised == NULLAGNUMBER) - first_initialised = index; - /* * Pre-calculated geometry */ - pag->block_count = __xfs_ag_block_count(mp, index, agcount, + pag->block_count = __xfs_ag_block_count(mp, index, new_agcount, dblocks); pag->min_block = XFS_AGFL_BLOCK(mp); __xfs_agino_range(mp, pag->block_count, &pag->agino_min, &pag->agino_max); } - index = xfs_set_inode_alloc(mp, agcount); + index = xfs_set_inode_alloc(mp, new_agcount); if (maxagi) *maxagi = index; @@ -381,8 +366,7 @@ out_remove_pag: out_free_pag: kfree(pag); out_unwind_new_pags: - /* unwind any prior newly initialized pags */ - xfs_free_unused_perag_range(mp, first_initialised, agcount); + xfs_free_unused_perag_range(mp, old_agcount, index); return error; } -- cgit v1.2.3-70-g09d2 From aa67ec6a25617e36eba4fb28a88159f500a6cac6 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 14 Oct 2024 08:04:51 +0200 Subject: xfs: merge the perag freeing helpers There is no good reason to have two different routines for freeing perag structures for the unmount and error cases. Add two arguments to specify the range of AGs to free to xfs_free_perag, and use that to replace xfs_free_unused_perag_range. The addition RCU grace period for the error case is harmless, and the extra check for the AG to actually exist is not required now that the callers pass the exact known allocated range. Signed-off-by: Christoph Hellwig Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Carlos Maiolino --- fs/xfs/libxfs/xfs_ag.c | 40 ++++++++++------------------------------ fs/xfs/libxfs/xfs_ag.h | 5 ++--- fs/xfs/xfs_fsops.c | 2 +- fs/xfs/xfs_mount.c | 5 ++--- 4 files changed, 15 insertions(+), 37 deletions(-) (limited to 'fs/xfs/libxfs/xfs_ag.c') diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c index 464f682eab46..8ace2cc200a6 100644 --- a/fs/xfs/libxfs/xfs_ag.c +++ b/fs/xfs/libxfs/xfs_ag.c @@ -185,17 +185,20 @@ out: } /* - * Free up the per-ag resources associated with the mount structure. + * Free up the per-ag resources within the specified AG range. */ void -xfs_free_perag( - struct xfs_mount *mp) +xfs_free_perag_range( + struct xfs_mount *mp, + xfs_agnumber_t first_agno, + xfs_agnumber_t end_agno) + { - struct xfs_perag *pag; xfs_agnumber_t agno; - for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) { - pag = xa_erase(&mp->m_perags, agno); + for (agno = first_agno; agno < end_agno; agno++) { + struct xfs_perag *pag = xa_erase(&mp->m_perags, agno); + ASSERT(pag); XFS_IS_CORRUPT(pag->pag_mount, atomic_read(&pag->pag_ref) != 0); xfs_defer_drain_free(&pag->pag_intents_drain); @@ -270,29 +273,6 @@ xfs_agino_range( return __xfs_agino_range(mp, xfs_ag_block_count(mp, agno), first, last); } -/* - * Free perag within the specified AG range, it is only used to free unused - * perags under the error handling path. - */ -void -xfs_free_unused_perag_range( - struct xfs_mount *mp, - xfs_agnumber_t agstart, - xfs_agnumber_t agend) -{ - struct xfs_perag *pag; - xfs_agnumber_t index; - - for (index = agstart; index < agend; index++) { - pag = xa_erase(&mp->m_perags, index); - if (!pag) - break; - xfs_buf_cache_destroy(&pag->pag_bcache); - xfs_defer_drain_free(&pag->pag_intents_drain); - kfree(pag); - } -} - int xfs_initialize_perag( struct xfs_mount *mp, @@ -366,7 +346,7 @@ out_remove_pag: out_free_pag: kfree(pag); out_unwind_new_pags: - xfs_free_unused_perag_range(mp, old_agcount, index); + xfs_free_perag_range(mp, old_agcount, index); return error; } diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h index 69fc31e7b847..6e68d6a3161a 100644 --- a/fs/xfs/libxfs/xfs_ag.h +++ b/fs/xfs/libxfs/xfs_ag.h @@ -144,13 +144,12 @@ __XFS_AG_OPSTATE(prefers_metadata, PREFERS_METADATA) __XFS_AG_OPSTATE(allows_inodes, ALLOWS_INODES) __XFS_AG_OPSTATE(agfl_needs_reset, AGFL_NEEDS_RESET) -void xfs_free_unused_perag_range(struct xfs_mount *mp, xfs_agnumber_t agstart, - xfs_agnumber_t agend); int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t old_agcount, xfs_agnumber_t agcount, xfs_rfsblock_t dcount, xfs_agnumber_t *maxagi); +void xfs_free_perag_range(struct xfs_mount *mp, xfs_agnumber_t first_agno, + xfs_agnumber_t end_agno); int xfs_initialize_perag_data(struct xfs_mount *mp, xfs_agnumber_t agno); -void xfs_free_perag(struct xfs_mount *mp); /* Passive AG references */ struct xfs_perag *xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno); diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index de2bf0594cb4..b247d895c276 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -229,7 +229,7 @@ out_trans_cancel: xfs_trans_cancel(tp); out_free_unused_perag: if (nagcount > oagcount) - xfs_free_unused_perag_range(mp, oagcount, nagcount); + xfs_free_perag_range(mp, oagcount, nagcount); return error; } diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index 6fa7239a4a01..25bbcc3f4ee0 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -1048,7 +1048,7 @@ xfs_mountfs( xfs_buftarg_drain(mp->m_logdev_targp); xfs_buftarg_drain(mp->m_ddev_targp); out_free_perag: - xfs_free_perag(mp); + xfs_free_perag_range(mp, 0, mp->m_sb.sb_agcount); out_free_dir: xfs_da_unmount(mp); out_remove_uuid: @@ -1129,8 +1129,7 @@ xfs_unmountfs( xfs_errortag_clearall(mp); #endif shrinker_free(mp->m_inodegc_shrinker); - xfs_free_perag(mp); - + xfs_free_perag_range(mp, 0, mp->m_sb.sb_agcount); xfs_errortag_del(mp); xfs_error_sysfs_del(mp); xchk_stats_unregister(mp->m_scrub_stats); -- cgit v1.2.3-70-g09d2 From 069cf5e32b700f94c6ac60f6171662bdfb04f325 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 14 Oct 2024 08:04:54 +0200 Subject: xfs: don't use __GFP_RETRY_MAYFAIL in xfs_initialize_perag __GFP_RETRY_MAYFAIL increases the likelyhood of allocations to fail, which isn't really helpful during log recovery. Remove the flag and stick to the default GFP_KERNEL policies. Signed-off-by: Christoph Hellwig Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Carlos Maiolino --- fs/xfs/libxfs/xfs_ag.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/xfs/libxfs/xfs_ag.c') diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c index 8ace2cc200a6..25cec9dc10c9 100644 --- a/fs/xfs/libxfs/xfs_ag.c +++ b/fs/xfs/libxfs/xfs_ag.c @@ -286,7 +286,7 @@ xfs_initialize_perag( int error; for (index = old_agcount; index < new_agcount; index++) { - pag = kzalloc(sizeof(*pag), GFP_KERNEL | __GFP_RETRY_MAYFAIL); + pag = kzalloc(sizeof(*pag), GFP_KERNEL); if (!pag) { error = -ENOMEM; goto out_unwind_new_pags; -- cgit v1.2.3-70-g09d2 From 4a201dcfa1ff0dcfe4348c40f3ad8bd68b97eb6c Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 14 Oct 2024 08:04:55 +0200 Subject: xfs: update the pag for the last AG at recovery time Currently log recovery never updates the in-core perag values for the last allocation group when they were grown by growfs. This leads to btree record validation failures for the alloc, ialloc or finotbt trees if a transaction references this new space. Found by Brian's new growfs recovery stress test. Signed-off-by: Christoph Hellwig Reviewed-by: Brian Foster Signed-off-by: Carlos Maiolino --- fs/xfs/libxfs/xfs_ag.c | 17 +++++++++++++++++ fs/xfs/libxfs/xfs_ag.h | 1 + fs/xfs/xfs_buf_item_recover.c | 19 ++++++++++++++++--- 3 files changed, 34 insertions(+), 3 deletions(-) (limited to 'fs/xfs/libxfs/xfs_ag.c') diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c index 25cec9dc10c9..5ca8d0106827 100644 --- a/fs/xfs/libxfs/xfs_ag.c +++ b/fs/xfs/libxfs/xfs_ag.c @@ -273,6 +273,23 @@ xfs_agino_range( return __xfs_agino_range(mp, xfs_ag_block_count(mp, agno), first, last); } +int +xfs_update_last_ag_size( + struct xfs_mount *mp, + xfs_agnumber_t prev_agcount) +{ + struct xfs_perag *pag = xfs_perag_grab(mp, prev_agcount - 1); + + if (!pag) + return -EFSCORRUPTED; + pag->block_count = __xfs_ag_block_count(mp, prev_agcount - 1, + mp->m_sb.sb_agcount, mp->m_sb.sb_dblocks); + __xfs_agino_range(mp, pag->block_count, &pag->agino_min, + &pag->agino_max); + xfs_perag_rele(pag); + return 0; +} + int xfs_initialize_perag( struct xfs_mount *mp, diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h index 6e68d6a3161a..9edfe0e96439 100644 --- a/fs/xfs/libxfs/xfs_ag.h +++ b/fs/xfs/libxfs/xfs_ag.h @@ -150,6 +150,7 @@ int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t old_agcount, void xfs_free_perag_range(struct xfs_mount *mp, xfs_agnumber_t first_agno, xfs_agnumber_t end_agno); int xfs_initialize_perag_data(struct xfs_mount *mp, xfs_agnumber_t agno); +int xfs_update_last_ag_size(struct xfs_mount *mp, xfs_agnumber_t prev_agcount); /* Passive AG references */ struct xfs_perag *xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno); diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c index a839ff5dcaa9..5180cbf5a90b 100644 --- a/fs/xfs/xfs_buf_item_recover.c +++ b/fs/xfs/xfs_buf_item_recover.c @@ -708,6 +708,11 @@ xlog_recover_do_primary_sb_buffer( xlog_recover_do_reg_buffer(mp, item, bp, buf_f, current_lsn); + if (orig_agcount == 0) { + xfs_alert(mp, "Trying to grow file system without AGs"); + return -EFSCORRUPTED; + } + /* * Update the in-core super block from the freshly recovered on-disk one. */ @@ -718,15 +723,23 @@ xlog_recover_do_primary_sb_buffer( return -EFSCORRUPTED; } + /* + * Growfs can also grow the last existing AG. In this case we also need + * to update the length in the in-core perag structure and values + * depending on it. + */ + error = xfs_update_last_ag_size(mp, orig_agcount); + if (error) + return error; + /* * Initialize the new perags, and also update various block and inode * allocator setting based off the number of AGs or total blocks. * Because of the latter this also needs to happen if the agcount did * not change. */ - error = xfs_initialize_perag(mp, orig_agcount, - mp->m_sb.sb_agcount, mp->m_sb.sb_dblocks, - &mp->m_maxagi); + error = xfs_initialize_perag(mp, orig_agcount, mp->m_sb.sb_agcount, + mp->m_sb.sb_dblocks, &mp->m_maxagi); if (error) { xfs_warn(mp, "Failed recovery per-ag init: %d", error); return error; -- cgit v1.2.3-70-g09d2