| Message ID | 156757189224.1838441.6843620202811391667.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfsprogs-5.3: various fixes | expand |
On 9/3/19 11:38 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > In rmap_store_ag_btree_rec(), we try to reserve 16 blocks to handle > adding all the AG btree rmaps to the rmap record. Unfortunately, at > that point in phase5 we haven't yet reinitialied sb_fdblocks, so > reserving blocks can fail if repair reconstructed the primary sb from a > secondary sb. Even if the function succeeds, this still leads to > incorrect fdblocks because phase 5 resets sb_fdblocks after running the > rmap transactions. > > To avoid all this, move the rmap_store_ag_btree_rec call to after the sb > has been reset. xfs/350 was helpful in finding cases where xfs_repair > errored out while repairing the filesystem. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> makes sense to me; would be nice to know what this failure looks like at repair time tho, for inclusion in the changelog, perhaps. Reviewed-by: Eric Sandeen <sandeen@redhat.com> > --- > repair/phase5.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > > diff --git a/repair/phase5.c b/repair/phase5.c > index 2e18cc69..7f7d3d18 100644 > --- a/repair/phase5.c > +++ b/repair/phase5.c > @@ -2233,7 +2233,6 @@ phase5_func( > #endif > xfs_agblock_t num_extents; > struct agi_stat agi_stat = {0,}; > - int error; > > if (verbose) > do_log(_(" - agno = %d\n"), agno); > @@ -2426,14 +2425,6 @@ phase5_func( > finish_cursor(&fino_btree_curs); > finish_cursor(&bcnt_btree_curs); > > - /* > - * Put the per-AG btree rmap data into the rmapbt > - */ > - error = rmap_store_ag_btree_rec(mp, agno); > - if (error) > - do_error( > -_("unable to add AG %u reverse-mapping data to btree.\n"), agno); > - > /* > * release the incore per-AG bno/bcnt trees so > * the extent nodes can be recycled > @@ -2561,6 +2552,21 @@ phase5(xfs_mount_t *mp) > */ > sync_sb(mp); > > + /* > + * Put the per-AG btree rmap data into the rmapbt now that we've reset > + * the superblock counters. > + */ > + for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) { > + error = rmap_store_ag_btree_rec(mp, agno); > + if (error) > + do_error( > +_("unable to add AG %u reverse-mapping data to btree.\n"), agno); > + } > + > + /* > + * Put blocks that were unnecessarily reserved for btree > + * reconstruction back into the filesystem free space data. > + */ > error = inject_lost_blocks(mp, lost_fsb); > if (error) > do_error(_("Unable to reinsert lost blocks into filesystem.\n")); >
diff --git a/repair/phase5.c b/repair/phase5.c index 2e18cc69..7f7d3d18 100644 --- a/repair/phase5.c +++ b/repair/phase5.c @@ -2233,7 +2233,6 @@ phase5_func( #endif xfs_agblock_t num_extents; struct agi_stat agi_stat = {0,}; - int error; if (verbose) do_log(_(" - agno = %d\n"), agno); @@ -2426,14 +2425,6 @@ phase5_func( finish_cursor(&fino_btree_curs); finish_cursor(&bcnt_btree_curs); - /* - * Put the per-AG btree rmap data into the rmapbt - */ - error = rmap_store_ag_btree_rec(mp, agno); - if (error) - do_error( -_("unable to add AG %u reverse-mapping data to btree.\n"), agno); - /* * release the incore per-AG bno/bcnt trees so * the extent nodes can be recycled @@ -2561,6 +2552,21 @@ phase5(xfs_mount_t *mp) */ sync_sb(mp); + /* + * Put the per-AG btree rmap data into the rmapbt now that we've reset + * the superblock counters. + */ + for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) { + error = rmap_store_ag_btree_rec(mp, agno); + if (error) + do_error( +_("unable to add AG %u reverse-mapping data to btree.\n"), agno); + } + + /* + * Put blocks that were unnecessarily reserved for btree + * reconstruction back into the filesystem free space data. + */ error = inject_lost_blocks(mp, lost_fsb); if (error) do_error(_("Unable to reinsert lost blocks into filesystem.\n"));