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

[Dan Williams]

On Sat, Nov 16, 2019 at 4:15 AM Aneesh Kumar K.V
<aneesh.kumar at linux.ibm.com> wrote:
>
>
> Hi Dan,
>
> "Aneesh Kumar K.V" <aneesh.kumar at linux.ibm.com> writes:
>
> > Dan Williams <dan.j.williams at intel.com> writes:
> >
> >> On Wed, Oct 30, 2019 at 10:35 PM Aneesh Kumar K.V
> >> <aneesh.kumar at 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)

This validation is a surprise failure to ndctl.

> 2) It also disable initializing a region if it find the size not
> correctly aligned as per the platform requirement.

There needs to be a way for the user to discover the correct alignment
that the kernel will accept.

> 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.

No, the namespace alignment is different than the page mapping size.
The alignment is only interpreted as a mapping size at the device-dax
level, otherwise at the raw namespace level it's just an arbitrary
alignment.

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

I don't think we've quite converged on a solution.