Re: [GIT PULL] Please pull NFS client bugfixes....

[Peter Staubach <staubach@redhat.com>]

Andi Kleen wrote:
>> Hi Linus,
>>
>> Please pull from the "bugfixes" branch of the repository at
>>
>>    git pull git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git bugfixes
>>
>> This will update the following files through the appended changesets.
> 
> How about the mmap lock order inversion patch?
> 
> That problem is still there and hits immediately after mounting root 
> with a lockdep warning on any lockdep enabled NFS root kernel.
> 

It seems to me that NFS must do some sort of validation at
this point.  This is one of the last points where the NFS
client can do validation for mmap'd files.

Simply crossing our fingers probably won't be sufficient.  :-)

I think that the file system must have a way to do validation
and page invalidation during mmap.  Perhaps we can reorder
things or add another entry point which is invoked after the
page mapping is set up which would give the file system to
check to ensure that the page cache is filled with valid data.

		ps

> -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;
>  }
>  
> 
>