xfs: stop using xfs_qm_dqtobp in xfs_qm_dqflush

In xfs_qm_dqflush we know that q_blkno must be initialized already from a
previous xfs_qm_dqread.  So instead of calling xfs_qm_dqtobp we can
simply read the quota buffer directly.  This also saves us from a duplicate
xfs_qm_dqcheck call check and allows xfs_qm_dqtobp to be simplified now
that it is always called for a newly initialized inode.  In addition to
that properly unwind all locks in xfs_qm_dqflush when xfs_qm_dqcheck
fails.

This mirrors a similar cleanup in the inode lookup done earlier.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alex Elder <aelder@sgi.com>

1 files changed, 76 insertions, 88 deletions

diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
index e1a2f6800e01..faf8e1a83a12 100644
--- a/fs/xfs/quota/xfs_dquot.c
+++ b/fs/xfs/quota/xfs_dquot.c
@@ -463,87 +463,68 @@ xfs_qm_dqtobp(
         uint                    flags)
 {
         xfs_bmbt_irec_t map;
-        int             nmaps, error;
+        int             nmaps = 1, error;
         xfs_buf_t       *bp;
-        xfs_inode_t     *quotip;
-        xfs_mount_t     *mp;
+        xfs_inode_t     *quotip = XFS_DQ_TO_QIP(dqp);
+        xfs_mount_t     *mp = dqp->q_mount;
         xfs_disk_dquot_t *ddq;
-        xfs_dqid_t      id;
-        boolean_t       newdquot;
+        xfs_dqid_t      id = be32_to_cpu(dqp->q_core.d_id);
         xfs_trans_t     *tp = (tpp ? *tpp : NULL);
 
-        mp = dqp->q_mount;
-        id = be32_to_cpu(dqp->q_core.d_id);
-        nmaps = 1;
-        newdquot = B_FALSE;
+        dqp->q_fileoffset = (xfs_fileoff_t)id / mp->m_quotainfo->qi_dqperchunk;
 
-        /*
-         * If we don't know where the dquot lives, find out.
-         */
-        if (dqp->q_blkno == (xfs_daddr_t) 0) {
-                /* We use the id as an index */
-                dqp->q_fileoffset = (xfs_fileoff_t)id /
-                                        mp->m_quotainfo->qi_dqperchunk;
-                nmaps = 1;
-                quotip = XFS_DQ_TO_QIP(dqp);
-                xfs_ilock(quotip, XFS_ILOCK_SHARED);
+        xfs_ilock(quotip, XFS_ILOCK_SHARED);
+        if (XFS_IS_THIS_QUOTA_OFF(dqp)) {
                 /*
-                 * Return if this type of quotas is turned off while we didn't
-                 * have an inode lock
+                 * Return if this type of quotas is turned off while we
+                 * didn't have the quota inode lock.
                  */
-                if (XFS_IS_THIS_QUOTA_OFF(dqp)) {
-                        xfs_iunlock(quotip, XFS_ILOCK_SHARED);
-                        return (ESRCH);
-                }
+                xfs_iunlock(quotip, XFS_ILOCK_SHARED);
+                return ESRCH;
+        }
+
+        /*
+         * Find the block map; no allocations yet
+         */
+        error = xfs_bmapi(NULL, quotip, dqp->q_fileoffset,
+                          XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
+                          NULL, 0, &map, &nmaps, NULL);
+
+        xfs_iunlock(quotip, XFS_ILOCK_SHARED);
+        if (error)
+                return error;
+
+        ASSERT(nmaps == 1);
+        ASSERT(map.br_blockcount == 1);
+
+        /*
+         * Offset of dquot in the (fixed sized) dquot chunk.
+         */
+        dqp->q_bufoffset = (id % mp->m_quotainfo->qi_dqperchunk) *
+                sizeof(xfs_dqblk_t);
+
+        ASSERT(map.br_startblock != DELAYSTARTBLOCK);
+        if (map.br_startblock == HOLESTARTBLOCK) {
                 /*
-                 * Find the block map; no allocations yet
+                 * We don't allocate unless we're asked to
                  */
-                error = xfs_bmapi(NULL, quotip, dqp->q_fileoffset,
-                                  XFS_DQUOT_CLUSTER_SIZE_FSB,
-                                  XFS_BMAPI_METADATA,
-                                  NULL, 0, &map, &nmaps, NULL);
+                if (!(flags & XFS_QMOPT_DQALLOC))
+                        return ENOENT;
 
-                xfs_iunlock(quotip, XFS_ILOCK_SHARED);
+                ASSERT(tp);
+                error = xfs_qm_dqalloc(tpp, mp, dqp, quotip,
+                                        dqp->q_fileoffset, &bp);
                 if (error)
-                        return (error);
-                ASSERT(nmaps == 1);
-                ASSERT(map.br_blockcount == 1);
+                        return error;
+                tp = *tpp;
+        } else {
+                trace_xfs_dqtobp_read(dqp);
 
                 /*
-                 * offset of dquot in the (fixed sized) dquot chunk.
+                 * store the blkno etc so that we don't have to do the
+                 * mapping all the time
                  */
-                dqp->q_bufoffset = (id % mp->m_quotainfo->qi_dqperchunk) *
-                        sizeof(xfs_dqblk_t);
-                if (map.br_startblock == HOLESTARTBLOCK) {
-                        /*
-                         * We don't allocate unless we're asked to
-                         */
-                        if (!(flags & XFS_QMOPT_DQALLOC))
-                                return (ENOENT);
-
-                        ASSERT(tp);
-                        if ((error = xfs_qm_dqalloc(tpp, mp, dqp, quotip,
-                                                dqp->q_fileoffset, &bp)))
-                                return (error);
-                        tp = *tpp;
-                        newdquot = B_TRUE;
-                } else {
-                        /*
-                         * store the blkno etc so that we don't have to do the
-                         * mapping all the time
-                         */
-                        dqp->q_blkno = XFS_FSB_TO_DADDR(mp, map.br_startblock);
-                }
-        }
-        ASSERT(dqp->q_blkno != DELAYSTARTBLOCK);
-        ASSERT(dqp->q_blkno != HOLESTARTBLOCK);
-
-        /*
-         * Read in the buffer, unless we've just done the allocation
-         * (in which case we already have the buf).
-         */
-        if (!newdquot) {
-                trace_xfs_dqtobp_read(dqp);
+                dqp->q_blkno = XFS_FSB_TO_DADDR(mp, map.br_startblock);
 
                 error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
                                            dqp->q_blkno,
@@ -552,13 +533,14 @@ xfs_qm_dqtobp(
                 if (error || !bp)
                         return XFS_ERROR(error);
         }
+
         ASSERT(XFS_BUF_ISBUSY(bp));
         ASSERT(XFS_BUF_VALUSEMA(bp) <= 0);
 
         /*
          * calculate the location of the dquot inside the buffer.
          */
-        ddq = (xfs_disk_dquot_t *)((char *)XFS_BUF_PTR(bp) + dqp->q_bufoffset);
+        ddq = (struct xfs_disk_dquot *)(XFS_BUF_PTR(bp) + dqp->q_bufoffset);
 
         /*
          * A simple sanity check in case we got a corrupted dquot...
@@ -1176,18 +1158,18 @@ xfs_qm_dqflush(
         xfs_dquot_t             *dqp,
         uint                    flags)
 {
-        xfs_mount_t             *mp;
-        xfs_buf_t               *bp;
-        xfs_disk_dquot_t        *ddqp;
+        struct xfs_mount        *mp = dqp->q_mount;
+        struct xfs_buf          *bp;
+        struct xfs_disk_dquot   *ddqp;
         int                     error;
 
         ASSERT(XFS_DQ_IS_LOCKED(dqp));
         ASSERT(!completion_done(&dqp->q_flush));
+
         trace_xfs_dqflush(dqp);
 
         /*
-         * If not dirty, or it's pinned and we are not supposed to
-         * block, nada.
+         * If not dirty, or it's pinned and we are not supposed to block, nada.
          */
         if (!XFS_DQ_IS_DIRTY(dqp) ||
             (!(flags & SYNC_WAIT) && atomic_read(&dqp->q_pincount) > 0)) {
@@ -1201,40 +1183,46 @@ xfs_qm_dqflush(
          * down forcibly. If that's the case we must not write this dquot
          * to disk, because the log record didn't make it to disk!
          */
-        if (XFS_FORCED_SHUTDOWN(dqp->q_mount)) {
-                dqp->dq_flags &= ~(XFS_DQ_DIRTY);
+        if (XFS_FORCED_SHUTDOWN(mp)) {
+                dqp->dq_flags &= ~XFS_DQ_DIRTY;
                 xfs_dqfunlock(dqp);
                 return XFS_ERROR(EIO);
         }
 
         /*
          * Get the buffer containing the on-disk dquot
-         * We don't need a transaction envelope because we know that the
-         * the ondisk-dquot has already been allocated for.
          */
-        if ((error = xfs_qm_dqtobp(NULL, dqp, &ddqp, &bp, XFS_QMOPT_DOWARN))) {
+        error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno,
+                                   mp->m_quotainfo->qi_dqchunklen, 0, &bp);
+        if (error) {
                 ASSERT(error != ENOENT);
-                /*
-                 * Quotas could have gotten turned off (ESRCH)
-                 */
                 xfs_dqfunlock(dqp);
-                return (error);
+                return error;
         }
 
-        if (xfs_qm_dqcheck(&dqp->q_core, be32_to_cpu(ddqp->d_id),
-                           0, XFS_QMOPT_DOWARN, "dqflush (incore copy)")) {
-                xfs_force_shutdown(dqp->q_mount, SHUTDOWN_CORRUPT_INCORE);
+        /*
+         * Calculate the location of the dquot inside the buffer.
+         */
+        ddqp = (struct xfs_disk_dquot *)(XFS_BUF_PTR(bp) + dqp->q_bufoffset);
+
+        /*
+         * A simple sanity check in case we got a corrupted dquot..
+         */
+        if (xfs_qm_dqcheck(&dqp->q_core, be32_to_cpu(ddqp->d_id), 0,
+                           XFS_QMOPT_DOWARN, "dqflush (incore copy)")) {
+                xfs_buf_relse(bp);
+                xfs_dqfunlock(dqp);
+                xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
                 return XFS_ERROR(EIO);
         }
 
         /* This is the only portion of data that needs to persist */
-        memcpy(ddqp, &(dqp->q_core), sizeof(xfs_disk_dquot_t));
+        memcpy(ddqp, &dqp->q_core, sizeof(xfs_disk_dquot_t));
 
         /*
          * Clear the dirty field and remember the flush lsn for later use.
          */
-        dqp->dq_flags &= ~(XFS_DQ_DIRTY);
-        mp = dqp->q_mount;
+        dqp->dq_flags &= ~XFS_DQ_DIRTY;
 
         xfs_trans_ail_copy_lsn(mp->m_ail, &dqp->q_logitem.qli_flush_lsn,
                                         &dqp->q_logitem.qli_item.li_lsn);