On Fri, Sep 19, 2008 at 01:15:08PM +1000, 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;
Just move the xfs_ilock call after these two statements, there is no
need to have it locked before inserting it into the radix tree.
> @@ -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;
But here we still need the fix. But as Tim mention we need to check
for lock_flags != 0 first. Long-term it might make sense to just make
xfs_iunlock a no-op if lock_flags == 0, but let's do that separately.
|