[v5,1/7] libndctl: Use the supported_alignment attribute

Commit Message

Oliver O'Halloran Jan. 30, 2019, 10:59 a.m. UTC

Newer kernels provide the "supported_alignments" sysfs attribute that
indicates what alignments can be used with a PFN or DAX namespace. This
patch adds the plumbing inside of libndctl to allow users to query this
information through using:
	ndctl_{dax|pfn}_get_supported_alignment(), and
	ndctl_{dax|pfn}_get_num_alignments()

Signed-off-by: Oliver O'Halloran <[e-mail]>
---
v5: Fixed comment wording

v4: Changed return code of ndctl_pfn_get_supported_alignment from -1 to
    -1 to -EINVAL.

    Reworded comment about why we default to 4K and 2M alignments when
    the sysfs attribute is missing.

    Shuffled around prototypes in ndctl.h.

    80 char compliance fixes.

    rebased onto pending branch

v3: Changed the return type of the *_get_supported_alignment() functions
    to unsigned long to match the existing *_get_alignment() functions.
---
 ndctl/lib/libndctl.c   | 43 ++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym |  4 ++++
 ndctl/libndctl.h       |  4 ++++
 3 files changed, 51 insertions(+)

Comments

Verma, Vishal L Jan. 30, 2019, 11:20 p.m. UTC | #1

On Wed, 2019-01-30 at 21:59 +1100, Oliver O'Halloran wrote:
> Newer kernels provide the "supported_alignments" sysfs attribute that
> indicates what alignments can be used with a PFN or DAX namespace. This
> patch adds the plumbing inside of libndctl to allow users to query this
> information through using:
> 	ndctl_{dax|pfn}_get_supported_alignment(), and
> 	ndctl_{dax|pfn}_get_num_alignments()
> 
> Signed-off-by: Oliver O'Halloran <[e-mail]>
> ---
> v5: Fixed comment wording
> 
> v4: Changed return code of ndctl_pfn_get_supported_alignment from -1 to
>     -1 to -EINVAL.
> 
>     Reworded comment about why we default to 4K and 2M alignments when
>     the sysfs attribute is missing.
> 
>     Shuffled around prototypes in ndctl.h.
> 
>     80 char compliance fixes.
> 
>     rebased onto pending branch
> 
> v3: Changed the return type of the *_get_supported_alignment() functions
>     to unsigned long to match the existing *_get_alignment() functions.
> ---
>  ndctl/lib/libndctl.c   | 43 ++++++++++++++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym |  4 ++++
>  ndctl/libndctl.h       |  4 ++++
>  3 files changed, 51 insertions(+)
> 
Thanks for the reworks, Oliver. I've applied the series.

Dan Williams March 6, 2019, 6:32 a.m. UTC | #2

On Wed, Jan 30, 2019 at 2:59 AM Oliver O'Halloran <[e-mail]> wrote:
>
> Newer kernels provide the "supported_alignments" sysfs attribute that
> indicates what alignments can be used with a PFN or DAX namespace. This
> patch adds the plumbing inside of libndctl to allow users to query this
> information through using:
>         ndctl_{dax|pfn}_get_supported_alignment(), and
>         ndctl_{dax|pfn}_get_num_alignments()
>
> Signed-off-by: Oliver O'Halloran <[e-mail]>
> ---
> v5: Fixed comment wording
>
> v4: Changed return code of ndctl_pfn_get_supported_alignment from -1 to
>     -1 to -EINVAL.
>
>     Reworded comment about why we default to 4K and 2M alignments when
>     the sysfs attribute is missing.
>
>     Shuffled around prototypes in ndctl.h.
>
>     80 char compliance fixes.
>
>     rebased onto pending branch
>
> v3: Changed the return type of the *_get_supported_alignment() functions
>     to unsigned long to match the existing *_get_alignment() functions.

After playing with this facility in the v64 release I think it needs a
redo for the following reasons:

* It's a bit too chatty and redundant even behind a "-v" because all
namespaces share the same attributes,

* this breaks the long standing, but admittedly not documented, style
guideline that all json values have an associated key, even if the key
is effectively just a generic index value. There are common methods to
enumerate the key space, but key-less arrays need to be handled
explicitly.

* It's the only field in the json that does not reflect the current
state of a device. For this reason I think it should be moved behind a
--capabilities flag and a named "capability" object as a child of a
region. This would also help with the redundant "supported" prefix.

Patch

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 830b791339d2..06f835d76117 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -31,6 +31,7 @@ 
 #include <ccan/build_assert/build_assert.h>
 
 #include <ndctl.h>
+#include <util/size.h>
 #include <util/sysfs.h>
 #include <ndctl/libndctl.h>
 #include <ndctl/namespace.h>
