Minki/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2024-12-03 17:41:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

xfs: simplify xchk_parent_validate

Browse Source 
This function is unnecessarily long because it contains code to
revalidate a dotdot entry after cycling locks to try to confirm a
subdirectory parent pointer.  Shorten the codebase by making the
parent's lookup call do double duty as the revalidation code.

This weakeans the efficacy of this scrub function temporarily, but the
next patch will resolve this as part of fixing an unhandled race that is
the result of the VFS rename locking model not working the way Darrick
thought it did.

Rename this stupid 'dnum' variable too.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
This commit is contained in:
Darrick J. Wong 2023-04-11 19:00:19 -07:00
parent cbab28f4c0
commit b049962c0f
1 changed files with 24 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
fs/xfs/scrub/parent.c
View File

@ -71,7 +71,7 @@ xchk_parent_actor(
STATIC int
xchk_parent_validate(
struct xfs_scrub *sc,
xfs_ino_t dnum,
xfs_ino_t parent_ino,
bool *try_again)
{
struct xchk_parent_ctx spc = {
@ -86,11 +86,16 @@ xchk_parent_validate(
*try_again = false;
if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
/* Is this the root dir? Then '..' must point to itself. */
if (sc->ip == mp->m_rootip) {
if (sc->ip->i_ino != mp->m_sb.sb_rootino ||
sc->ip->i_ino != parent_ino)
xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
goto out;
}
/* '..' must not point to ourselves. */
if (sc->ip->i_ino == dnum) {
if (sc->ip->i_ino == parent_ino) {
xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
goto out;
}
@ -115,7 +120,7 @@ xchk_parent_validate(
* -EFSCORRUPTED or -EFSBADCRC then the parent is corrupt which is a
* cross referencing error. Any other error is an operational error.
*/
error = xfs_iget(mp, sc->tp, dnum, XFS_IGET_UNTRUSTED, 0, &dp);
error = xfs_iget(mp, sc->tp, parent_ino, XFS_IGET_UNTRUSTED, 0, &dp);
if (error == -EINVAL || error == -ENOENT) {
error = -EFSCORRUPTED;
xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error);
@ -135,71 +140,19 @@ xchk_parent_validate(
* use nowait here to avoid an ABBA deadlock on the parent and
* the child inodes.
*/
if (xfs_ilock_nowait(dp, XFS_IOLOCK_SHARED)) {
lock_mode = xfs_ilock_data_map_shared(dp);
error = xchk_dir_walk(sc, dp, xchk_parent_actor, &spc);
xfs_iunlock(dp, lock_mode);
if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0,
&error))
goto out_unlock;
if (spc.nlink != expected_nlink)
xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
goto out_unlock;
if (!xfs_ilock_nowait(dp, XFS_IOLOCK_SHARED)) {
*try_again = true;
goto out_rele;
}
/*
* The game changes if we get here. We failed to lock the parent,
* so we're going to try to verify both pointers while only holding
* one lock so as to avoid deadlocking with something that's actually
* trying to traverse down the directory tree.
*/
xfs_iunlock(sc->ip, sc->ilock_flags);
sc->ilock_flags = 0;
error = xchk_ilock_inverted(dp, XFS_IOLOCK_SHARED);
if (error)
goto out_rele;
/* Go looking for our dentry. */
lock_mode = xfs_ilock_data_map_shared(dp);
error = xchk_dir_walk(sc, dp, xchk_parent_actor, &spc);
xfs_iunlock(dp, lock_mode);
if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0, &error))
goto out_unlock;
/* Drop the parent lock, relock this inode. */
xfs_iunlock(dp, XFS_IOLOCK_SHARED);
error = xchk_ilock_inverted(sc->ip, XFS_IOLOCK_EXCL);
if (error)
goto out_rele;
sc->ilock_flags = XFS_IOLOCK_EXCL;
/*
* If we're an unlinked directory, the parent /won't/ have a link
* to us. Otherwise, it should have one link. We have to re-set
* it here because we dropped the lock on sc->ip.
*/
expected_nlink = VFS_I(sc->ip)->i_nlink == 0 ? 0 : 1;
/* Look up '..' to see if the inode changed. */
error = xfs_dir_lookup(sc->tp, sc->ip, &xfs_name_dotdot, &dnum, NULL);
if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error))
goto out_rele;
/* Drat, parent changed. Try again! */
if (dnum != dp->i_ino) {
xfs_irele(dp);
*try_again = true;
return 0;
}
xfs_irele(dp);
/*
* '..' didn't change, so check that there was only one entry
* for us in the parent.
*/
if (spc.nlink != expected_nlink)
xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
return error;
out_unlock:
xfs_iunlock(dp, XFS_IOLOCK_SHARED);
@ -215,7 +168,7 @@ xchk_parent(
struct xfs_scrub *sc)
{
struct xfs_mount *mp = sc->mp;
xfs_ino_t dnum;
xfs_ino_t parent_ino;
bool try_again;
int tries = 0;
int error = 0;
@ -243,25 +196,18 @@ xchk_parent(
sc->ilock_flags &= ~(XFS_ILOCK_EXCL | XFS_MMAPLOCK_EXCL);
xfs_iunlock(sc->ip, XFS_ILOCK_EXCL | XFS_MMAPLOCK_EXCL);
/* Look up '..' */
error = xfs_dir_lookup(sc->tp, sc->ip, &xfs_name_dotdot, &dnum, NULL);
if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error))
goto out;
if (!xfs_verify_dir_ino(mp, dnum)) {
xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
goto out;
}
/* Is this the root dir? Then '..' must point to itself. */
if (sc->ip == mp->m_rootip) {
if (sc->ip->i_ino != mp->m_sb.sb_rootino ||
sc->ip->i_ino != dnum)
xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
goto out;
}
do {
error = xchk_parent_validate(sc, dnum, &try_again);
/* Look up '..' */
error = xfs_dir_lookup(sc->tp, sc->ip, &xfs_name_dotdot,
&parent_ino, NULL);
if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error))
goto out;
if (!xfs_verify_dir_ino(mp, parent_ino)) {
xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
goto out;
}
error = xchk_parent_validate(sc, parent_ino, &try_again);
if (error)
goto out;
} while (try_again && ++tries < 20);