On Wed, Nov 14, 2007 at 09:01:39PM +0100, Peter Zijlstra wrote:
> Normal locking order is:
>
> i_mutex
> mmap_sem
>
> However NFS's ->mmap hook, which is called under mmap_sem, can take i_mutex.
> Avoid this potential deadlock by doing the work that requires i_mutex from
> the new ->mmap_prepare().
>
> [ Is this sufficient, or does it introduce a race? ]
Seems like an OK patchset in my opinion. I don't know much about NFS
unfortunately, but I wonder what prevents the condition fixed by
nfs_revalidate_mapping from happening again while the mmap is active...?
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> ---
> fs/nfs/file.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> Index: linux-2.6/fs/nfs/file.c
> ===================================================================
> --- linux-2.6.orig/fs/nfs/file.c
> +++ linux-2.6/fs/nfs/file.c
> @@ -41,6 +41,9 @@
> static int nfs_file_open(struct inode *, struct file *);
> static int nfs_file_release(struct inode *, struct file *);
> static loff_t nfs_file_llseek(struct file *file, loff_t offset, int origin);
> +static int
> +nfs_file_mmap_prepare(struct file * file, unsigned long len,
> + unsigned long prot, unsigned long flags, unsigned long pgoff);
> static int nfs_file_mmap(struct file *, struct vm_area_struct *);
> static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
> struct pipe_inode_info *pipe,
> @@ -64,6 +67,7 @@ const struct file_operations nfs_file_op
> .write = do_sync_write,
> .aio_read = nfs_file_read,
> .aio_write = nfs_file_write,
> + .mmap_prepare = nfs_file_mmap_prepare,
> .mmap = nfs_file_mmap,
> .open = nfs_file_open,
> .flush = nfs_file_flush,
> @@ -270,7 +274,8 @@ nfs_file_splice_read(struct file *filp,
> }
>
> static int
> -nfs_file_mmap(struct file * file, struct vm_area_struct * vma)
> +nfs_file_mmap_prepare(struct file * file, unsigned long len,
> + unsigned long prot, unsigned long flags, unsigned long pgoff)
> {
> struct dentry *dentry = file->f_path.dentry;
> struct inode *inode = dentry->d_inode;
> @@ -279,13 +284,17 @@ nfs_file_mmap(struct file * file, struct
> dfprintk(VFS, "nfs: mmap(%s/%s)\n",
> dentry->d_parent->d_name.name, dentry->d_name.name);
>
> - status = nfs_revalidate_mapping(inode, file->f_mapping);
> - if (!status) {
> - vma->vm_ops = &nfs_file_vm_ops;
> - vma->vm_flags |= VM_CAN_NONLINEAR;
> - file_accessed(file);
> - }
> - return status;
> + return nfs_revalidate_mapping(inode, file->f_mapping);
> +}
> +
> +static int
> +nfs_file_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + vma->vm_ops = &nfs_file_vm_ops;
> + vma->vm_flags |= VM_CAN_NONLINEAR;
> + file_accessed(file);
> +
> + return 0;
> }
>
> /*
>
> --
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
