From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755746AbXI0Nv3 (ORCPT ); Thu, 27 Sep 2007 09:51:29 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756424AbXI0NvU (ORCPT ); Thu, 27 Sep 2007 09:51:20 -0400 Received: from pentafluge.infradead.org ([213.146.154.40]:52111 "EHLO pentafluge.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752279AbXI0NvS (ORCPT ); Thu, 27 Sep 2007 09:51:18 -0400 Subject: Re: lockdep wierdness... From: Peter Zijlstra To: Steven Rostedt Cc: Trond Myklebust , Andrew Morton , Ingo Molnar , linux-kernel@vger.kernel.org, Christoph Hellwig In-Reply-To: <20070925021359.GB11809@goodmis.org> References: <1190671658.6700.31.camel@heimdal.trondhjem.org> <20070925021359.GB11809@goodmis.org> Content-Type: text/plain Date: Thu, 27 Sep 2007 15:51:07 +0200 Message-Id: <1190901067.31636.6.camel@twins> Mime-Version: 1.0 X-Mailer: Evolution 2.10.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2007-09-24 at 22:13 -0400, Steven Rostedt wrote: > On Mon, Sep 24, 2007 at 06:07:38PM -0400, Trond Myklebust wrote: > > I'm seeing lockdep warning about a potential lock inversion between > > &mm->mmap_sem and &inode->i_mutex in NFS (see attachment). > > > > Unfortunately the basis for the warning appears to be the behaviour in > > ext3(???). AFAICS there is no way for NFS to share an inode->i_mutex > > with ext3. What to do? > > Actually this can probably happen just on NFS alone. > > > > > Trond > > > ======================================================= > > [ INFO: possible circular locking dependency detected ] > > 2.6.23-rc7-g8809e921 #1 > > ------------------------------------------------------- > > beagle-build-in/24375 is trying to acquire lock: > > (&mm->mmap_sem){----}, at: [] do_page_fault+0x17d/0x591 > > > > but task is already holding lock: > > (&inode->i_mutex){--..}, at: [] mutex_lock+0x1c/0x1f > > > > which lock already depends on the new lock. > > > > > > the existing dependency chain (in reverse order) is: > > > > -> #1 (&inode->i_mutex){--..}: > > [] __lock_acquire+0x9f3/0xba6 > > [] lock_acquire+0x5f/0x78 > > [] __mutex_lock_slowpath+0xe5/0x27a > > [] mutex_lock+0x1c/0x1f > > [] nfs_revalidate_mapping+0x64/0x9c [nfs] > > [] nfs_file_mmap+0x46/0x75 [nfs] > > [] mmap_region+0x1ea/0x3b8 > > [] do_mmap_pgoff+0x27b/0x2da > > [] sys_mmap2+0x9b/0xb5 > > [] sysenter_past_esp+0x5f/0x99 > > [] 0xffffffff > > > > -> #0 (&mm->mmap_sem){----}: > > [] __lock_acquire+0x8df/0xba6 > > [] lock_acquire+0x5f/0x78 > > [] down_read+0x3a/0x4c > > [] do_page_fault+0x17d/0x591 > > [] error_code+0x72/0x78 > > [] call_filldir+0xac/0xc3 [ext3] > > [] ext3_readdir+0x217/0x5e5 [ext3] > > [] vfs_readdir+0x67/0x93 > > [] sys_getdents+0x5f/0x9d > > [] sysenter_past_esp+0x5f/0x99 > > [] 0xffffffff > > The circular lock seems to be this: > > #1: > > sys_mmap2: down_write(&mm->mmap_sem); > nfs_revalidate_mapping: mutex_lock(&inode->i_mutex); > > > #0: > > vfs_readdir: mutex_lock(&inode->i_mutex); > - during the readdir (filldir64), we take a user fault (missing page?) > and call do_page_fault - > do_page_fault: down_read(&mm->mmap_sem); > > > So it does indeed look like a circular locking. Now the question is, "is > this a bug?". Looking like the inode of #1 must be a file or something > else that you can mmap and the inode of #0 seems it must be a directory. > I would say "no". > > Now if you can readdir on a file or mmap a directory, then this could be > an issue. > > Otherwise, I'd love to see someone teach lockdep about this issue! ;-) Christoph, does Steve's story make sense? If so, do we know at alloc_inode() time what type of inode we're requesting; file or dir. Again, if so, the lockdep annotation should be trivial in the light of the recently merged patch: + lockdep-give-each-filesystem-its-own-inode-lock-class.patch All that would need to be done is add an extra lock_class_key to file_system_type for i_mutex_dir_key, and extend alloc_inode to say something like: if (dir) lockdep_set_class(&inode->i_mutex, &sb->s_type->i_mutex_dir_key); else lockdep_set_class(&inode->i_mutex, &sb->s_type->i_mutex_key);