lkml.org 
[lkml]   [2011]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 3.1.0-rc3 -- INFO: possible circular locking dependency detected
    On Tue, 23 Aug 2011, Josh Boyer wrote:
    > On Tue, Aug 23, 2011 at 9:04 AM, Dave Chinner <david@fromorbit.com> wrote:
    > > On Tue, Aug 23, 2011 at 07:59:20AM -0400, Josh Boyer wrote:
    > >> On Tue, Aug 23, 2011 at 7:49 AM, Dave Chinner <david@fromorbit.com> wrote:
    > >> >> >  Possible unsafe locking scenario:
    > >> >> >
    > >> >> >       CPU0                    CPU1
    > >> >> >       ----                    ----
    > >> >> >  lock(&mm->mmap_sem);
    > >> >> >                               lock(&sb->s_type->i_mutex_key);
    > >> >> >                               lock(&mm->mmap_sem);
    > >> >> >  lock(&sb->s_type->i_mutex_key);
    > >> >> >
    > >> >> >  *** DEADLOCK ***
    > >> >>
    > >> >> This one was reported yesterday: https://lkml.org/lkml/2011/8/21/163
    > >> >> and we're hoping Ted (or someone else from the ext4 camp) can comment
    > >> >> on why ext4_evict_inode is holding i_mutex.
    > >> >
    > >> > Actually, the problem has nothing to do with ext4. the problem is
    > >> > that remove_vma() is holding the mmap_sem while calling fput(). The
    > >> > correct locking order is i_mutex->mmap_sem, as documented in
    > >> > mm/filemap.c:
    > >> >
    > >> >  *  ->i_mutex                   (generic_file_buffered_write)
    > >> >  *    ->mmap_sem                (fault_in_pages_readable->do_page_fault)
    > >> >
    > >> >
    > >> > The way remove_vma() calls fput() also triggers lockdep reports in
    > >> > XFS and it will do so with any filesystem that takes an inode
    > >> > specific lock in it's evict() processing. IOWs, remove_vma() needs
    > >> > fixing, not ext4....
    > >>
    > >> Er... ok.  So the remove_vma code hasn't changed since 2008.  We're
    > >> only seeing this issue now because the debugging code has improved,
    > >> or?
    > >
    > > The problem has been there since at least 2008.  Here's an early
    > > XFS report from 2.6.24:
    > >
    > > http://oss.sgi.com/archives/xfs/2008-02/msg00931.html
    > >
    > > Here's an XFS report
    > > to match the ext4 one in this thread from 2009:
    > >
    > > http://oss.sgi.com/archives/xfs/2009-03/msg00149.html
    > >
    > > You won't find reports much older than this - it only started to be
    > > reported when lockdep support in XFS matured and it started to be
    > > widely used....
    > >
    > >> At any rate, the proposed solution is to make remove_vma drop mmap_sem
    > >> before calling fput, or make it not call fput, or?
    > >
    > > Ask the VM folk - this is the only response I can remember from them
    > > is this:
    > >
    > > http://oss.sgi.com/archives/xfs/2009-03/msg00224.html
    > >
    > > Maybe now that ext4 is hitting the problem something will be done
    > > about it...
    >
    > OK. I've CC'd Andrew and Hugh, so maybe we can get a discussion going.

    My first reaction would be that this is quite simply a filesystem bug.
    Apparently a long-standing bug in the XFS case, but one in which ext4
    has just now (3.1-rc) joined it.

    The mm/fs locking hierarchy has been that way forever: mm does not assume
    that the fs will not take any inode-specific lock in its fput(), but yes,
    it does expect fput() not to take the i_mutex.

    In this new ext4 case, it appears to be just an issue on final eviction
    of the inode, which I think makes actual deadlock (when writing to file
    needs to fault in a page from mm) impossible - we wouldn't be evicting
    it if there were still references. Just needs some lockdep notation?

    Dropping mmap_sem while doing fput() in munmap() doesn't sound appealing
    to me: although we have converted a number of paths to drop mmap_sem in
    strategic places, getting back to the (possibly changed) vma sequence
    afterwards is tiresome (when the munmap covers multiple vmas), and
    instinct says that munmap() might be a more difficult case to get
    right than most. Leave the fput()s to a workqueue instead?

    Hugh
    \
     
     \ /
      Last update: 2011-08-25 00:35    [W:0.027 / U:2.664 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site