Hi Bob,

On Fri, Feb 11, 2022 at 4:51 PM Bob Peterson <rpete...@redhat.com> wrote:
> Before this patch quota function bh_get called gfs2_iomap_get after it
> had locked the sd_quota_mutex. That's a problem because that holds the
> i_rw_mutex, and that lock order is different from other code that
> locks i_rw_mutex first, then the sd_quota_mutex:

I see the problem, but the patch is pretty awful.  Can you resend based
on the following suggestion?

Thanks,
Andreas

---
 fs/gfs2/quota.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 59d727a4ae2c..58fe5377d589 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -370,14 +370,12 @@ static int bh_get(struct gfs2_quota_data *qd)
        unsigned int block, offset;
        struct buffer_head *bh;
        struct iomap iomap = { };
-       int error;
+       int error = 0;
 
        mutex_lock(&sdp->sd_quota_mutex);
-
-       if (qd->qd_bh_count++) {
-               mutex_unlock(&sdp->sd_quota_mutex);
-               return 0;
-       }
+       if (qd->qd_bh_count)
+               goto inc_out;
+       mutex_unlock(&sdp->sd_quota_mutex);
 
        block = qd->qd_slot / sdp->sd_qc_per_block;
        offset = qd->qd_slot % sdp->sd_qc_per_block;
@@ -386,32 +384,31 @@ static int bh_get(struct gfs2_quota_data *qd)
                               (loff_t)block << inode->i_blkbits,
                               i_blocksize(inode), &iomap);
        if (error)
-               goto fail;
-       error = -ENOENT;
+               return error;
        if (iomap.type != IOMAP_MAPPED)
-               goto fail;
+               return -ENOENT;
 
        error = gfs2_meta_read(ip->i_gl, iomap.addr >> inode->i_blkbits,
                               DIO_WAIT, 0, &bh);
        if (error)
-               goto fail;
-       error = -EIO;
-       if (gfs2_metatype_check(sdp, bh, GFS2_METATYPE_QC))
-               goto fail_brelse;
+               return error;
+       if (gfs2_metatype_check(sdp, bh, GFS2_METATYPE_QC)) {
+               brelse(bh);
+               return -EIO;
+       }
 
+       mutex_lock(&sdp->sd_quota_mutex);
+       if (qd->qd_bh_count) {
+               brelse(bh);
+               goto inc_out;
+       }
        qd->qd_bh = bh;
        qd->qd_bh_qc = (struct gfs2_quota_change *)
                (bh->b_data + sizeof(struct gfs2_meta_header) +
                 offset * sizeof(struct gfs2_quota_change));
 
-       mutex_unlock(&sdp->sd_quota_mutex);
-
-       return 0;
-
-fail_brelse:
-       brelse(bh);
-fail:
-       qd->qd_bh_count--;
+inc_out:
+       qd->qd_bh_count++;
        mutex_unlock(&sdp->sd_quota_mutex);
        return error;
 }
-- 
2.35.1


Reply via email to