On Fri, Jan 29, 2021 at 2:12 PM Pavel Tatashin
[off-list ref] wrote:

On Fri, Jan 29, 2021 at 2:06 PM Pavel Tatashin
[off-list ref] wrote:

quoted

quoted

quoted

Definitely, but we should try figuring out what's going on here. I
assume on x86-64 it behaves differently?

Yes, we should root cause. I highly suspect that there is somewhere
alignment miscalculations happen that cause this memory waste with the
offset 16M. I am also not sure why the 2M label size was increased,
and  why 16M is now an alignment requirement.

This appears to be because even if we set vmemmap to be outside of the
dax device, the alignment calculates the maximum size of vmemmap for
this device, and subtracts it from the devdax size.
See [1], line 795 is where this offset is calculated.

This also explains why with 64K pages, the 16M offset worked: because
fewer struct pages were able to fit within 16M - label size.

[1] https://soleen.com/source/xref/linux/drivers/nvdimm/pfn_devs.c?r=b7b3c01b&mo=18459&fi=718#795

Actually, strike the previous e-mail. The extra space is when we
reserve vmemmap from devdax. IFF we do it from mem, the extra space is
not added. Now, this alignment makes total sense.

commit 2522afb86a8cceba0f67dbf05772d21b76d79f06
Author: Dan Williams [off-list ref]
Date:   Thu Jan 30 12:06:23 2020 -0800

    libnvdimm/region: Introduce an 'align' attribute


This is the patch that introduced the 16M alignment.

/*
 * PowerPC requires this alignment for memremap_pages(). All other archs
 * should be ok with SUBSECTION_SIZE (see memremap_compat_align()).
 */
#define MEMREMAP_COMPAT_ALIGN_MAX SZ_16M

static unsigned long default_align(struct nd_region *nd_region)
{
        unsigned long align;
        int i, mappings;
        u32 remainder;

        if (is_nd_blk(&nd_region->dev))
                align = PAGE_SIZE;
        else
                align = MEMREMAP_COMPAT_ALIGN_MAX;

Dan, is this logic correct? Why is_nd_pmem() cannot be set to
SUBSECTION_SIZE alignment?

Thank you,
Pasha

Pasha