lkml.org 
[lkml]   [2009]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: next-20090220: XFS: inconsistent lock state
    Date

    On Mar 3, 2009, at 10:00 AM, Eric Sandeen wrote:

    > 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();

    That would mean that instead of bombing out here, we do it
    in xfs debug kernels only, which is a good thing. However,
    do we just silently ignore it in non debug kernels, and
    later try to unlock without locking first?
    Maybe the following be better:

    if (lock_flags) {
    if (!xfs_ilock_nowait(ip, lock_flags)) {
    ASSERT(0);
    error = EAGAIN;
    goto out_destroy;
    }
    }

    Or just keep the BUG(); , as it shouldn't happen (we hope).

    Reviewed-by: Felix Blyakher <felixb@sgi.com>


    > I might prefer that
    > but either way:
    >
    > Reviewed-by: Eric Sandeen <sandeen@sandeen.net>
    >
    >>
    >> 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@oss.sgi.com
    >> http://oss.sgi.com/mailman/listinfo/xfs
    >>
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-
    > kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at http://www.tux.org/lkml/



    \
     
     \ /
      Last update: 2009-03-03 17:59    [W:0.025 / U:92.560 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site