On Mon, Jun 7, 2021 at 5:03 AM Joao Martins [off-list ref] wrote:
On 6/1/21 8:30 PM, Dan Williams wrote:
quoted
Sorry for the delay, and the sync up on IRC. I think this subject
needs to change to "optimize memory savings for compound pud memmap
geometry", and move this to the back of the series to make it clear
which patches are base functionality and which extend the idea
further.
OK
quoted
As far as I can see this patch can move to the end of the
series.
Maybe its prefered that this patch could be deferred to out of the series as
a followup improvement, and leave this series with the base feature set only?
My preference is to keep it in just clarify that it's an optimization
and make sure it does not come before any fundamental patches that
implement the base support.
quoted
Some additional changelog feedback below:
On Thu, Mar 25, 2021 at 4:10 PM Joao Martins [off-list ref] wrote:
quoted
Right now basepages are used to populate the PUD tail pages, and it
picks the address of the previous page of the subsection that preceeds
s/preceeds/precedes/
Yeap.
quoted
quoted
the memmap we are initializing. This is done when a given memmap
address isn't aligned to the pgmap @align (which is safe to do because
@ranges are guaranteed to be aligned to @align).
You know what would help is if you could draw an ascii art picture of
the before and after of the head page vs tail page arrangement. I can
see how the words line up to the code, but it takes a while to get the
picture in my head and I think future work in this area will benefit
from having a place in Documentation that draws a picture of the
layout of the various geometries.
Makes sense, I will add docs. Mike K. and others had similar trouble following
the page structs arrangement which ultimately lead to this section of commentary
at the beginning of the new source file added here ...
https://www.ozlabs.org/~akpm/mmotm/broken-out/mm-hugetlb-free-the-vmemmap-pages-associated-with-each-hugetlb-page.patch
Ah, looks good, but that really belongs in Documentation/ not a comment block.
quoted
I've used asciiflow.com for these types of diagrams in the past.
... so perhaps I can borrow some of that and place it to
a common place like in Documentation/vm/compound_pagemaps.rst
This patch specifically would need a new diagram added on top covering
the PMD page case.
Sounds good.
[..]
quoted
Other than that the implementation looks ok to me, modulo previous
comments about @block type and the use of the "geometry" term.
OK.
Btw, speaking of geometry, could you have a look at this thread:
https://lore.kernel.org/linux-mm/8c922a58-c901-1ad9-5d19-1182bd6dea1e@oracle.com/ (local)
.. and let me know what you think?
Ok, will take a look