2009-03-03 16:45:45

by Felix Blyakher

[permalink] [raw]
Subject: Re: next-20090220: XFS: inconsistent lock state

On Feb 24, 2009, at 2:07 PM, Christoph Hellwig wrote:

> On Fri, Feb 20, 2009 at 08:52:59PM +0300, Alexander Beregalov wrote:
>> Hi
>>
>> [ INFO: inconsistent lock state ]
>> 2.6.29-rc5-next-20090220 #2
>> ---------------------------------
>> inconsistent {RECLAIM_FS-ON-W} -> {IN-RECLAIM_FS-R} usage.
>> kswapd0/324 [HC0[0]:SC0[0]:HE1:SE1] takes:
>> (&(&ip->i_lock)->mr_lock){+++++?}, at: [<ffffffff803ca60a>]
>> xfs_ilock+0xaa/0x120
>> {RECLAIM_FS-ON-W} state was registered at:
>
> That's a false positive. While the ilock can be taken in reclaim the
> allocation here is done before the inode is added to the inode cache.
>
> The patch below should help avoiding the warning:
>
>
> Index: xfs/fs/xfs/xfs_iget.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_iget.c 2009-02-24 20:56:00.716027739 +0100
> +++ xfs/fs/xfs/xfs_iget.c 2009-02-24 20:56:46.089031360 +0100
> @@ -246,9 +246,6 @@ xfs_iget_cache_miss(
> goto out_destroy;
> }
>
> - if (lock_flags)
> - xfs_ilock(ip, lock_flags);
> -
> /*
> * Preload the radix tree so we can insert safely under the
> * write spinlock. Note that we cannot sleep inside the preload
> @@ -259,6 +256,15 @@ xfs_iget_cache_miss(
> goto out_unlock;

Since we removed call to xfs_ilock() above, this should change
to 'goto out_destroy;'
Otherwise, seems goot to me.

Reviewed-by: Felix Blyakher <[email protected]>


>
> }
>
> + /*
> + * Because the inode hasn't been added to the radix-tree yet it
> can't
> + * be found by another thread, so we can do the non-sleeping lock
> here.
> + */
> + if (lock_flags) {
> + if (!xfs_ilock_nowait(ip, lock_flags))
> + BUG();

>
> + }
> +
> mask = ~(((XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog)) - 1);
> first_index = agino & mask;
> write_lock(&pag->pag_ici_lock);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/