Hi Vishal,

Thanks for your comments.

On 7/8/2021 8:21 AM, Verma, Vishal L wrote:
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.

It's based on the current 'master' branch.
I don't see a LIBNDCTL_26 section, just 'LIBNDCTL_25'.
How about adding 'ndctl_dimm_sizeof_namespace_index' to LIBNDCTL_25 section ?

Thanks,
Jingqi

Reply via email to