public inbox for nvdimm@lists.linux.dev
 help / color / mirror / Atom feed
From: Alison Schofield <alison.schofield@intel.com>
To: nvdimm@lists.linux.dev, linux-cxl@vger.kernel.org
Cc: Alison Schofield <alison.schofield@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Andreas Hasenack <andreas.hasenack@canonical.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Dave Jiang <dave.jiang@intel.com>
Subject: [ndctl PATCH v3] cxl/list: remove libtracefs build dependency for --media-errors
Date: Thu, 25 Sep 2025 00:54:29 -0700	[thread overview]
Message-ID: <20250925075438.148935-1-alison.schofield@intel.com> (raw)

When the --media-errors option was added to cxl list it inadvertently
changed the optional libtracefs requirement into a mandatory one.
Ndctl versions 80,81,82 no longer build without libtracefs.

Remove that dependency.

When libtracefs is disabled the user will see a this message logged
to stderr:
	$ cxl list -r region0 --media-errors --targets
	cxl list: --media-errors support disabled at build time

...followed by the region listing including the output for any other
valid command line options, like --targets in the example above.

When libtracefs is disabled the cxl-poison.sh unit test is omitted.

The man page gets a note:
	The media-error option is only available with -Dlibtracefs=enabled.

Reported-by: Andreas Hasenack <andreas.hasenack@canonical.com>
Fixes: d7532bb049e0 ("cxl/list: add --media-errors option to cxl list")
Closes: https://github.com/pmem/ndctl/issues/289
Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Link: https://lore.kernel.org/r/20250924045302.90074-1-alison.schofield@intel.com
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

Changes in v3:
- Remove ifdef's from .c files (Dan)
  Move ifdef chunk in json.c to a new conditionally compiled json_poison.c.
  Move ifdef notice message to !ENABLE_LIBTRACEFS stub in event_trace.h.
Changes in v2:
- Notify and continue when --media-error info is unavailable (Dan)

 Documentation/cxl/cxl-list.txt |   2 +
 config.h.meson                 |   2 +-
 cxl/json.c                     | 180 --------------------------------
 cxl/json_poison.c              | 185 +++++++++++++++++++++++++++++++++
 cxl/meson.build                |   1 +
 test/meson.build               |   9 +-
 util/event_trace.h             |  14 +++
 7 files changed, 210 insertions(+), 183 deletions(-)
 create mode 100644 cxl/json_poison.c

diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
index 9a9911e7dd9b..0595638ee054 100644
--- a/Documentation/cxl/cxl-list.txt
+++ b/Documentation/cxl/cxl-list.txt
@@ -425,6 +425,8 @@ OPTIONS
 	"source:" is one of: External, Internal, Injected, Vendor Specific,
 	or Unknown, as defined in CXL Specification v3.1 Table 8-140.
 
+The media-errors option is only available with '-Dlibtracefs=enabled'.
+
 ----
 # cxl list -m mem9 --media-errors -u
 {
diff --git a/config.h.meson b/config.h.meson
index f75db3e6360f..e8539f8d04df 100644
--- a/config.h.meson
+++ b/config.h.meson
@@ -19,7 +19,7 @@
 /* ndctl test support */
 #mesondefine ENABLE_TEST
 
-/* cxl monitor support */
+/* cxl monitor and cxl list --media-errors support */
 #mesondefine ENABLE_LIBTRACEFS
 
 /* Define to 1 if big-endian-arch */
diff --git a/cxl/json.c b/cxl/json.c
index e65bd803b706..bde4589065e7 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -9,7 +9,6 @@
 #include <json-c/json.h>
 #include <json-c/printbuf.h>
 #include <ccan/short_types/short_types.h>
-#include <tracefs.h>
 
 #include "filter.h"
 #include "json.h"
@@ -575,185 +574,6 @@ err_jobj:
 	return NULL;
 }
 
