Hi Dave,

Thanks for taking a look at this! Some comments below...

On 16/02/2024 00:04, Dave Chinner wrote:

On Thu, Feb 15, 2024 at 03:40:59PM +0000, Ryan Roberts wrote:

quoted

Change the readahead config so that if it is being requested for an
executable mapping, do a synchronous read of an arch-specified size in a
naturally aligned manner.

On arm64 if memory is physically contiguous and naturally aligned to the
"contpte" size, we can use contpte mappings, which improves utilization
of the TLB. When paired with the "multi-size THP" changes, this works
well to reduce dTLB pressure. However iTLB pressure is still high due to
executable mappings having a low liklihood of being in the required
folio size and mapping alignment, even when the filesystem supports
readahead into large folios (e.g. XFS).

The reason for the low liklihood is that the current readahead algorithm
starts with an order-2 folio and increases the folio order by 2 every
time the readahead mark is hit. But most executable memory is faulted in
fairly randomly and so the readahead mark is rarely hit and most
executable folios remain order-2.

Yup, this is a bug in the readahead code, and really has nothing to
do with executable files, mmap or the architecture.  We don't want
some magic new VM_EXEC min folio size per architecture thingy to be
set - we just want readahead to do the right thing.

It sounds like we agree that there is a bug but we don't agree on what the bug
is? My view is that executable segments are accessed in a ~random manner and
therefore readahead (as currently configured) is not very useful. But data may
well be accessed more sequentially and therefore readahead is useful. Given both
data and text can come from the same file, I don't think this can just be a
mapping setting? (my understanding is that there is one "mapping" for the whole
file?) So we need to look to VM_EXEC for that decision.

Indeed, we are already adding a mapping minimum folio order
directive to the address space to allow for filesystem block sizes
greater than PAGE_SIZE. That's the generic mechanism that this
functionality requires. See here:

https://lore.kernel.org/linux-xfs/20240213093713.1753368-5-kernel@pankajraghav.com/ (local)

Great, I'm vaguely aware of this work, but haven't looked in detail. I'll go
read it. But from your brief description, IIUC, this applies to the whole file,
and is a constraint put in place by the filesystem? Applying to the whole file
may make sense - that means more opportunity for contpte mappings for data pages
too, although I guess this adds more scope for write amplificaiton because data
tends to be writable, and text isn't. But for my use case, its not a hard
constraint, its just a preference which can improve performance. And the
filesystem is the wrong place to make the decision; its the arch that knows
about the performacne opportunities with different block mapping sizes.

As a side note, concerns have been expressed about the possibility of physical
memory fragmentation becoming problematic, meaning we degrade back to small
folios over time with my mTHP work. The intuituon is that if the whole system is
using a few folio sizes in ~equal quantities then we might be ok, but I don't
have any data yet. Do you have any data on fragmentation? I guess this could be
more concerning for your use case?

(Probably worth reading some of the other readahead mods in that
series and the discussion because readahead needs to ensure that it
fill entire high order folios in a single IO to avoid partial folio
up-to-date states from partial reads.)

IOWs, it seems to me that we could use this proposed generic mapping
min order functionality when mmap() is run and VM_EXEC is set to set
the min order to, say, 64kB. Then the readahead code would simply do
the right thing, as would all other reads and writes to that
mapping.

Ahh yes, hooking into your new logic to set a min order based on VM_EXEC sounds
perfect...

We could trigger this in the ->mmap() method of the filesystem so
that filesysetms that can use large folios can turn it on, whilst
other filesystems remain blissfully unaware of the functionality.
Filesystems could also do smarter things here, too. eg. enable PMD
alignment for large mapped files....

...but I don't think the filesystem is the right place. The size preference
should be driven by arch IMHO.

Thanks,
Ryan

-Dave.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel