On Mon 16-06-25 20:33:28, Lorenzo Stoakes wrote:
> Update nearly all generic_file_mmap() and generic_file_readonly_mmap()
> callers to use generic_file_mmap_prepare() and
> generic_file_readonly_mmap_prepare() respectively.
>
> We update blkdev, 9p, afs, erofs, ext2, nfs, ntfs3, smb, ubifs and vboxsf
> file systems this way.
>
> Remaining users we cannot yet update are ecryptfs, fuse and cramfs. The
> former two are nested file systems that must support any underlying file
> ssytem, and cramfs inserts a mixed mapping which currently requires a VMA.
>
> Once all file systems have been converted to mmap_prepare(), we can then
> update nested file systems.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoa...@oracle.com>
Overall the patch looks good. Just a couple of notes regarding pointless
local variable being created...
> ---
> block/fops.c | 9 +++++----
> fs/9p/vfs_file.c | 11 ++++++-----
> fs/afs/file.c | 11 ++++++-----
> fs/erofs/data.c | 16 +++++++++-------
> fs/ext2/file.c | 12 +++++++-----
> fs/nfs/file.c | 13 +++++++------
> fs/nfs/internal.h | 2 +-
> fs/nfs/nfs4file.c | 2 +-
> fs/ntfs3/file.c | 15 ++++++++-------
> fs/smb/client/cifsfs.c | 12 ++++++------
> fs/smb/client/cifsfs.h | 4 ++--
> fs/smb/client/file.c | 14 ++++++++------
> fs/ubifs/file.c | 8 ++++----
> fs/vboxsf/file.c | 8 ++++----
> 14 files changed, 74 insertions(+), 63 deletions(-)
>
> diff --git a/block/fops.c b/block/fops.c
> index 1309861d4c2c..5a0ebc81e489 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -911,14 +911,15 @@ static long blkdev_fallocate(struct file *file, int
> mode, loff_t start,
> return error;
> }
>
> -static int blkdev_mmap(struct file *file, struct vm_area_struct *vma)
> +static int blkdev_mmap_prepare(struct vm_area_desc *desc)
> {
> + struct file *file = desc->file;
> struct inode *bd_inode = bdev_file_inode(file);
I guess no need to create 'file' variable here since it has only one use in
the line above...
>
> if (bdev_read_only(I_BDEV(bd_inode)))
> - return generic_file_readonly_mmap(file, vma);
> + return generic_file_readonly_mmap_prepare(desc);
>
> - return generic_file_mmap(file, vma);
> + return generic_file_mmap_prepare(desc);
> }
>
> const struct file_operations def_blk_fops = {
...
> @@ -492,16 +492,17 @@ static void afs_drop_open_mmap(struct afs_vnode *vnode)
> /*
> * Handle setting up a memory mapping on an AFS file.
> */
> -static int afs_file_mmap(struct file *file, struct vm_area_struct *vma)
> +static int afs_file_mmap_prepare(struct vm_area_desc *desc)
> {
> + struct file *file = desc->file;
> struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
Same comment about pointless local variable here as well.
> int ret;
>
> afs_add_open_mmap(vnode);
>
> - ret = generic_file_mmap(file, vma);
> + ret = generic_file_mmap_prepare(desc);
> if (ret == 0)
> - vma->vm_ops = &afs_vm_ops;
> + desc->vm_ops = &afs_vm_ops;
> else
> afs_drop_open_mmap(vnode);
> return ret;
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 6a329c329f43..52dfd1a44c43 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -409,20 +409,22 @@ static const struct vm_operations_struct
> erofs_dax_vm_ops = {
> .huge_fault = erofs_dax_huge_fault,
> };
>
> -static int erofs_file_mmap(struct file *file, struct vm_area_struct *vma)
> +static int erofs_file_mmap_prepare(struct vm_area_desc *desc)
> {
> + struct file *file = desc->file;
> +
> if (!IS_DAX(file_inode(file)))
And here...
> - return generic_file_readonly_mmap(file, vma);
> + return generic_file_readonly_mmap_prepare(desc);
>
> - if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
> + if ((desc->vm_flags & VM_SHARED) && (desc->vm_flags & VM_MAYWRITE))
> return -EINVAL;
>
> - vma->vm_ops = &erofs_dax_vm_ops;
> - vm_flags_set(vma, VM_HUGEPAGE);
> + desc->vm_ops = &erofs_dax_vm_ops;
> + desc->vm_flags |= VM_HUGEPAGE;
> return 0;
> }
...
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index 9835672267d2..2ed5173cfa73 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -2995,8 +2995,9 @@ static const struct vm_operations_struct
> cifs_file_vm_ops = {
> .page_mkwrite = cifs_page_mkwrite,
> };
>
> -int cifs_file_strict_mmap(struct file *file, struct vm_area_struct *vma)
> +int cifs_file_strict_mmap_prepare(struct vm_area_desc *desc)
> {
> + struct file *file = desc->file;
> int xid, rc = 0;
> struct inode *inode = file_inode(file);
Again pointless local variable 'file' here.
>
> @@ -3005,16 +3006,17 @@ int cifs_file_strict_mmap(struct file *file, struct
> vm_area_struct *vma)
> if (!CIFS_CACHE_READ(CIFS_I(inode)))
> rc = cifs_zap_mapping(inode);
> if (!rc)
> - rc = generic_file_mmap(file, vma);
> + rc = generic_file_mmap_prepare(desc);
> if (!rc)
> - vma->vm_ops = &cifs_file_vm_ops;
> + desc->vm_ops = &cifs_file_vm_ops;
>
> free_xid(xid);
> return rc;
> }
>
> -int cifs_file_mmap(struct file *file, struct vm_area_struct *vma)
> +int cifs_file_mmap_prepare(struct vm_area_desc *desc)
> {
> + struct file *file = desc->file;
> int rc, xid;
And here (the only use is in cifs_revalidate_file(file)).
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
