On Tue, Jun 17, 2025 at 08:18:07PM -0300, Jason Gunthorpe wrote: > On Tue, Jun 17, 2025 at 04:56:13PM -0400, Peter Xu wrote: > > On Mon, Jun 16, 2025 at 08:00:11PM -0300, Jason Gunthorpe wrote: > > > On Mon, Jun 16, 2025 at 06:06:23PM -0400, Peter Xu wrote: > > > > > > > Can I understand it as a suggestion to pass in a bitmask into the core mm > > > > API (e.g. keep the name of mm_get_unmapped_area_aligned()), instead of a > > > > constant "align", so that core mm would try to allocate from the largest > > > > size to smaller until it finds some working VA to use? > > > > > > I don't think you need a bitmask. > > > > > > Split the concerns, the caller knows what is inside it's FD. It only > > > needs to provide the highest pgoff aligned folio/pfn within the FD. > > > > Ultimately I even dropped this hint. I found that it's not really > > get_unmapped_area()'s job to detect over-sized pgoffs. It's mmap()'s job. > > So I decided to avoid this parameter as of now. > > Well, the point of the pgoff is only what you said earlier, to adjust > the starting alignment so the pgoff aligned high order folios/pfns > line up properly. I meant "highest pgoff" that I dropped. We definitely need the pgoff to make it work. So here I dropped "highest pgoff" passed from the caller because I decided to leave such check to the mmap() hook later. > > > > The mm knows what leaf page tables options exist. It should try to > > > align to the closest leaf page table size that is <= the FD's max > > > aligned folio. > > > > So again IMHO this is also not per-FD information, but needs to be passed > > over from the driver for each call. > > It is per-FD in the sense that each FD is unique and each range of > pgoff could have a unique maximum. > > > Likely the "order" parameter appeared in other discussions to imply a > > maximum supported size from the driver side (or, for a folio, but that is > > definitely another user after this series can land). > > Yes, it is the only information the driver can actually provide and > comes directly from what it will install in the VMA. > > > So far I didn't yet add the "order", because currently VFIO definitely > > supports all max orders the system supports. Maybe we can add the order > > when there's a real need, but maybe it won't happen in the near > > future? > > The purpose of the order is to prevent over alignment and waste of > VMA. Your technique to use the length to limit alignment instead is > good enough for VFIO but not very general. Yes that's also something I didn't like. I think I'll just go ahead and add the order parameter, then use it in previous patch too. I'll wait for some more time though for others' input before a respin. Thanks, > > The VFIO part looks pretty good, I still don't really understand why > you'd have CONFIG_ARCH_SUPPORTS_HUGE_PFNMAP though. The inline > fallback you have for it seems good enough and we don't care if things > are overaligned for ioremap. > > Jason > -- Peter Xu
