Messages in this thread | | | Subject | Re: NFS lockdep lock misordering mmap_sem<->i_mutex_key with 2.6.32-git1 | From | Trond Myklebust <> | Date | Tue, 15 Dec 2009 18:54:37 -0500 |
| |
On Wed, 2009-12-16 at 00:38 +0100, Andi Kleen wrote: > I suppose an easy workaround would be to not revalidate in mmap, > because open should have already done that? > > Very lightly tested RFC patch attached. > > -Andi > > --- > > NFS: don't revalidate in mmap > > nfs_revalidate_mapping takes i_mutex, but mmap already has mmap_sem > hold and taking i_mutex inside mmap_sem is not allowed by the VFS. > > So don't revalidate on mmap time and trust it has been already done. > > Signed-off-by: Andi Kleen <ak@linux.intel.com> > > --- > fs/nfs/file.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > Index: linux-2.6.32-ak/fs/nfs/file.c > =================================================================== > --- linux-2.6.32-ak.orig/fs/nfs/file.c > +++ linux-2.6.32-ak/fs/nfs/file.c > @@ -297,14 +297,9 @@ nfs_file_mmap(struct file * file, struct > dprintk("NFS: mmap(%s/%s)\n", > dentry->d_parent->d_name.name, dentry->d_name.name); > > - /* Note: generic_file_mmap() returns ENOSYS on nommu systems > - * so we call that before revalidating the mapping > - */ > status = generic_file_mmap(file, vma); > - if (!status) { > + if (!status) > vma->vm_ops = &nfs_file_vm_ops; > - status = nfs_revalidate_mapping(inode, file->f_mapping); > - } > return status; > } >
If you want to work around the problem rather than going for something like Peter's split up of the mmap() callback, then I'd suggest changing to using nfs_revalidate_mapping_nolock() instead. The fact that we are seeing these lock misordering warnings is proof that the call to nfs_revalidate_mapping() is not always a no-op.
By not taking the i_mutex your call to invalidate_inode_pages2() can potentially end up racing with another process that is writing to the file, but that should be a rare occurrence. The effect will be that the two processes can end up fighting to alternatively dirty and then clean the pages...
Cheers Trond
| |