Re: [PATCH] ovl: implement ->get_unmapped_area()

Re: [PATCH] ovl: implement ->get_unmapped_area()

[Amir Goldstein]

On Thu, Aug 27, 2026 at 7:07 PM Yuan-Hao Hsu <aa9736195201@gmail.com> wrote:
>
> ovl_mmap() passes realfile to the underlying filesystem, so the VMA it
> returns is backed by realfile, not by the overlayfs file.
>
> Address selection is not delegated.  __get_unmapped_area() looks at the
> overlayfs file's f_op, ovl_file_operations does not implement
> ->get_unmapped_area(), and the mapping lands wherever
> mm_get_unmapped_area() puts it.  ext4, xfs and btrfs all point
> ->get_unmapped_area() at thp_get_unmapped_area(); none of that is reached.
>
> vaddr - file_offset is then not a multiple of PMD_SIZE,
> thp_vma_suitable_order() rejects the VMA, and the PMD-sized folios the
> underlying filesystem already has in the page cache are never mapped by
> a PMD.
>
> Delegate to the file ovl_mmap() will use.  Calling thp_get_unmapped_area()
> here instead would force alignment on filesystems that deliberately do
> not implement ->get_unmapped_area(), which commit 34d7cf637c43 ("mm:
> don't try THP alignment for FS without get_unmapped_area") avoided.
> fs/proc delegates the same way in pde_get_unmapped_area().
>
> Reading a 1025M ext4 file through mmap(NULL, ...), 97% of its page cache
> in PMD-sized folios, 15 runs:
>
>                      PMD congruent  minor faults  page tables
>   ext4, directly             15/15           764          68K
>   overlayfs, before            0/5         16186        2056K
>   overlayfs, after           15/15           764          68K
>
> Fault counts had 0 stddev.  Median read time 17.4ms -> 4.80ms, against
> 4.92ms for the same file on ext4.
>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Yuan-Hao Hsu <aa9736195201@gmail.com>
>
> ---
>  fs/overlayfs/file.c |   26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index f3d97eb146e8..0d985025f4c8 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -13,6 +13,7 @@
>  #include <linux/security.h>
>  #include <linux/fs.h>
>  #include <linux/backing-file.h>
> +#include <linux/sched/mm.h>
>  #include "overlayfs.h"
>
>  static char ovl_whatisit(struct inode *inode, struct inode *realinode)
> @@ -465,6 +466,30 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
>                 return vfs_fsync_range(upperfile, start, end, datasync);
>  }
>
> +static unsigned long ovl_get_unmapped_area(struct file *file,
> +                                          unsigned long addr, unsigned long len,
> +                                          unsigned long pgoff, unsigned long flags)
> +{
> +       struct ovl_file *of = file->private_data;
> +       struct file *realfile = of->realfile;
> +
> +       /*
> +        * ovl_mmap() hands realfile to the underlying filesystem, so the vma
> +        * ends up backed by realfile.  Let that filesystem pick the address
> +        * too, or one that needs a specific alignment - to allow PMD mappings,
> +        * for example - never gets asked for one.
> +        */
> +       if (realfile->f_op->get_unmapped_area)
> +               return realfile->f_op->get_unmapped_area(realfile, addr, len,
> +                                                        pgoff, flags);
> +
> +#ifdef CONFIG_MMU
> +       return mm_get_unmapped_area(file, addr, len, pgoff, flags);
> +#endif

That looks quite ugly.
Can't we have a noop inline helper for mm_get_unmapped_area()
fo nommu?

> +
> +       return addr;
> +}
> +
>  static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>         struct ovl_file *of = file->private_data;
> @@ -654,6 +679,7 @@ const struct file_operations ovl_file_operations = {
>         .write_iter     = ovl_write_iter,
>         .fsync          = ovl_fsync,
>         .mmap           = ovl_mmap,
> +       .get_unmapped_area = ovl_get_unmapped_area,
>         .fallocate      = ovl_fallocate,
>         .fadvise        = ovl_fadvise,
>         .flush          = ovl_flush,
>
> base-commit: 26260251022fbc2f248a3d747a9b2b961b18d2d8

Since mmap is going through backing_file_mmap() probably a good idea to
pass this method through a backing_file helper as well.

But more importantly, adding MM people to this patch review, mainly the ones
that participated in the discussion over patch to fix similar issues
in 2024 [1].
Most of the concerns in that patch seem to have been addressed (?)

Thanks,
Amir.

[1] https://lore.kernel.org/linux-unionfs/20241205143038.3260233-1-tujinjiang@huawei.com/

Re: [PATCH] ovl: implement ->get_unmapped_area()

[Matthew Wilcox]

On Fri, Aug 28, 2026 at 02:23:11PM +0200, Amir Goldstein wrote:
> Since mmap is going through backing_file_mmap() probably a good idea to
> pass this method through a backing_file helper as well.
> 
> But more importantly, adding MM people to this patch review, mainly the ones
> that participated in the discussion over patch to fix similar issues
> in 2024 [1].
> Most of the concerns in that patch seem to have been addressed (?)
> 
> [1] https://lore.kernel.org/linux-unionfs/20241205143038.3260233-1-tujinjiang@huawei.com/

No, not at all.  All this patch needs to be is:

+++ b/fs/overlayfs/file.c
@@ -657,6 +657,7 @@ const struct file_operations ovl_file_operations = {
        .fallocate      = ovl_fallocate,
        .fadvise        = ovl_fadvise,
        .flush          = ovl_flush,
+       .get_unmapped_area = thp_get_unmapped_area,
        .splice_read    = ovl_splice_read,
        .splice_write   = ovl_splice_write,


All this "oh we have to call the underlying filesystem's
get_unmapepd_area" betrays a lack of understanding of the problem.

And Lorenzo suggested this in the linked thread:

https://lore.kernel.org/linux-unionfs/69b72e3d-b101-4641-9ce5-51346c93a98d@lucifer.local/