-/* CXL Spec 3.1 Table 8-140 Media Error Record */
-#define CXL_POISON_SOURCE_MAX 7
-static const char *const poison_source[] = { "Unknown", "External", "Internal",
-					     "Injected", "Reserved", "Reserved",
-					     "Reserved", "Vendor Specific" };
-
-/* CXL Spec 3.1 Table 8-139 Get Poison List Output Payload */
-#define CXL_POISON_FLAG_MORE BIT(0)
-#define CXL_POISON_FLAG_OVERFLOW BIT(1)
-#define CXL_POISON_FLAG_SCANNING BIT(2)
-
-static int poison_event_to_json(struct tep_event *event,
-				struct tep_record *record,
-				struct event_ctx *e_ctx)
-{
-	struct cxl_poison_ctx *p_ctx = e_ctx->poison_ctx;
-	struct json_object *jp, *jobj, *jpoison = p_ctx->jpoison;
-	struct cxl_memdev *memdev = p_ctx->memdev;
-	struct cxl_region *region = p_ctx->region;
-	unsigned long flags = e_ctx->json_flags;
-	const char *region_name = NULL;
-	char flag_str[32] = { '\0' };
-	bool overflow = false;
-	u8 source, pflags;
-	u64 offset, ts;
-	u32 length;
-	char *str;
-	int len;
-
-	jp = json_object_new_object();
-	if (!jp)
-		return -ENOMEM;
-
-	/* Skip records not in this region when listing by region */
-	if (region)
-		region_name = cxl_region_get_devname(region);
-	if (region_name)
-		str = tep_get_field_raw(NULL, event, "region", record, &len, 0);
-	if ((region_name) && (strcmp(region_name, str) != 0)) {
-		json_object_put(jp);
-		return 0;
-	}
-	/* Include offset,length by region (hpa) or by memdev (dpa) */
-	if (region) {
-		offset = trace_get_field_u64(event, record, "hpa");
-		if (offset != ULLONG_MAX) {
-			offset = offset - cxl_region_get_resource(region);
-			jobj = util_json_object_hex(offset, flags);
-			if (jobj)
-				json_object_object_add(jp, "offset", jobj);
-		}
-	} else if (memdev) {
-		offset = trace_get_field_u64(event, record, "dpa");
-		if (offset != ULLONG_MAX) {
-			jobj = util_json_object_hex(offset, flags);
-			if (jobj)
-				json_object_object_add(jp, "offset", jobj);
-		}
-	}
-	length = trace_get_field_u32(event, record, "dpa_length");
-	jobj = util_json_object_size(length, flags);
-	if (jobj)
-		json_object_object_add(jp, "length", jobj);
-
-	/* Always include the poison source */
-	source = trace_get_field_u8(event, record, "source");
-	if (source <= CXL_POISON_SOURCE_MAX)
-		jobj = json_object_new_string(poison_source[source]);
-	else
-		jobj = json_object_new_string("Reserved");
-	if (jobj)
-		json_object_object_add(jp, "source", jobj);
-
-	/* Include flags and overflow time if present */
-	pflags = trace_get_field_u8(event, record, "flags");
-	if (pflags && pflags < UCHAR_MAX) {
-		if (pflags & CXL_POISON_FLAG_MORE)
-			strcat(flag_str, "More,");
-		if (pflags & CXL_POISON_FLAG_SCANNING)
-			strcat(flag_str, "Scanning,");
-		if (pflags & CXL_POISON_FLAG_OVERFLOW) {
-			strcat(flag_str, "Overflow,");
-			overflow = true;
-		}
-		jobj = json_object_new_string(flag_str);
-		if (jobj)
-			json_object_object_add(jp, "flags", jobj);
-	}
-	if (overflow) {
-		ts = trace_get_field_u64(event, record, "overflow_ts");
-		jobj = util_json_object_hex(ts, flags);
-		if (jobj)
-			json_object_object_add(jp, "overflow_t", jobj);
-	}
-	json_object_array_add(jpoison, jp);
-
-	return 0;
-}
-
-static struct json_object *
-util_cxl_poison_events_to_json(struct tracefs_instance *inst,
-			       struct cxl_poison_ctx *p_ctx,
-			       unsigned long flags)
-{
-	struct event_ctx ectx = {
-		.event_name = "cxl_poison",
-		.event_pid = getpid(),
-		.system = "cxl",
-		.poison_ctx = p_ctx,
-		.json_flags = flags,
-		.parse_event = poison_event_to_json,
-	};
-	int rc;
-
-	p_ctx->jpoison = json_object_new_array();
-	if (!p_ctx->jpoison)
-		return NULL;
-
-	rc = trace_event_parse(inst, &ectx);
-	if (rc < 0) {
-		fprintf(stderr, "Failed to parse events: %d\n", rc);
-		goto put_jobj;
-	}
-	if (json_object_array_length(p_ctx->jpoison) == 0)
-		goto put_jobj;
-
-	return p_ctx->jpoison;
-
-put_jobj:
-	json_object_put(p_ctx->jpoison);
-	return NULL;
-}
-
-static struct json_object *
-util_cxl_poison_list_to_json(struct cxl_region *region,
-			     struct cxl_memdev *memdev,
-			     unsigned long flags)
-{
-	struct json_object *jpoison = NULL;
-	struct cxl_poison_ctx p_ctx;
-	struct tracefs_instance *inst;
-	int rc;
-
-	inst = tracefs_instance_create("cxl list");
-	if (!inst) {
-		fprintf(stderr, "tracefs_instance_create() failed\n");
-		return NULL;
-	}
-
-	rc = trace_event_enable(inst, "cxl", "cxl_poison");
-	if (rc < 0) {
-		fprintf(stderr, "Failed to enable trace: %d\n", rc);
-		goto err_free;
-	}
-
-	if (region)
-		rc = cxl_region_trigger_poison_list(region);
-	else
-		rc = cxl_memdev_trigger_poison_list(memdev);
-	if (rc)
-		goto err_free;
-
-	rc = trace_event_disable(inst);
-	if (rc < 0) {
-		fprintf(stderr, "Failed to disable trace: %d\n", rc);
-		goto err_free;
-	}
-
-	p_ctx = (struct cxl_poison_ctx){
-		.region = region,
-		.memdev = memdev,
-	};
-	jpoison = util_cxl_poison_events_to_json(inst, &p_ctx, flags);
-
-err_free:
-	tracefs_instance_free(inst);
-	return jpoison;
-}
-
 struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 		unsigned long flags)
 {
diff --git a/cxl/json_poison.c b/cxl/json_poison.c
new file mode 100644
index 000000000000..c8f18c73b83d
--- /dev/null
+++ b/cxl/json_poison.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2015-2025 Intel Corporation. All rights reserved.
+#include <errno.h>
+#include <util/bitmap.h>
+#include <util/json.h>
+#include <cxl/libcxl.h>
+
+#include "../util/event_trace.h"
+
+/* CXL Spec 3.1 Table 8-140 Media Error Record */
+#define CXL_POISON_SOURCE_MAX 7
+static const char *const poison_source[] = { "Unknown", "External", "Internal",
+					     "Injected", "Reserved", "Reserved",
+					     "Reserved", "Vendor Specific" };
+
+/* CXL Spec 3.1 Table 8-139 Get Poison List Output Payload */
+#define CXL_POISON_FLAG_MORE BIT(0)
+#define CXL_POISON_FLAG_OVERFLOW BIT(1)
+#define CXL_POISON_FLAG_SCANNING BIT(2)
+
+static int poison_event_to_json(struct tep_event *event,
+				struct tep_record *record,
+				struct event_ctx *e_ctx)
+{
+	struct cxl_poison_ctx *p_ctx = e_ctx->poison_ctx;
+	struct json_object *jp, *jobj, *jpoison = p_ctx->jpoison;
+	struct cxl_memdev *memdev = p_ctx->memdev;
+	struct cxl_region *region = p_ctx->region;
+	unsigned long flags = e_ctx->json_flags;
+	const char *region_name = NULL;
+	char flag_str[32] = { '\0' };
+	bool overflow = false;
+	u8 source, pflags;
+	u64 offset, ts;
+	u32 length;
+	char *str;
+	int len;
+
+	jp = json_object_new_object();
+	if (!jp)
+		return -ENOMEM;
+
+	/* Skip records not in this region when listing by region */
+	if (region)
+		region_name = cxl_region_get_devname(region);
+	if (region_name)
+		str = tep_get_field_raw(NULL, event, "region", record, &len, 0);
+	if ((region_name) && (strcmp(region_name, str) != 0)) {
+		json_object_put(jp);
+		return 0;
+	}
+	/* Include offset,length by region (hpa) or by memdev (dpa) */
+	if (region) {
+		offset = trace_get_field_u64(event, record, "hpa");
+		if (offset != ULLONG_MAX) {
+			offset = offset - cxl_region_get_resource(region);
+			jobj = util_json_object_hex(offset, flags);
+			if (jobj)
+				json_object_object_add(jp, "offset", jobj);
+		}
+	} else if (memdev) {
+		offset = trace_get_field_u64(event, record, "dpa");
+		if (offset != ULLONG_MAX) {
+			jobj = util_json_object_hex(offset, flags);
+			if (jobj)
+				json_object_object_add(jp, "offset", jobj);
+		}
+	}
+	length = trace_get_field_u32(event, record, "dpa_length");
+	jobj = util_json_object_size(length, flags);
+	if (jobj)
+		json_object_object_add(jp, "length", jobj);
+
+	/* Always include the poison source */
+	source = trace_get_field_u8(event, record, "source");
+	if (source <= CXL_POISON_SOURCE_MAX)
+		jobj = json_object_new_string(poison_source[source]);
+	else
+		jobj = json_object_new_string("Reserved");
+	if (jobj)
+		json_object_object_add(jp, "source", jobj);
+
+	/* Include flags and overflow time if present */
+	pflags = trace_get_field_u8(event, record, "flags");
+	if (pflags && pflags < UCHAR_MAX) {
+		if (pflags & CXL_POISON_FLAG_MORE)
+			strcat(flag_str, "More,");
+		if (pflags & CXL_POISON_FLAG_SCANNING)
+			strcat(flag_str, "Scanning,");
+		if (pflags & CXL_POISON_FLAG_OVERFLOW) {
+			strcat(flag_str, "Overflow,");
+			overflow = true;
+		}
+		jobj = json_object_new_string(flag_str);
+		if (jobj)
+			json_object_object_add(jp, "flags", jobj);
+	}
+	if (overflow) {
+		ts = trace_get_field_u64(event, record, "overflow_ts");
+		jobj = util_json_object_hex(ts, flags);
+		if (jobj)
+			json_object_object_add(jp, "overflow_t", jobj);
+	}
+	json_object_array_add(jpoison, jp);
+
+	return 0;
+}
+
+static struct json_object *
+util_cxl_poison_events_to_json(struct tracefs_instance *inst,
+			       struct cxl_poison_ctx *p_ctx,
+			       unsigned long flags)
+{
+	struct event_ctx ectx = {
+		.event_name = "cxl_poison",
+		.event_pid = getpid(),
+		.system = "cxl",
+		.poison_ctx = p_ctx,
+		.json_flags = flags,
+		.parse_event = poison_event_to_json,
+	};
+	int rc;
+
+	p_ctx->jpoison = json_object_new_array();
+	if (!p_ctx->jpoison)
+		return NULL;
+
+	rc = trace_event_parse(inst, &ectx);
+	if (rc < 0) {
+		fprintf(stderr, "Failed to parse events: %d\n", rc);
+		goto put_jobj;
+	}
+	if (json_object_array_length(p_ctx->jpoison) == 0)
+		goto put_jobj;
+
+	return p_ctx->jpoison;
+
+put_jobj:
+	json_object_put(p_ctx->jpoison);
+	return NULL;
+}
+
+struct json_object *util_cxl_poison_list_to_json(struct cxl_region *region,
+			struct cxl_memdev *memdev, unsigned long flags)
+{
+	struct json_object *jpoison = NULL;
+	struct cxl_poison_ctx p_ctx;
+	struct tracefs_instance *inst;
+	int rc;
+
+	inst = tracefs_instance_create("cxl list");
+	if (!inst) {
+		fprintf(stderr, "tracefs_instance_create() failed\n");
+		return NULL;
+	}
+
+	rc = trace_event_enable(inst, "cxl", "cxl_poison");
+	if (rc < 0) {
+		fprintf(stderr, "Failed to enable trace: %d\n", rc);
+		goto err_free;
+	}
+
+	if (region)
+		rc = cxl_region_trigger_poison_list(region);
+	else
+		rc = cxl_memdev_trigger_poison_list(memdev);
+	if (rc)
+		goto err_free;
+
+	rc = trace_event_disable(inst);
+	if (rc < 0) {
+		fprintf(stderr, "Failed to disable trace: %d\n", rc);
+		goto err_free;
+	}
+
+	p_ctx = (struct cxl_poison_ctx){
+		.region = region,
+		.memdev = memdev,
+	};
+	jpoison = util_cxl_poison_events_to_json(inst, &p_ctx, flags);
+
+err_free:
+	tracefs_instance_free(inst);
+	return jpoison;
+}
diff --git a/cxl/meson.build b/cxl/meson.build
index e4d1683ce8c6..b9924ae8efea 100644
--- a/cxl/meson.build
+++ b/cxl/meson.build
@@ -29,6 +29,7 @@ if get_option('libtracefs').enabled()
   cxl_src += [
     '../util/event_trace.c',
     'monitor.c',
+    'json_poison.c',
   ]
   deps += [
     traceevent,
diff --git a/test/meson.build b/test/meson.build
index 775542c1b787..615376ea635a 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -167,7 +167,6 @@ cxl_events = find_program('cxl-events.sh')
 cxl_sanitize = find_program('cxl-sanitize.sh')
 cxl_destroy_region = find_program('cxl-destroy-region.sh')
 cxl_qos_class = find_program('cxl-qos-class.sh')
-cxl_poison = find_program('cxl-poison.sh')
 
 tests = [
   [ 'libndctl',               libndctl,		  'ndctl' ],
@@ -200,7 +199,6 @@ tests = [
   [ 'cxl-sanitize.sh',        cxl_sanitize,       'cxl'   ],
   [ 'cxl-destroy-region.sh',  cxl_destroy_region, 'cxl'   ],
   [ 'cxl-qos-class.sh',       cxl_qos_class,      'cxl'   ],
-  [ 'cxl-poison.sh',          cxl_poison,         'cxl'   ],
 ]
 
 if get_option('destructive').enabled()
@@ -253,6 +251,13 @@ if get_option('fwctl').enabled()
   ]
 endif
 
+if get_option('libtracefs').enabled()
+  cxl_poison = find_program('cxl-poison.sh')
+  tests += [
+    [ 'cxl-poison.sh', cxl_poison, 'cxl' ],
+  ]
+endif
+
 test_env = [
     'LC_ALL=C',
     'NDCTL=@0@'.format(ndctl_tool.full_path()),
diff --git a/util/event_trace.h b/util/event_trace.h
index a87407ff8296..989c02ec2259 100644
--- a/util/event_trace.h
+++ b/util/event_trace.h
@@ -7,6 +7,9 @@
 #include <ccan/list/list.h>
 #include <ccan/short_types/short_types.h>
 
+#ifdef ENABLE_LIBTRACEFS
+#include <tracefs.h>
+
 struct jlist_node {
 	struct json_object *jobj;
 	struct list_node list;
@@ -39,4 +42,15 @@ u32 trace_get_field_u32(struct tep_event *event, struct tep_record *record,
 			const char *name);
 u64 trace_get_field_u64(struct tep_event *event, struct tep_record *record,
 			const char *name);
+struct json_object *util_cxl_poison_list_to_json(struct cxl_region *region,
+			struct cxl_memdev *memdev, unsigned long flags);
+#else
+struct json_object *util_cxl_poison_list_to_json(struct cxl_region *region,
+			struct cxl_memdev *memdev, unsigned long flags)
+{
+	fprintf(stderr, "cxl list --media-errors support disabled at build time\n");
+	return NULL;
+}
+#endif /* ENABLE_LIBTRACEFS */
+
 #endif
-- 
2.37.3


             reply	other threads:[~2025-09-25  7:54 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-25  7:54 Alison Schofield [this message]
2025-09-25 15:55 ` [ndctl PATCH v3] cxl/list: remove libtracefs build dependency for --media-errors dan.j.williams
2025-09-25 17:04   ` Alison Schofield

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250925075438.148935-1-alison.schofield@intel.com \
    --to=alison.schofield@intel.com \
    --cc=andreas.hasenack@canonical.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox