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