* [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; 3+ 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 related [flat|nested] 3+ 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; 3+ 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] 3+ messages in thread
* Re: [PATCH] ovl: implement ->get_unmapped_area() 2026-08-28 12:23 ` Amir Goldstein @ 2026-08-28 12:41 ` Matthew Wilcox 0 siblings, 0 replies; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2026-08-28 12:42 UTC | newest] Thread overview: 3+ 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
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.