xfs
[Top] [All Lists]

Re: next-20090220: XFS: inconsistent lock state

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: next-20090220: XFS: inconsistent lock state
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Tue, 03 Mar 2009 10:00:01 -0600
Cc: Alexander Beregalov <a.beregalov@xxxxxxxxx>, "linux-next@xxxxxxxxxxxxxxx" <linux-next@xxxxxxxxxxxxxxx>, LKML <linux-kernel@xxxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20090224200740.GA9266@xxxxxxxxxxxxx>
References: <a4423d670902200952v5dc2fd91w3b54ab1db51a7fe2@xxxxxxxxxxxxxx> <20090224200740.GA9266@xxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.19 (Macintosh/20081209)
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:

Seems ok to me.  I hate to see the BUG() added but I guess in this case
something truly bizarre would have to happen for the ilock to fail on
this inode.

on irc you sugggested ASSERT(0); instead of BUG(); I might prefer that
but either way:

Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxxx>

> 
> 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;
>       }
>  
> +     /*
> +      * 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);
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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