Re: [RFC PATCH 1/4] libnvdimm/namespace: Make namespace size validation arch dependent

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

Hi Dan,

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:

> Dan Williams <dan.j.williams@intel.com> writes:
>
>> On Wed, Oct 30, 2019 at 10:35 PM Aneesh Kumar K.V
>> <aneesh.kumar@linux.ibm.com> wrote:
>> [..]
>>> > True, for the pfn device and the device-dax mapping size, but I'm
>>> > suggesting adding another instance of alignment control at the raw
>>> > namespace level. That would need to be disconnected from the
>>> > device-dax page mapping granularity.
>>> >
>>>
>>> Can you explain what you mean by raw namespace level ? We don't have
>>> multiple values against which we need to check the alignment of
>>> namespace start and namespace size.
>>>
>>> If you can outline how and where you would like to enforce that check I
>>> can start working on it.
>>>
>>
>> What I mean is that the process of setting up a pfn namespace goes
>> something like this in shell script form:
>>
>> 1/ echo $size > /sys/bus/nd/devices/$namespace/size
>> 2/ echo $namespace > /sys/bus/nd/devices/$pfn/namespace
>> 3/ echo $pfn_align > /sys/bus/nd/devices/$pfn/align
>>
>> What I'm suggesting is add an optional 0th step that does:
>>
>> echo $raw_align > /sys/bus/nd/devices/$namespace/align
>>
>> Where the raw align needs to be needs to be max($pfn_align,
>> arch_mapping_granulariy).
>
>
> I started looking at this and was wondering about userspace being aware
> of the direct-map mapping size. We can figure that out by parsing kernel
> log
>
> [    0.000000] Page orders: linear mapping = 24, virtual = 16, io = 16, vmemmap = 24
>
>
> But I am not sure we want to do that. There is not set of raw_align
> value to select. What we need to make sure is the below.
>
> 1) While creating a namespace we need to make sure that namespace size
> is multiple of direct-map mapping size. If we ensure that
> size is aligned, we should also get the namespace start to be aligned?
>
> 2) While initialzing a namespace by scanning label area, we need to make
> sure every namespace in the region satisfy the above requirement. If not
> we should mark the region disabled. 
>
>
>>
>> So on powerpc where PAGE_SIZE < arch_mapping_granulariy, the following:
>>
>> cat /sys/bus/nd/devices/$namespace/supported_aligns
>>
>> ...would show the same output as:
>>
>> cat /sys/bus/nd/devices/$pfn/align
>>
>> ...but with any alignment choice less than arch_mapping_granulariy removed.
>>
>
> I am not sure why we need to do that. For example: even if we have
> direct-map mapping size as PAGE_SIZE (64K), we still should allow an user
> mapping with hugepage size (16M)?
>


Considering the direct-map map size is not going to be user selectable,
do you agree that we can skip the above step 0 configuration you
suggested.

The changes proposed in the patch series essentially does the rest.

1) It validate the size against the arch specific limit during
namespace creation. (part of step 1)
2) It also disable initializing a region if it find the size not
correctly aligned as per the platform requirement.
3) Direct map  mapping size is different from supported_alignment for a
namespace. The supported alignment controls what possible PAGE SIZE user want the
namespace to be mapped to user space.


With the above do you think the current patch series is good?

-aneesh