Re: [PATCH -next] ovl: respect underlying filesystem's get_unmapped_area()

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

(fixing typo in cc list: tujinjiang@huawe.com -> tujinjiang@huawei.com)

+ Liam

(JinJiang - you forgot to cc the correct maintainers, please ensure you run
scripts/get_maintainers.pl on files you change)

On Thu, Dec 05, 2024 at 04:12:12PM +0100, Amir Goldstein wrote:
> On Thu, Dec 5, 2024 at 4:04 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > + Matthew for large folio aspect
> >
> > On Thu, Dec 05, 2024 at 10:30:38PM +0800, Jinjiang Tu wrote:
> > > During our tests in containers, there is a read-only file (i.e., shared
> > > libraies) in the overlayfs filesystem, and the underlying filesystem is
> > > ext4, which supports large folio. We mmap the file with PROT_READ prot,
> > > and then call madvise(MADV_COLLAPSE) for it. However, the madvise call
> > > fails and returns EINVAL.
> > >
> > > The reason is that the mapping address isn't aligned to PMD size. Since
> > > overlayfs doesn't support large folio, __get_unmapped_area() doesn't call
> > > thp_get_unmapped_area() to get a THP aligned address.
> > >
> > > To fix it, call get_unmapped_area() with the realfile.
> >
> > Isn't the correct solution to get overlayfs to support large folios?
> >
> > >
> > > Besides, since overlayfs may be built with CONFIG_OVERLAY_FS=m, we should
> > > export get_unmapped_area().
> >
> > Yeah, not in favour of this at all. This is an internal implementation
> > detail. It seems like you're trying to hack your way into avoiding
> > providing support for large folios and to hand it off to the underlying
> > file system.
> >
> > Again, why don't you just support large folios in overlayfs?
> >
>
> This whole discussion seems moot.
> overlayfs does not have address_space operations
> It does not have its own page cache.

And here we see my total lack of knowledge of overlayfs coming into play
here :) Thanks for pointing this out.

In that case, I object even further to the original of course...

>
> The file in  vma->vm_file is not an overlayfs file at all - it is the
> real (e.g. ext4) file
> when returning from ovl_mmap() => backing_file_mmap()
> so I have very little clue why the proposed solution even works,
> but it certainly does not look correct.

I think then Jinjiang in this cause you ought to go back to the drawing
board and reconsider what might be the underlying issue here.

>
> Thanks,
> Amir.

Cheers, Lorenzo