Some needed changes I noticed while trying to take this onto the
'pending' branch
On Mon, Oct 24, 2016 at 4:21 PM, Dave Jiang <dave.ji...@intel.com> wrote:
> Existing implementation defaults all pages allocated as 2M superpages.
> For nfit_test dax device we need 4k pages allocated to work properly.
> Adding an --align,-a option to provide the alignment. Will accept
> 4k and 2M at the moment as proper parameter. No -a option still defaults
> to 2M.
>
> Signed-off-by: Dave Jiang <dave.ji...@intel.com>
> ---
> ndctl/builtin-xaction-namespace.c | 22 ++++++++++++++++++++--
> util/size.h | 1 +
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/ndctl/builtin-xaction-namespace.c
> b/ndctl/builtin-xaction-namespace.c
> index 9b1702d..89ce6ce 100644
> --- a/ndctl/builtin-xaction-namespace.c
> +++ b/ndctl/builtin-xaction-namespace.c
> @@ -49,6 +49,7 @@ static struct parameters {
> const char *region;
> const char *reconfig;
> const char *sector_size;
> + const char *align;
> } param;
>
> void builtin_xaction_namespace_reset(void)
> @@ -71,6 +72,7 @@ struct parsed_parameters {
> enum ndctl_namespace_mode mode;
> unsigned long long size;
> unsigned long sector_size;
> + unsigned long align;
> };
>
> #define debug(fmt, ...) \
> @@ -104,6 +106,8 @@ OPT_STRING('l', "sector-size", ¶m.sector_size,
> "lba-size", \
> "specify the logical sector size in bytes"), \
> OPT_STRING('t', "type", ¶m.type, "type", \
> "specify the type of namespace to create 'pmem' or 'blk'"), \
> +OPT_STRING('a', "align", ¶m.align, "align", \
> + "specify the namespace alignment in bytes (default: 0x200000 (2M))"),
> \
> OPT_BOOLEAN('f', "force", &force, "reconfigure namespace even if currently
> active")
>
> static const struct option base_options[] = {
> @@ -319,7 +323,7 @@ static int setup_namespace(struct ndctl_region *region,
>
> try(ndctl_pfn, set_uuid, pfn, uuid);
> try(ndctl_pfn, set_location, pfn, p->loc);
> - try(ndctl_pfn, set_align, pfn, SZ_2M);
> + try(ndctl_pfn, set_align, pfn, p->align);
This will now collide wit the new "ndctl_pfn_has_align()" check that
got added to fix support for pre-4.5 kernels.
> try(ndctl_pfn, set_namespace, pfn, ndns);
> rc = ndctl_pfn_enable(pfn);
> } else if (p->mode == NDCTL_NS_MODE_DAX) {
> @@ -327,7 +331,7 @@ static int setup_namespace(struct ndctl_region *region,
>
> try(ndctl_dax, set_uuid, dax, uuid);
> try(ndctl_dax, set_location, dax, p->loc);
> - try(ndctl_dax, set_align, dax, SZ_2M);
> + try(ndctl_dax, set_align, dax, p->align);
> try(ndctl_dax, set_namespace, dax, ndns);
> rc = ndctl_dax_enable(dax);
> } else if (p->mode == NDCTL_NS_MODE_SAFE) {
> @@ -383,6 +387,20 @@ static int validate_namespace_options(struct
> ndctl_region *region,
>
> memset(p, 0, sizeof(*p));
>
> + if (param.align) {
> + p->align = parse_size64(param.align);
> + switch (p->align) {
> + case SZ_4K:
> + case SZ_2M:
> + break;
> + case SZ_1G: /* unsupported yet... */
> + default:
> + debug("%s: invalid align\n", __func__);
> + return -EINVAL;
> + }
> + } else
> + p->align = SZ_2M;
> +
I think this check should come after we have determined that the mode
is either "memory" or "dax", and error out otherwise. Also, when the
alignment is not 2M, we should check that the kernel has alignment
setting support with the new ndctl_pfn_has_align() api. Note that
kernels that support device-dax implicitly support the alignment
property.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
