Re: [PATCH v2 1/4] mm/memremap_pages: Introduce memremap_compat_align()

[Jeff Moyer]

Dan Williams <dan.j.williams@xxxxxxxxx> writes:

> On Thu, Feb 13, 2020 at 8:58 AM Jeff Moyer <jmoyer@xxxxxxxxxx> wrote:

>> I have just a couple of questions.
>>
>> First, can you please add a comment above the generic implementation of
>> memremap_compat_align describing its purpose, and why a platform might
>> want to override it?
>
> Sure, how about:
>
> /*
>  * The memremap() and memremap_pages() interfaces are alternately used
>  * to map persistent memory namespaces. These interfaces place different
>  * constraints on the alignment and size of the mapping (namespace).
>  * memremap() can map individual PAGE_SIZE pages. memremap_pages() can
>  * only map subsections (2MB), and at least one architecture (PowerPC)
>  * the minimum mapping granularity of memremap_pages() is 16MB.
>  *
>  * The role of memremap_compat_align() is to communicate the minimum
>  * arch supported alignment of a namespace such that it can freely
>  * switch modes without violating the arch constraint. Namely, do not
>  * allow a namespace to be PAGE_SIZE aligned since that namespace may be
>  * reconfigured into a mode that requires SUBSECTION_SIZE alignment.
>  */

Well, if we modify the x86 variant to be PAGE_SIZE, I think that text
won't work.  How about:

/*
 * memremap_compat_align should return the minimum alignment for
 * mapping memory via memremap() and memremap_pages().  For x86, this
 * is the system PAGE_SIZE.  Other architectures may impose different
 * restrictions, as is seen on powerpc where the minimum alignment is
 * tied to the linear mapping page size.
 *
 * When creating persistent memory namespaces, the alignment is forced
 * to the least common denominator (MEMREMAP_COMPAT_ALIGN_MAX,
 * currently 16MB).  However, older kernels did not enforce this
 * behavior, so we allow mapping namespaces with smaller alignments,
 * so long as the platform supports it.  See nvdimm_namespace_common_probe.
 */

-Jeff