On 3/19/26 19:23, Lorenzo Stoakes (Oracle) wrote:
> Commit 9d5403b1036c ("fs: convert most other generic_file_*mmap() users to
> .mmap_prepare()") updated AFS to use the mmap_prepare callback in favour
> of the deprecated mmap callback.
>
> However, it did not account for the fact that mmap_prepare is called
> pre-merge, and may then be merged, nor that mmap_prepare can fail to map
> due to an out of memory error.
So that means a file can become pinned forever? OOM is probably only a
problem with fault injection in practice, but the merge case can happen. And
9d5403b1036c is pre-6.18 LTS. Are we going to need Fixes: and Cc: stable then?
> Both of those are cases in which we should not be incrementing a reference
> count.
>
> With the newly added vm_ops->mapped callback available, we can simply
> defer this operation to that callback which is only invoked once the
> mapping is successfully in place (but not yet visible to userspace as the
> mmap and VMA write locks are held).
>
> Therefore add afs_mapped() to implement this callback for AFS, and remove
> the code doing so in afs_mmap_prepare().
>
> Also update afs_vm_open(), afs_vm_close() and afs_vm_map_pages() to be
> consistent in how the vnode is accessed.
>
> Signed-off-by: Lorenzo Stoakes (Oracle) <[email protected]>
> ---
> fs/afs/file.c | 36 ++++++++++++++++++++++++++----------
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/fs/afs/file.c b/fs/afs/file.c
> index f609366fd2ac..85696ac984cc 100644
> --- a/fs/afs/file.c
> +++ b/fs/afs/file.c
> @@ -28,6 +28,8 @@ static ssize_t afs_file_splice_read(struct file *in, loff_t
> *ppos,
> static void afs_vm_open(struct vm_area_struct *area);
> static void afs_vm_close(struct vm_area_struct *area);
> static vm_fault_t afs_vm_map_pages(struct vm_fault *vmf, pgoff_t
> start_pgoff, pgoff_t end_pgoff);
> +static int afs_mapped(unsigned long start, unsigned long end, pgoff_t pgoff,
> + const struct file *file, void **vm_private_data);
>
> const struct file_operations afs_file_operations = {
> .open = afs_open,
> @@ -61,6 +63,7 @@ const struct address_space_operations afs_file_aops = {
> };
>
> static const struct vm_operations_struct afs_vm_ops = {
> + .mapped = afs_mapped,
> .open = afs_vm_open,
> .close = afs_vm_close,
> .fault = filemap_fault,
> @@ -494,32 +497,45 @@ static void afs_drop_open_mmap(struct afs_vnode *vnode)
> */
> static int afs_file_mmap_prepare(struct vm_area_desc *desc)
> {
> - struct afs_vnode *vnode = AFS_FS_I(file_inode(desc->file));
> int ret;
>
> - afs_add_open_mmap(vnode);
> -
> ret = generic_file_mmap_prepare(desc);
> - if (ret == 0)
> - desc->vm_ops = &afs_vm_ops;
> - else
> - afs_drop_open_mmap(vnode);
> + if (ret)
> + return ret;
> +
> + desc->vm_ops = &afs_vm_ops;
> return ret;
> }
>
> +static int afs_mapped(unsigned long start, unsigned long end, pgoff_t pgoff,
> + const struct file *file, void **vm_private_data)
> +{
> + struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
> +
> + afs_add_open_mmap(vnode);
> + return 0;
> +}
> +
> static void afs_vm_open(struct vm_area_struct *vma)
> {
> - afs_add_open_mmap(AFS_FS_I(file_inode(vma->vm_file)));
> + struct file *file = vma->vm_file;
> + struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
> +
> + afs_add_open_mmap(vnode);
> }
>
> static void afs_vm_close(struct vm_area_struct *vma)
> {
> - afs_drop_open_mmap(AFS_FS_I(file_inode(vma->vm_file)));
> + struct file *file = vma->vm_file;
> + struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
> +
> + afs_drop_open_mmap(vnode);
> }
>
> static vm_fault_t afs_vm_map_pages(struct vm_fault *vmf, pgoff_t
> start_pgoff, pgoff_t end_pgoff)
> {
> - struct afs_vnode *vnode = AFS_FS_I(file_inode(vmf->vma->vm_file));
> + struct file *file = vmf->vma->vm_file;
> + struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
>
> if (afs_check_validity(vnode))
> return filemap_map_pages(vmf, start_pgoff, end_pgoff);
