Re: [PATCH 1/2] mm: Add unmap_mapping_file() helper

["David Hildenbrand (Arm)" <david@kernel.org>]

On 7/21/26 20:52, Krzysztof Wilczyński wrote:
> Currently, unmap_mapping_pages() and unmap_mapping_range() unmap
> a page offset window of an address_space, and teardown paths such
> as kernfs_drain_open_files() rely on them to remove the mappings
> of a file that is going away.
> 
> Files with f_mapping swapped to a shared address space at open time,
> such as PCI resource files using iomem_get_mapping(), have their
> VMAs attached to the shared mapping instead of the file's own inode
> mapping, so a teardown that unmaps the inode's address space cannot
> reach them, and they stay live after the file is removed.  As such,
> the only correct discriminator for these VMAs is the originating
> struct file recorded in vma->vm_file.
> 
> Add unmap_mapping_file(), which walks the VMA interval tree of the
> file's f_mapping under i_mmap_lock_read() and zaps every VMA with
> vm_file set to the given file, using the same per-VMA zap teardown
> as unmap_mapping_pages().
> 
> Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
> ---
>  include/linux/mm.h |  2 ++
>  mm/memory.c        | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 485df9c2dbdd..e882a03eaddd 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3183,6 +3183,7 @@ extern int fixup_user_fault(struct mm_struct *mm,
>  			    bool *unlocked);
>  void unmap_mapping_pages(struct address_space *mapping,
>  		pgoff_t start, pgoff_t nr, bool even_cows);
> +void unmap_mapping_file(struct file *file);
>  void unmap_mapping_range(struct address_space *mapping,
>  		loff_t const holebegin, loff_t const holelen, int even_cows);
>  #else
> @@ -3203,6 +3204,7 @@ static inline int fixup_user_fault(struct mm_struct *mm, unsigned long address,
>  }
>  static inline void unmap_mapping_pages(struct address_space *mapping,
>  		pgoff_t start, pgoff_t nr, bool even_cows) { }
> +static inline void unmap_mapping_file(struct file *file) { }
>  static inline void unmap_mapping_range(struct address_space *mapping,
>  		loff_t const holebegin, loff_t const holelen, int even_cows) { }
>  #endif
> diff --git a/mm/memory.c b/mm/memory.c
> index ff338c2abe92..f9cfb88ac4f7 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4423,6 +4423,32 @@ void unmap_mapping_pages(struct address_space *mapping, pgoff_t start,
>  }
>  EXPORT_SYMBOL_GPL(unmap_mapping_pages);
>  
> +/**
> + * unmap_mapping_file() - Unmap all mmaps of an open file.
> + * @file: The file to unmap.

We are unmapping pages/folios, not really mmaps.

"Unmap folio from all mmaps of a file." ?

> + *
> + * Unmap every VMA with vm_file set to @file, regardless of the address

Again, we are not unmapping VMAs, we are unmapping pages/folios within all VMAs
that map the file.

> + * space it is attached to.  This also covers files with f_mapping
> + * swapped to a different address space at open time, since such VMAs
> + * cannot be found through the file's own inode mapping.

God this is confusing :)

Sorry for the stupid question, but why can't we call

	unmap_mapping_range(file->f_mapping, 0, 0, 1);


What is a bit annoying is that you now do in the caller:

	if (of->file->f_mapping != inode->i_mapping)
		unmap_mapping_file(of->file);
	else
		unmap_mapping_range(inode->i_mapping, 0, 0, 1);

Shouldn't we just have one function that takes care of that internally ... so
it's harder to get wrong?

-- 
Cheers,

David