On Fri, Feb 10, 2012 at 10:57:54PM -0800, Brian Norris wrote:
> On Wed, Feb 8, 2012 at 12:09 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
