On Wed, 2021-06-09 at 11:06 +0800, Jingqi Liu wrote:
> The following bug is caused by setting the size of Label Index Block
> to a fixed 256 bytes.
>
> Use the following Qemu command to start a Guest with 2MB label-size:
> -object
> memory-backend-file,id=mem1,share=on,mem-path=/dev/dax1.1,size=14G,align=2M
> -device nvdimm,memdev=mem1,id=nv1,label-size=2M
>
> There is a namespace in the Guest as follows:
> $ ndctl list
> [
> {
> "dev":"namespace0.0",
> "mode":"devdax",
> "map":"dev",
> "size":14780727296,
> "uuid":"58ad5282-5a16-404f-b8ee-e28b4c784eb8",
> "chardev":"dax0.0",
> "align":2097152,
> "name":"namespace0.0"
> }
> ]
>
> Fail to read labels. The result is as follows:
> $ ndctl read-labels -u nmem0
> [
> ]
> read 0 nmem
>
> If using the following Qemu command to start the Guest with 128K
> label-size, this label can be read correctly.
> -object
> memory-backend-file,id=mem1,share=on,mem-path=/dev/dax1.1,size=14G,align=2M
> -device nvdimm,memdev=mem1,id=nv1,label-size=128K
>
> The size of a Label Index Block depends on how many label slots fit into
> the label storage area. The minimum size of an index block is 256 bytes
> and the size must be a multiple of 256 bytes. For a storage area of 128KB,
> the corresponding Label Index Block size is 256 bytes. But if the label
> storage area is not 128KB, the Label Index Block size should not be 256 bytes.
>
> Namespace Label Index Block appears twice at the top of the label storage
> area.
> Following the two index blocks, an array for storing labels takes up the
> remainder of the label storage area.
>
> For obtaining the size of Namespace Index Block, we also cannot rely on
> the field of 'mysize' in this index block since it might be corrupted.
> Similar to the linux kernel, we use sizeof_namespace_index() to get the size
> of Namespace Index Block. Then we can also correctly calculate the starting
> offset of the following namespace labels.
>
> Suggested-by: Dan Williams <[email protected]>
> Signed-off-by: Jingqi Liu <[email protected]>
> ---
> ndctl/dimm.c | 19 +++++++++++++++----
> ndctl/lib/dimm.c | 5 +++++
> ndctl/lib/libndctl.sym | 1 +
> ndctl/libndctl.h | 1 +
> 4 files changed, 22 insertions(+), 4 deletions(-)
Hi Jingqi,
This looks fine, one comment below.
[..]
>
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index 0a82616..0ce2bb9 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -290,6 +290,7 @@ global:
> ndctl_dimm_validate_labels;
> ndctl_dimm_init_labels;
> ndctl_dimm_sizeof_namespace_label;
> + ndctl_dimm_sizeof_namespace_index;
This can't go into an 'old' section of the symbol version script - if
you base off the current 'pending' branch, you should see a LIBNDCTL_26
section at the bottom. You can add this there.
> ndctl_mapping_get_position;
> ndctl_namespace_set_enforce_mode;
> ndctl_namespace_get_enforce_mode;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index 60e1288..9a1a799 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -335,6 +335,7 @@ int ndctl_dimm_init_labels(struct ndctl_dimm *dimm,
> enum ndctl_namespace_version v);
> unsigned long ndctl_dimm_get_available_labels(struct ndctl_dimm *dimm);
> unsigned int ndctl_dimm_sizeof_namespace_label(struct ndctl_dimm *dimm);
> +unsigned int ndctl_dimm_sizeof_namespace_index(struct ndctl_dimm *dimm);
> unsigned int ndctl_cmd_cfg_size_get_size(struct ndctl_cmd *cfg_size);
> ssize_t ndctl_cmd_cfg_read_get_data(struct ndctl_cmd *cfg_read, void *buf,
> unsigned int len, unsigned int offset);
