All of lore.kernel.org
 help / color / mirror / Atom feed
From: kdasu <kdasu.kdev@gmail.com>
To: xfs@oss.sgi.com
Subject: [PATCH 1/4] xfs: only lock the rt bitmap inode once per allocation
Date: Fri, 17 Feb 2012 14:51:15 -0800 (PST)	[thread overview]
Message-ID: <33346009.post@talk.nabble.com> (raw)
In-Reply-To: <33345988.post@talk.nabble.com>


Currently both xfs_rtpick_extent and xfs_rtallocate_extent call
xfs_trans_iget to grab and lock the rt bitmap inode, which results in a
deadlock since the removal of the lock recursion counters in commit

        "xfs: simplify inode to transaction joining"

Fix this by acquiring and locking the inode in xfs_bmap_rtalloc before
calling into xfs_rtpick_extent and xfs_rtallocate_extent.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alex Elder <aelder@sgi.com>
Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 fs/xfs/xfs_bmap.c    |   11 +++++++++++
 fs/xfs/xfs_rtalloc.c |   34 +++++++++++++---------------------
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index 4111cd3..9d9970b 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -2316,6 +2316,7 @@ xfs_bmap_rtalloc(
        xfs_extlen_t    prod = 0;       /* product factor for allocators */
        xfs_extlen_t    ralen = 0;      /* realtime allocation length */
        xfs_extlen_t    align;          /* minimum allocation alignment */
+       xfs_inode_t     *ip;            /* bitmap incore inode */
        xfs_rtblock_t   rtb;
kdasu@kdasu-VirtualBox:~/linux-2.6$ more
0001-xfs-only-lock-the-rt-bitmap-inode-once-per-allocatio.patch 
>From f59ab29cc191a5955c4d44c0b92a537f981184c2 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@infradead.org>
Date: Tue, 25 Jan 2011 09:06:19 +0000
Subject: [PATCH 1/4] xfs: only lock the rt bitmap inode once per allocation

Currently both xfs_rtpick_extent and xfs_rtallocate_extent call
xfs_trans_iget to grab and lock the rt bitmap inode, which results in a
deadlock since the removal of the lock recursion counters in commit

        "xfs: simplify inode to transaction joining"

Fix this by acquiring and locking the inode in xfs_bmap_rtalloc before
calling into xfs_rtpick_extent and xfs_rtallocate_extent.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alex Elder <aelder@sgi.com>
Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 fs/xfs/xfs_bmap.c    |   11 +++++++++++
 fs/xfs/xfs_rtalloc.c |   34 +++++++++++++---------------------
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index 4111cd3..9d9970b 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -2316,6 +2316,7 @@ xfs_bmap_rtalloc(
        xfs_extlen_t    prod = 0;       /* product factor for allocators */
        xfs_extlen_t    ralen = 0;      /* realtime allocation length */
        xfs_extlen_t    align;          /* minimum allocation alignment */
+       xfs_inode_t     *ip;            /* bitmap incore inode */
        xfs_rtblock_t   rtb;
 
        mp = ap->ip->i_mount;
@@ -2348,6 +2349,16 @@ xfs_bmap_rtalloc(
         */
        if (ralen * mp->m_sb.sb_rextsize >= MAXEXTLEN)
                ralen = MAXEXTLEN / mp->m_sb.sb_rextsize;
+
+       /*
+        * Lock out other modifications to the RT bitmap inode.
+        */
+       error = xfs_trans_iget(mp, ap->tp, mp->m_sb.sb_rbmino, 0,
+                              XFS_ILOCK_EXCL, &ip);
+       if (error)
+               return error;
+       ASSERT(ip == mp->m_rbmip);
+
        /*
         * If it's an allocation to an empty file at offset 0,
         * pick an extent that will space things out in the rt area.
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 12a1913..037fab1 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -2075,15 +2075,15 @@ xfs_rtallocate_extent(
        xfs_extlen_t    prod,           /* extent product factor */
        xfs_rtblock_t   *rtblock)       /* out: start block allocated */
 {
+       xfs_mount_t     *mp = tp->t_mountp;
        int             error;          /* error value */
-       xfs_inode_t     *ip;            /* inode for bitmap file */
-       xfs_mount_t     *mp;            /* file system mount structure */
        xfs_rtblock_t   r;              /* result allocated block */
        xfs_fsblock_t   sb;             /* summary file block number */
        xfs_buf_t       *sumbp;         /* summary file block buffer */
 
+       ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
        ASSERT(minlen > 0 && minlen <= maxlen);
-       mp = tp->t_mountp;
+
        /*
         * If prod is set then figure out what to do to minlen and maxlen.
         */
@@ -2099,12 +2099,7 @@ xfs_rtallocate_extent(
                        return 0;
                }
        }
-       /*
-        * Lock out other callers by grabbing the bitmap inode lock.
-        */
-       if ((error = xfs_trans_iget(mp, tp, mp->m_sb.sb_rbmino, 0,
-                                       XFS_ILOCK_EXCL, &ip)))
-               return error;
+
        sumbp = NULL;
        /*
         * Allocate by size, or near another block, or exactly at some
block.
@@ -2123,11 +2118,12 @@ xfs_rtallocate_extent(
                                len, &sumbp, &sb, prod, &r);
                break;
        default:
+               error = EIO;
                ASSERT(0);
        }
-       if (error) {
+       if (error)
                return error;
-       }
+
        /*
         * If it worked, update the superblock.
         */
@@ -2306,20 +2302,16 @@ xfs_rtpick_extent(
        xfs_rtblock_t   *pick)          /* result rt extent */
 {
        xfs_rtblock_t   b;              /* result block */
-       int             error;          /* error return value */
-       xfs_inode_t     *ip;            /* bitmap incore inode */
        int             log2;           /* log of sequence number */
        __uint64_t      resid;          /* residual after log removed */
        __uint64_t      seq;            /* sequence number of file creation
*/
        __uint64_t      *seqp;          /* pointer to seqno in inode */
 
-       if ((error = xfs_trans_iget(mp, tp, mp->m_sb.sb_rbmino, 0,
-                                       XFS_ILOCK_EXCL, &ip)))
-               return error;
+
        sumbp = NULL;
        /*
         * Allocate by size, or near another block, or exactly at some
block.
@@ -2123,11 +2118,12 @@ xfs_rtallocate_extent(
                                len, &sumbp, &sb, prod, &r);
                break;
        default:
+               error = EIO;
                ASSERT(0);
        }
-       if (error) {
+       if (error)
                return error;
-       }
+
        /*
         * If it worked, update the superblock.
         */
@@ -2306,20 +2302,16 @@ xfs_rtpick_extent(
        xfs_rtblock_t   *pick)          /* result rt extent */
 {
        xfs_rtblock_t   b;              /* result block */
-       int             error;          /* error return value */
-       xfs_inode_t     *ip;            /* bitmap incore inode */
        int             log2;           /* log of sequence number */
        __uint64_t      resid;          /* residual after log removed */
        __uint64_t      seq;            /* sequence number of file creation
*/
        __uint64_t      *seqp;          /* pointer to seqno in inode */
 
-       if ((error = xfs_trans_iget(mp, tp, mp->m_sb.sb_rbmino, 0,
-                                       XFS_ILOCK_EXCL, &ip)))
-               return error;
-       ASSERT(ip == mp->m_rbmip);
-       seqp = (__uint64_t *)&ip->i_d.di_atime;
-       if (!(ip->i_d.di_flags & XFS_DIFLAG_NEWRTBM)) {
-               ip->i_d.di_flags |= XFS_DIFLAG_NEWRTBM;
+       ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
+
+       seqp = (__uint64_t *)&mp->m_rbmip->i_d.di_atime;
+       if (!(mp->m_rbmip->i_d.di_flags & XFS_DIFLAG_NEWRTBM)) {
+               mp->m_rbmip->i_d.di_flags |= XFS_DIFLAG_NEWRTBM;
                *seqp = 0;
        }
        seq = *seqp;
@@ -2335,7 +2327,7 @@ xfs_rtpick_extent(
                        b = mp->m_sb.sb_rextents - len;
        }
        *seqp = seq + 1;
-       xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+       xfs_trans_log_inode(tp, mp->m_rbmip, XFS_ILOG_CORE);
        *pick = b;
        return 0;
 }
-- 
1.7.5.4
-- 
View this message in context: http://old.nabble.com/-PATCH-0-4--xfs%3A-resurrect-realtime-subvolume-support-on-kernel-2.6.37-tp33345988p33346009.html
Sent from the Xfs - General mailing list archive at Nabble.com.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2012-02-17 22:51 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-17 22:46 [PATCH 0/4] xfs: resurrect realtime subvolume support on kernel 2.6.37 kdasu
2012-02-17 22:51 ` kdasu [this message]
2012-02-17 22:55   ` [PATCH 2/4] xfs: fix xfs_get_extsz_hint for a zero extent size hint kdasu
2012-02-17 22:58     ` [PATCH 3/4] xfs: add lockdep annotations for the rt inodes kdasu
2012-02-17 23:00       ` [PATCH 4/4] xfs: fix deadlock in xfs_rtfree_extent with kernel v2.6.37 kdasu
2012-02-19 22:41         ` Christoph Hellwig
2012-02-21 17:22           ` Kamal Dasu
2012-02-23 16:52             ` [PATCH 4/4] V2 " Kamal Dasu
2012-02-25  9:40               ` Christoph Hellwig
2012-02-25 15:46                 ` Kamal Dasu
2012-02-28  8:36                   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=33346009.post@talk.nabble.com \
    --to=kdasu.kdev@gmail.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.