xfs
[Top] [All Lists]

Re: [PATCH] Unlock inode before calling xfs_idestroy()

To: Timothy Shimmin <tes@xxxxxxx>
Subject: Re: [PATCH] Unlock inode before calling xfs_idestroy()
From: Lachlan McIlroy <lachlan@xxxxxxx>
Date: Mon, 22 Sep 2008 13:52:22 +1000
Cc: xfs-oss <xfs@xxxxxxxxxxx>, xfs-dev <xfs-dev@xxxxxxx>
In-reply-to: <48D326A7.6070508@xxxxxxx>
References: <48D3193C.6060201@xxxxxxx> <48D326A7.6070508@xxxxxxx>
Reply-to: lachlan@xxxxxxx
User-agent: Thunderbird 2.0.0.16 (X11/20080707)
Timothy Shimmin wrote:
Lachlan McIlroy wrote:
Lock debugging reported the ilock was being destroyed
without being unlocked.

--- a/fs/xfs/xfs_iget.c    2008-09-19 13:03:57.000000000 +1000
+++ b/fs/xfs/xfs_iget.c    2008-09-19 13:12:38.000000000 +1000
@@ -214,6 +214,7 @@ finish_inode:
        xfs_ilock(ip, lock_flags);

    if ((ip->i_d.di_mode == 0) && !(flags & XFS_IGET_CREATE)) {
+        xfs_iunlock(ip, lock_flags);
        xfs_idestroy(ip);
        xfs_put_perag(mp, pag);
        return ENOENT;
@@ -224,6 +225,7 @@ finish_inode:
     * write spinlock.
     */
    if (radix_tree_preload(GFP_KERNEL)) {
+        xfs_iunlock(ip, lock_flags);
        xfs_idestroy(ip);
        delay(1);
        goto again;
@@ -239,6 +241,7 @@ finish_inode:
        BUG_ON(error != -EEXIST);
        write_unlock(&pag->pag_ici_lock);
        radix_tree_preload_end();
+        xfs_iunlock(ip, lock_flags);
        xfs_idestroy(ip);
        XFS_STATS_INC(xs_ig_dup);
        goto again;

I'm just wondering about the case where lock_flags==0
and the inode is not locked.
I think it would fail an assert in xfs_iunlock().
Good catch Tim, thanks.



--Tim


        if (lock_flags)
                xfs_ilock(ip, lock_flags);

        if ((ip->i_d.di_mode == 0) && !(flags & XFS_IGET_CREATE)) {
                xfs_iunlock(ip, lock_flags);
                xfs_idestroy(ip);
                xfs_put_perag(mp, pag);
                return ENOENT;
        }

void
xfs_iunlock(
        xfs_inode_t             *ip,
        uint                    lock_flags)
{
        /*
         * You can't set both SHARED and EXCL for the same lock,
         * and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_ILOCK_SHARED,
         * and XFS_ILOCK_EXCL are valid values to set in lock_flags.
         */
        ASSERT((lock_flags & (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL)) !=
               (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
        ASSERT((lock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) !=
               (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
        ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_IUNLOCK_NONOTIFY |
                        XFS_LOCK_DEP_MASK)) == 0);
        ASSERT(lock_flags != 0);



<Prev in Thread] Current Thread [Next in Thread>