Message ID | 170191666222.1182270.11568535367691161509.stgit@frogsfrogsfrogs (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | xfs: online repair of inodes and forks | expand |
> +/* > + * Decide if this directory has been zapped to satisfy the inode and ifork > + * verifiers. Checking and repairing should be postponed until the directory > + * is fixed. > + */ > +bool > +xchk_dir_looks_zapped( > + struct xfs_inode *dp) > +{ > + /* Repair zapped this dir's data fork a short time ago */ > + if (xfs_ifork_zapped(dp, XFS_DATA_FORK)) > + return true; > + > + /* > + * If the dinode repair found a bad data fork, it will reset the fork > + * to extents format with zero records and wait for the bmapbtd > + * scrubber to reconstruct the block mappings. Directories always > + * contain some content, so this is a clear sign of a zapped directory. > + */ > + return dp->i_df.if_format == XFS_DINODE_FMT_EXTENTS && > + dp->i_df.if_nextents == 0; Correct me if I'm wrong: in general the xfs_ifork_zapped should be all that's needed here now, and the check below just finds another obvious case if we crashed / unmounted and lost the zapped flag? If so maybe update the comment a bit. Otherwise: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Dec 06, 2023 at 10:03:36PM -0800, Christoph Hellwig wrote: > > +/* > > + * Decide if this directory has been zapped to satisfy the inode and ifork > > + * verifiers. Checking and repairing should be postponed until the directory > > + * is fixed. > > + */ > > +bool > > +xchk_dir_looks_zapped( > > + struct xfs_inode *dp) > > +{ > > + /* Repair zapped this dir's data fork a short time ago */ > > + if (xfs_ifork_zapped(dp, XFS_DATA_FORK)) > > + return true; > > + > > + /* > > + * If the dinode repair found a bad data fork, it will reset the fork > > + * to extents format with zero records and wait for the bmapbtd > > + * scrubber to reconstruct the block mappings. Directories always > > + * contain some content, so this is a clear sign of a zapped directory. > > + */ > > + return dp->i_df.if_format == XFS_DINODE_FMT_EXTENTS && > > + dp->i_df.if_nextents == 0; > > Correct me if I'm wrong: in general the xfs_ifork_zapped should be > all that's needed here now, and the check below just finds another > obvious case if we crashed / unmounted and lost the zapped flag? > If so maybe update the comment a bit. Correct. The comment now reads: /* * If the dinode repair found a bad data fork, it will reset the fork * to extents format with zero records and wait for the bmapbtd * scrubber to reconstruct the block mappings. Directories always * contain some content, so this is a clear sign of a zapped directory. * The state checked by xfs_ifork_zapped is not persisted, so this is * our backup strategy if repairs are interrupted by a crash or an * unmount. */ > Otherwise: > > Reviewed-by: Christoph Hellwig <hch@lst.de> Thanks! --D
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c index bff0a374fb1b4..b2df093a7c5fa 100644 --- a/fs/xfs/scrub/common.c +++ b/fs/xfs/scrub/common.c @@ -25,6 +25,7 @@ #include "xfs_trans_priv.h" #include "xfs_da_format.h" #include "xfs_da_btree.h" +#include "xfs_dir2_priv.h" #include "xfs_attr.h" #include "xfs_reflink.h" #include "xfs_ag.h" diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h index c69cacb0b6962..ec5755266259d 100644 --- a/fs/xfs/scrub/common.h +++ b/fs/xfs/scrub/common.h @@ -198,6 +198,8 @@ static inline bool xchk_skip_xref(struct xfs_scrub_metadata *sm) XFS_SCRUB_OFLAG_XCORRUPT); } +bool xchk_dir_looks_zapped(struct xfs_inode *dp); + #ifdef CONFIG_XFS_ONLINE_REPAIR /* Decide if a repair is required. */ static inline bool xchk_needs_repair(const struct xfs_scrub_metadata *sm) diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c index 0b491784b7594..d63bd24745d14 100644 --- a/fs/xfs/scrub/dir.c +++ b/fs/xfs/scrub/dir.c @@ -788,3 +788,26 @@ xchk_directory( error = 0; return error; } + +/* + * Decide if this directory has been zapped to satisfy the inode and ifork + * verifiers. Checking and repairing should be postponed until the directory + * is fixed. + */ +bool +xchk_dir_looks_zapped( + struct xfs_inode *dp) +{ + /* Repair zapped this dir's data fork a short time ago */ + if (xfs_ifork_zapped(dp, XFS_DATA_FORK)) + return true; + + /* + * If the dinode repair found a bad data fork, it will reset the fork + * to extents format with zero records and wait for the bmapbtd + * scrubber to reconstruct the block mappings. Directories always + * contain some content, so this is a clear sign of a zapped directory. + */ + return dp->i_df.if_format == XFS_DINODE_FMT_EXTENTS && + dp->i_df.if_nextents == 0; +} diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index e6155d86f7916..7db8736721461 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -156,6 +156,16 @@ xchk_parent_validate( goto out_rele; } + /* + * We cannot yet validate this parent pointer if the directory looks as + * though it has been zapped by the inode record repair code. + */ + if (xchk_dir_looks_zapped(dp)) { + error = -EBUSY; + xchk_set_incomplete(sc); + goto out_unlock; + } + /* Look for a directory entry in the parent pointing to the child. */ error = xchk_dir_walk(sc, dp, xchk_parent_actor, &spc); if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0, &error)) @@ -217,6 +227,13 @@ xchk_parent( */ error = xchk_parent_validate(sc, parent_ino); } while (error == -EAGAIN); + if (error == -EBUSY) { + /* + * We could not scan a directory, so we marked the check + * incomplete. No further error return is necessary. + */ + return 0; + } return error; }