xfs: try to attach dquots to files before repairing them
authorDarrick J. Wong <djwong@kernel.org>
Fri, 15 Dec 2023 18:03:34 +0000 (10:03 -0800)
committerDarrick J. Wong <djwong@kernel.org>
Fri, 15 Dec 2023 18:03:34 +0000 (10:03 -0800)
Inode resource usage is tracked in the quota metadata.  Repairing a file
might change the resources used by that file, which means that we need
to attach dquots to the file that we're examining before accessing
anything in the file protected by the ILOCK.

However, there's a twist: a dquot cache miss requires the dquot to be
read in from the quota file, during which we drop the ILOCK on the file
being examined.  This means that we *must* try to attach the dquots
before taking the ILOCK.

Therefore, dquots must be attached to files in the scrub setup function.
If doing so yields corruption errors (or unknown dquot errors), we
instead clear the quotachecked status, which will cause a quotacheck on
next mount.  A future series will make this trigger live quotacheck.

While we're here, change the xrep_ino_dqattach function to use the
unlocked dqattach functions so that we avoid cycling the ILOCK if the
inode already has dquots attached.  This makes the naming and locking
requirements consistent with the rest of the filesystem.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
fs/xfs/scrub/bmap.c
fs/xfs/scrub/common.c
fs/xfs/scrub/common.h
fs/xfs/scrub/inode.c
fs/xfs/scrub/repair.c
fs/xfs/scrub/rtbitmap.c
fs/xfs/scrub/rtsummary.c

index 06d8c1996a3389e4396b3c354a3f88ab76c94d81..f74bd2a97c7f734d3208a13ab43a01f817a5635c 100644 (file)
@@ -78,6 +78,10 @@ xchk_setup_inode_bmap(
        if (error)
                goto out;
 
+       error = xchk_ino_dqattach(sc);
+       if (error)
+               goto out;
+
        xchk_ilock(sc, XFS_ILOCK_EXCL);
 out:
        /* scrub teardown will unlock and release the inode */
index e0d6d8c9f6402253b5ba7d1490af557de15845ef..bff0a374fb1b40fb0dc7a87548a7f65900718d13 100644 (file)
@@ -819,6 +819,26 @@ again:
        return 0;
 }
 
+#ifdef CONFIG_XFS_QUOTA
+/*
+ * Try to attach dquots to this inode if we think we might want to repair it.
+ * Callers must not hold any ILOCKs.  If the dquots are broken and cannot be
+ * attached, a quotacheck will be scheduled.
+ */
+int
+xchk_ino_dqattach(
+       struct xfs_scrub        *sc)
+{
+       ASSERT(sc->tp != NULL);
+       ASSERT(sc->ip != NULL);
+
+       if (!xchk_could_repair(sc))
+               return 0;
+
+       return xrep_ino_dqattach(sc);
+}
+#endif
+
 /* Install an inode that we opened by handle for scrubbing. */
 int
 xchk_install_handle_inode(
@@ -1030,6 +1050,11 @@ xchk_setup_inode_contents(
        error = xchk_trans_alloc(sc, resblks);
        if (error)
                goto out;
+
+       error = xchk_ino_dqattach(sc);
+       if (error)
+               goto out;
+
        xchk_ilock(sc, XFS_ILOCK_EXCL);
 out:
        /* scrub teardown will unlock and release the inode for us */
index c31be570e7d88a016144c9708b10e4f96edff029..c69cacb0b69620e86879a589bab1dd03131adc3e 100644 (file)
@@ -103,9 +103,15 @@ xchk_setup_rtsummary(struct xfs_scrub *sc)
 }
 #endif
 #ifdef CONFIG_XFS_QUOTA
+int xchk_ino_dqattach(struct xfs_scrub *sc);
 int xchk_setup_quota(struct xfs_scrub *sc);
 #else
 static inline int
+xchk_ino_dqattach(struct xfs_scrub *sc)
+{
+       return 0;
+}
+static inline int
 xchk_setup_quota(struct xfs_scrub *sc)
 {
        return -ENOENT;
index b7a93380a1ab08942c075ea18e3c9fcb2bd7b182..7e97db8255c63878160bd7d9d15da0a9a9fe26f1 100644 (file)
@@ -39,6 +39,10 @@ xchk_prepare_iscrub(
        if (error)
                return error;
 
+       error = xchk_ino_dqattach(sc);
+       if (error)
+               return error;
+
        xchk_ilock(sc, XFS_ILOCK_EXCL);
        return 0;
 }
index b4e7c4ad779f95088c3557c6f1c2de902c7c053f..021f6ec72e873200cf80f24e7a108d60933f9475 100644 (file)
@@ -700,10 +700,10 @@ xrep_force_quotacheck(
  *
  * This function ensures that the appropriate dquots are attached to an inode.
  * We cannot allow the dquot code to allocate an on-disk dquot block here
- * because we're already in transaction context with the inode locked.  The
- * on-disk dquot should already exist anyway.  If the quota code signals
- * corruption or missing quota information, schedule quotacheck, which will
- * repair corruptions in the quota metadata.
+ * because we're already in transaction context.  The on-disk dquot should
+ * already exist anyway.  If the quota code signals corruption or missing quota
+ * information, schedule quotacheck, which will repair corruptions in the quota
+ * metadata.
  */
 int
 xrep_ino_dqattach(
@@ -711,7 +711,10 @@ xrep_ino_dqattach(
 {
        int                     error;
 
-       error = xfs_qm_dqattach_locked(sc->ip, false);
+       ASSERT(sc->tp != NULL);
+       ASSERT(sc->ip != NULL);
+
+       error = xfs_qm_dqattach(sc->ip);
        switch (error) {
        case -EFSBADCRC:
        case -EFSCORRUPTED:
index 41a1d89ae8e6cdce6e465ea68d7fc77a02c189ba..d509a08d3fc3e9649a251ff18dcfcb0bfbc86465 100644 (file)
@@ -32,6 +32,10 @@ xchk_setup_rtbitmap(
        if (error)
                return error;
 
+       error = xchk_ino_dqattach(sc);
+       if (error)
+               return error;
+
        xchk_ilock(sc, XFS_ILOCK_EXCL | XFS_ILOCK_RTBITMAP);
        return 0;
 }
index 8b15c47408d0310db7889864b9b0b10a3a7a60f7..f94800a029f358e0a487a790db1b0ccc334b3ea8 100644 (file)
@@ -63,6 +63,10 @@ xchk_setup_rtsummary(
        if (error)
                return error;
 
+       error = xchk_ino_dqattach(sc);
+       if (error)
+               return error;
+
        /*
         * Locking order requires us to take the rtbitmap first.  We must be
         * careful to unlock it ourselves when we are done with the rtbitmap