On Tue, May 29, 2018 at 4:15 PM, Vishal Verma <[email protected]> wrote:
> For 'fsdax' and 'devdax' namespaces, a 'map' location may be specified
> for page structures storage. This can be 'mem', for system RAM, or 'dev'
> for using pmem as the backing storage. Once set, there was no way of
> telling using ndctl, which of the two locations a namespace was
> configured for. Add this in util_namespace_to_json so that all
> namespace listings contain the map location.
>
> Reported-by: "Yigal Korman" <[email protected]>
> Cc: Dan Williams <[email protected]>
> Signed-off-by: Vishal Verma <[email protected]>
> ---
> util/json.c | 32 +++++++++++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> v2: Also account for memmap=ss!nn or legacy-e820 namespaces. (Dan)
>
> diff --git a/util/json.c b/util/json.c
> index c606e1c..b020300 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -667,11 +667,17 @@ struct json_object *util_namespace_to_json(struct
> ndctl_namespace *ndns,
> {
> struct json_object *jndns = json_object_new_object();
> struct json_object *jobj, *jbbs = NULL;
> + const char *locations[] = {
> + [NDCTL_PFN_LOC_NONE] = "none",
> + [NDCTL_PFN_LOC_RAM] = "mem",
> + [NDCTL_PFN_LOC_PMEM] = "dev",
> + };
> unsigned long long size = ULLONG_MAX;
> unsigned int sector_size = UINT_MAX;
> enum ndctl_namespace_mode mode;
> const char *bdev = NULL, *name;
> unsigned int bb_count = 0;
> + enum ndctl_pfn_loc loc;
we could initialize loc to NDCTL_PFN_LOC_NONE here.
> struct ndctl_btt *btt;
> struct ndctl_pfn *pfn;
> struct ndctl_dax *dax;
> @@ -693,33 +699,49 @@ struct json_object *util_namespace_to_json(struct
> ndctl_namespace *ndns,
> mode = ndctl_namespace_get_mode(ndns);
> switch (mode) {
> case NDCTL_NS_MODE_MEMORY:
> - if (pfn) /* dynamic memory mode */
> + jobj = json_object_new_string("fsdax");
> + if (jobj)
> + json_object_object_add(jndns, "mode", jobj);
> + loc = ndctl_pfn_get_location(pfn);
NULL ptr de-reference if pfn is NULL
> + if (pfn) { /* dynamic memory mode */
> size = ndctl_pfn_get_size(pfn);
> - else /* native/static memory mode */
> + jobj = json_object_new_string(locations[loc]);
> + } else { /* native/static memory mode */
> size = ndctl_namespace_get_size(ndns);
> - jobj = json_object_new_string("fsdax");
> + jobj = json_object_new_string("mem");
We could just set NDCTL_PFN_LOC_RAM here.
> + }
> + if (jobj)
> + json_object_object_add(jndns, "map", jobj);
> break;
> case NDCTL_NS_MODE_DAX:
> if (!dax)
> goto err;
> size = ndctl_dax_get_size(dax);
> jobj = json_object_new_string("devdax");
> + if (jobj)
> + json_object_object_add(jndns, "mode", jobj);
> + loc = ndctl_dax_get_location(dax);
> + jobj = json_object_new_string(locations[loc]);
> + if (jobj)
> + json_object_object_add(jndns, "map", jobj);
> break;
> case NDCTL_NS_MODE_SAFE:
> if (!btt)
> goto err;
> jobj = json_object_new_string("sector");
> + if (jobj)
> + json_object_object_add(jndns, "mode", jobj);
> size = ndctl_btt_get_size(btt);
> break;
> case NDCTL_NS_MODE_RAW:
> size = ndctl_namespace_get_size(ndns);
> jobj = json_object_new_string("raw");
> + if (jobj)
> + json_object_object_add(jndns, "mode", jobj);
> break;
> default:
> jobj = NULL;
> }
> - if (jobj)
> - json_object_object_add(jndns, "mode", jobj);
Why is mode getting moved into each case?
I'd put the
jobj = json_object_new_string(locations[loc]);
if (jobj)
json_object_object_add(jndns, "map", jobj);
...at the end here and just gate it on mode != NDCTL_NS_MODE_SAFE &&
mode != NDCTL_NS_MODE_RAW
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
