From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from joshcartwright.net ([69.164.192.171] helo=li107-171.members.linode.com) by casper.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1Rx27H-00066i-Kl for linux-mtd@lists.infradead.org; Mon, 13 Feb 2012 20:03:16 +0000 Date: Mon, 13 Feb 2012 15:02:32 -0500 From: Josh Cartwright To: Brian Norris Subject: Re: jffs2 filesystem: possible circular locking dependency detected Message-ID: <20120213200232.GA19958@joshcartwright.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Cc: linux-rt-users , Darcy Watkins , Peter Zijlstra , linux-mtd@lists.infradead.org, Thomas Gleixner , David Woodhouse List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, Feb 10, 2012 at 10:57:54PM -0800, Brian Norris wrote: > On Wed, Feb 8, 2012 at 12:09 PM, Thomas Gleixner wrote: > > On Wed, 8 Feb 2012, Darcy Watkins wrote: > >> [   20.932000] ======================================================= > >> [   20.932000] [ INFO: possible circular locking dependency detected ] > >> [   20.932000] 3.0.18-rt34 #41 > >> [   20.932000] ------------------------------------------------------- > >> [   20.932000] depmod/734 is trying to acquire lock: > >> [   20.932000]  (&mm->mmap_sem){++++++}, at: [<800e82d0>] might_fault+0x4c/0xa4 > >> [   20.932000] > >> [   20.932000] but task is already holding lock: > >> [   20.932000]  (&f->sem){+.+.+.}, at: [<80184f88>] jffs2_readdir+0x108/0x1c0 > >> [   20.932000] > >> [   20.932000] which lock already depends on the new lock. > > > > Classic ABBA deadlock. I don't think it's RT specific, but I might be > > wrong as usual. Will have a look later this week, when noone beats me. > > Looks like someone beat you :) Josh Cartwright has a patch here: > http://lists.infradead.org/pipermail/linux-mtd/2012-February/039787.html Unfortunately, Darcy's lockdep splat implicates a different set of locks, so I think it is a different issue then I resolved in the linked patch. Looking into this one, however, I think I convinced myself that the lockdep warning is bogus. Here are two stack snippets that lockdep claims would be problematic if interleaved: do_page_fault() down_read(¤t->mm->mmap_sem) /* readahead... */ jffs2_readpage() mutex_lock(&JFFS2_INODE_INFO(inode)->sem) vfs_readdir() /* ... */ jffs2_readdir() mutex_lock(&JFFS2_INODE_INFO(inode)->sem) filldir() __put_user() /* fault ... */ do_page_fault() down_read(¤t->mm->mmap_sem) In Darcy's case, the validator saw the do_page_fault() segment first, and decided the lock order should be [mmap_sem, &JFFS2_INODE_INFO(inode)->sem]. It complained when it then saw the vfs_readdir() codepath reverse the order [1]. This would be problematic, if it wasn't for the guarantee that the jffs2_inode_info::sem in both paths will be different. In the readdir() path, the inode is the directory inode, whose i_fops doesn't even support mmap(), and so couldn't possibly be involved in a fault(). 1: Well, not exactly the same codepath, since a fault was not generated. put_user() includes a might_fault() which hints to lockdep that mmap_sem _could_ be acquired if a fault occurs. -- joshc