@@ -237,6 +238,7 @@  struct ndctl_pfn {
 	int buf_len;
 	uuid_t uuid;
 	int id, generation;
+	struct ndctl_lbasize alignments;
 };
 
 struct ndctl_dax {
@@ -4814,6 +4816,19 @@  static void *__add_pfn(struct ndctl_pfn *pfn, const char *pfn_base)
 	else
 		pfn->size = strtoull(buf, NULL, 0);
 
+	/*
+	 * The supported_alignments attribute was added before arches other
+	 * than x86 had pmem support. If the kernel doesn't provide the
+	 * attribute then it's safe to assume that we running on x86 where
+	 * 4KiB and 2MiB have always been supported.
+	 */
+	sprintf(path, "%s/supported_alignments", pfn_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		sprintf(buf, "%d %d", SZ_4K, SZ_2M);
+
+	if (parse_lbasize_supported(ctx, pfn_base, buf, &pfn->alignments) < 0)
+		goto err_read;
+
 	free(path);
 	return pfn;
 
@@ -5048,6 +5063,23 @@  NDCTL_EXPORT int ndctl_pfn_set_align(struct ndctl_pfn *pfn, unsigned long align)
 	return 0;
 }
 
+NDCTL_EXPORT int ndctl_pfn_get_num_alignments(struct ndctl_pfn *pfn)
+{
+	return pfn->alignments.num;
+}
+
+NDCTL_EXPORT unsigned long ndctl_pfn_get_supported_alignment(
+		struct ndctl_pfn *pfn, int i)
+{
+	if (pfn->alignments.num == 0)
+		return 0;
+
+	if (i < 0 || i > pfn->alignments.num)
+		return -EINVAL;
+	else
+		return pfn->alignments.supported[i];
+}
+
 NDCTL_EXPORT int ndctl_pfn_set_namespace(struct ndctl_pfn *pfn,
 		struct ndctl_namespace *ndns)
 {
@@ -5270,6 +5302,17 @@  NDCTL_EXPORT unsigned long ndctl_dax_get_align(struct ndctl_dax *dax)
 	return ndctl_pfn_get_align(&dax->pfn);
 }
 
+NDCTL_EXPORT int ndctl_dax_get_num_alignments(struct ndctl_dax *dax)
+{
+	return ndctl_pfn_get_num_alignments(&dax->pfn);
+}
+
+NDCTL_EXPORT unsigned long ndctl_dax_get_supported_alignment(
+		struct ndctl_dax *dax, int i)
+{
+	return ndctl_pfn_get_supported_alignment(&dax->pfn, i);
+}
+
 NDCTL_EXPORT int ndctl_dax_has_align(struct ndctl_dax *dax)
 {
 	return ndctl_pfn_has_align(&dax->pfn);
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 275db92ee103..a30a93e3c012 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -390,4 +390,8 @@  LIBNDCTL_19 {
 global:
 	ndctl_cmd_xlat_firmware_status;
 	ndctl_cmd_submit_xlat;
+	ndctl_pfn_get_supported_alignment;
+	ndctl_pfn_get_num_alignments;
+	ndctl_dax_get_supported_alignment;
+	ndctl_dax_get_num_alignments;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index e55a5932781d..ac639b7d9142 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -597,7 +597,9 @@  int ndctl_pfn_set_uuid(struct ndctl_pfn *pfn, uuid_t uu);
 void ndctl_pfn_get_uuid(struct ndctl_pfn *pfn, uuid_t uu);
 int ndctl_pfn_has_align(struct ndctl_pfn *pfn);
 int ndctl_pfn_set_align(struct ndctl_pfn *pfn, unsigned long align);
+int ndctl_pfn_get_num_alignments(struct ndctl_pfn *pfn);
 unsigned long ndctl_pfn_get_align(struct ndctl_pfn *pfn);
+unsigned long ndctl_pfn_get_supported_alignment(struct ndctl_pfn *pfn, int i);
 unsigned long long ndctl_pfn_get_resource(struct ndctl_pfn *pfn);
 unsigned long long ndctl_pfn_get_size(struct ndctl_pfn *pfn);
 int ndctl_pfn_set_namespace(struct ndctl_pfn *pfn, struct ndctl_namespace *ndns);
@@ -628,7 +630,9 @@  unsigned long long ndctl_dax_get_resource(struct ndctl_dax *dax);
 int ndctl_dax_set_uuid(struct ndctl_dax *dax, uuid_t uu);
 enum ndctl_pfn_loc ndctl_dax_get_location(struct ndctl_dax *dax);
 int ndctl_dax_set_location(struct ndctl_dax *dax, enum ndctl_pfn_loc loc);
+int ndctl_dax_get_num_alignments(struct ndctl_dax *dax);
 unsigned long ndctl_dax_get_align(struct ndctl_dax *dax);
+unsigned long ndctl_dax_get_supported_alignment(struct ndctl_dax *dax, int i);
 int ndctl_dax_has_align(struct ndctl_dax *dax);
 int ndctl_dax_set_align(struct ndctl_dax *dax, unsigned long align);
 int ndctl_dax_set_namespace(struct ndctl_dax *dax,