Re: [PATCH 2 1/2] ndctl: introduce 4k allocation support for creating namespace

[Vishal Verma]

On 12/08, Dave Jiang 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 |   34 +++++++++++++++++++++++++++++++---
>  util/size.h                       |    1 +
>  2 files changed, 32 insertions(+), 3 deletions(-)

A patch for bash completiion for this is at the bottom.

A minor nit below, but other than that:

Reviewed-by: Vishal Verma <vishal.l.ve...@intel.com>

> 
> diff --git a/ndctl/builtin-xaction-namespace.c 
> b/ndctl/builtin-xaction-namespace.c
> index 8257eb9..038e59f 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", &param.sector_size, 
> "lba-size", \
>       "specify the logical sector size in bytes"), \
>  OPT_STRING('t', "type", &param.type, "type", \
>       "specify the type of namespace to create 'pmem' or 'blk'"), \
> +OPT_STRING('a', "align", &param.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,14 +323,13 @@ static int setup_namespace(struct ndctl_region *region,
>  
>               try(ndctl_pfn, set_uuid, pfn, uuid);
>               try(ndctl_pfn, set_location, pfn, p->loc);
> -
>               /*
>                * TODO: when we allow setting a non-default alignment
>                * we'll need to check for "has_align" earlier and fail
>                * non-default attempts on older kernels.
>                */
>               if (ndctl_pfn_has_align(pfn))
> -                     try(ndctl_pfn, set_align, pfn, SZ_2M);
> +                     try(ndctl_pfn, set_align, pfn, p->align);
>               try(ndctl_pfn, set_namespace, pfn, ndns);
>               rc = ndctl_pfn_enable(pfn);
>       } else if (p->mode == NDCTL_NS_MODE_DAX) {
> @@ -335,7 +338,7 @@ static int setup_namespace(struct ndctl_region *region,
>               try(ndctl_dax, set_uuid, dax, uuid);
>               try(ndctl_dax, set_location, dax, p->loc);
>               /* device-dax assumes 'align' attribute present */
> -             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) {
> @@ -439,6 +442,31 @@ static int validate_namespace_options(struct 
> ndctl_region *region,
>       } else if (ndns)
>               p->mode = ndctl_namespace_get_mode(ndns);
>  
> +     if ((p->mode == NDCTL_NS_MODE_MEMORY) ||
> +                     (p->mode == NDCTL_NS_MODE_DAX)) {
> +             struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
> +
> +             if (!ndctl_pfn_has_align(pfn))
> +                     return -EINVAL;
> +
> +             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__);

s/align/alignment ?

<snip>

8<----

>From e705e371fd3c5ab00572db30955f2ee18ba74134 Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.ve...@intel.com>
Date: Thu, 8 Dec 2016 17:16:45 -0700
Subject: [ndctl PATCH] ndctl: add the new --align option to bash completion

Add completion for --align, and provide only the expected options of 4K,
2M, and 1G.

Signed-off-by: Vishal Verma <vishal.l.ve...@intel.com>
---
 contrib/ndctl | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contrib/ndctl b/contrib/ndctl
index ea7303c..162d47f 100755
--- a/contrib/ndctl
+++ b/contrib/ndctl
@@ -91,7 +91,7 @@ __ndctlcomp()
 
        COMPREPLY=( $( compgen -W "$1" -- "$2" ) )
        for cword in "${COMPREPLY[@]}"; do
-               if [[ "$cword" == 
@(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace)
 ]]; then
+               if [[ "$cword" == 
@(--bus|--region|--type|--mode|--size|--dimm|--reconfig|--uuid|--name|--sector-size|--map|--namespace|--align)
 ]]; then
                        COMPREPLY[$i]="${cword}="
                else
                        COMPREPLY[$i]="${cword} "
@@ -171,6 +171,9 @@ __ndctl_comp_options()
                --map)
                        opts="mem dev"
                        ;;
+               --align)
+                       opts="4K 2M 1G"
+                       ;;
                *)
                        return
                        ;;
-- 
2.7.4

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm