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

From: Jeff Moyer
Date: Fri Feb 14 2020 - 15:59:19 EST


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