mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ovl: implement ->get_unmapped_area()
@ 2026-08-27 17:07 Yuan-Hao Hsu
  2026-08-28 12:23 ` Amir Goldstein
  0 siblings, 1 reply; 5+ messages in thread
From: Yuan-Hao Hsu @ 2026-08-27 17:07 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein; +Cc: linux-unionfs, linux-kernel, Yuan-Hao Hsu

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
+
+	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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ovl: implement ->get_unmapped_area()
  2026-08-27 17:07 [PATCH] ovl: implement ->get_unmapped_area() Yuan-Hao Hsu
@ 2026-08-28 12:23 ` Amir Goldstein
  2026-08-28 12:41   ` Matthew Wilcox
  0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2026-08-28 12:23 UTC (permalink / raw)
  To: Yuan-Hao Hsu
  Cc: Miklos Szeredi, linux-unionfs, linux-kernel, Andrew Morton,
	Lorenzo Stoakes, Vlastimil Babka, Jann Horn, David Hildenbrand,
	Linux MM, Matthew Wilcox, Christian Brauner, zhangyi (F),
	tujinjiang

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/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ovl: implement ->get_unmapped_area()
  2026-08-28 12:23 ` Amir Goldstein
@ 2026-08-28 12:41   ` Matthew Wilcox
  2026-08-31  8:18     ` Miklos Szeredi
  0 siblings, 1 reply; 5+ messages in thread
From: Matthew Wilcox @ 2026-08-28 12:41 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Yuan-Hao Hsu, Miklos Szeredi, linux-unionfs, linux-kernel,
	Andrew Morton, Lorenzo Stoakes, Vlastimil Babka, Jann Horn,
	David Hildenbrand, Linux MM, Christian Brauner, zhangyi (F),
	tujinjiang

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/


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ovl: implement ->get_unmapped_area()
  2026-08-28 12:41   ` Matthew Wilcox
@ 2026-08-31  8:18     ` Miklos Szeredi
  2026-09-14 11:42       ` Yuan-Hao Hsu
  0 siblings, 1 reply; 5+ messages in thread
From: Miklos Szeredi @ 2026-08-31  8:18 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Amir Goldstein, Yuan-Hao Hsu, linux-unionfs, linux-kernel,
	Andrew Morton, Lorenzo Stoakes, Vlastimil Babka, Jann Horn,
	David Hildenbrand, Linux MM, Christian Brauner, zhangyi (F),
	tujinjiang

On Fri, 28 Aug 2026 at 14:41, Matthew Wilcox <willy@infradead.org> wrote:

> 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/

There seems to be a logical fallacy in saying thp_get_unmapepd_area is
the most suitable value, yet not making it the default for all
filesystems.

Saying this without any understanding of the subtle difference between
setting .get_unmapepd_area to NULL and to thp_get_unmapepd_area.   All
I can see that the difference is indeed subtle.

Can you please explain?

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ovl: implement ->get_unmapped_area()
  2026-08-31  8:18     ` Miklos Szeredi
@ 2026-09-14 11:42       ` Yuan-Hao Hsu
  0 siblings, 0 replies; 5+ messages in thread
From: Yuan-Hao Hsu @ 2026-09-14 11:42 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Matthew Wilcox, Amir Goldstein, Andrew Morton, Lorenzo Stoakes,
	Vlastimil Babka, Jann Horn, David Hildenbrand, Christian Brauner,
	Zhang Yi, Jinjiang Tu, linux-unionfs, linux-mm, linux-kernel

On Mon, 31 Aug 2026 at 16:18, Miklos Szeredi <miklos@szeredi.hu> wrote:
> There seems to be a logical fallacy in saying thp_get_unmapepd_area is
> the most suitable value, yet not making it the default for all
> filesystems.
>
> Saying this without any understanding of the subtle difference between
> setting .get_unmapepd_area to NULL and to thp_get_unmapepd_area.   All
> I can see that the difference is indeed subtle.
>
> Can you please explain?

I had a look at this.  thp_get_unmapped_area() aligns anything with
room for a whole PMD in it; nothing asks whether the file can use it.
NULL means no alignment at all, and 34d7cf637c43 kept files there to
avoid "unnecessary VMA fragmentation and potentially worse performance
on filesystems that do not actually support THPs".  The case where
that matters is a lower without large folios.  32 x 3M files,
mmap(NULL, ...):

                          ovl over ext4           ovl over squashfs
  NULL (today)          16/32 aligned, no gaps  16/32 aligned, no gaps
  delegate (v1)         32/32, 31744K of gaps   16/32, no gaps
  thp_get_unmapped_area 32/32, 31744K of gaps   32/32, 31744K of gaps

squashfs has no large folios, so the alignment buys nothing there
(fault counts were identical) and costs 1M of address space per
mapping.  v1 only avoids that by inheriting the lower's choice.  fuse
passthrough already pays the same, as Jann noted in the 2024 thread.

Hope this helps.

Thanks,
Yuan-Hao Hsu

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-14 11:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-27 17:07 [PATCH] ovl: implement ->get_unmapped_area() Yuan-Hao Hsu
2026-08-28 12:23 ` Amir Goldstein
2026-08-28 12:41   ` Matthew Wilcox
2026-08-31  8:18     ` Miklos Szeredi
2026-09-14 11:42       ` Yuan-Hao Hsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®