nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count
@ 2018-09-28  0:41 Dan Williams
  2018-09-28  0:41 ` [ndctl PATCH v3 1/3] ndctl, lib: Add dirty-shutdown-count retrieval helper Dan Williams
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Dan Williams @ 2018-09-28  0:41 UTC (permalink / raw)
  To: linux-nvdimm

Changes since v2 [1]:
* Drop the 'dirty-dimm' command
* Add ndctl_dimm_get_dirty_shutdown() api

[1]: https://lists.01.org/pipermail/linux-nvdimm/2018-September/017890.html

---

The latch mechanism is awkward especially when all that it needed is a
rolling count of dirty-shutdown events. The expectation going forward is
that the platform firmware will handle the latch, if it is present, and
the OS need only consume the dirty-shutdown count. The ndctl
implementation called libndctl apis from the udev queue which we
discovered injects unnecessary udev queue drains / stalls into the boot
path. Lastly, the userspace caching scheme for non-root users to consume
the dirty-shutdown-count just isn't as efficient as teaching the kernel
to cache this value and export it as a standard sysfs attribute.

---

Dan Williams (3):
      ndctl, lib: Add dirty-shutdown-count retrieval helper
      ndctl: Revert "ndctl, intel: Fallback to smart cached shutdown_count"
      ndctl: Revert "ndctl: Create ndctl udev rules for dirty shutdown"


 .gitignore             |    1 
 Makefile.am            |    3 -
 configure.ac           |   10 ---
 contrib/80-ndctl.rules |    3 -
 ndctl.spec.in          |    3 -
 ndctl/Makefile.am      |    5 --
 ndctl/lib/intel.c      |   41 -------------
 ndctl/lib/libndctl.c   |   16 ++++-
 ndctl/lib/libndctl.sym |    5 ++
 ndctl/lib/private.h    |    4 -
 ndctl/libndctl.h       |    1 
 ndctl/ndctl-udev.c     |  150 ------------------------------------------------
 test/libndctl.c        |   29 +++++++--
 13 files changed, 40 insertions(+), 231 deletions(-)
 delete mode 100644 contrib/80-ndctl.rules
 delete mode 100644 ndctl/ndctl-udev.c
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [ndctl PATCH v3 1/3] ndctl, lib: Add dirty-shutdown-count retrieval helper
  2018-09-28  0:41 [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count Dan Williams
@ 2018-09-28  0:41 ` Dan Williams
  2018-09-28  0:41 ` [ndctl PATCH v3 2/3] ndctl: Revert "ndctl, intel: Fallback to smart cached shutdown_count" Dan Williams
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2018-09-28  0:41 UTC (permalink / raw)
  To: linux-nvdimm

The kernel now exports nmemX/nfit/dirty_shutdown for DIMMs/platforms
that provide a free running dirty-shutdown-counter.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/libndctl.c   |   10 ++++++++++
 ndctl/lib/libndctl.sym |    5 +++++
 ndctl/lib/private.h    |    1 +
 ndctl/libndctl.h       |    1 +
 test/libndctl.c        |   29 ++++++++++++++++++++++-------
 5 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 481b110d2a18..737b1de2c930 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1322,6 +1322,7 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
 	dimm->device_id = -1;
 	dimm->revision_id = -1;
 	dimm->health_eventfd = -1;
+	dimm->dirty_shutdown = -ENOENT;
 	dimm->subsystem_vendor_id = -1;
 	dimm->subsystem_device_id = -1;
 	dimm->subsystem_revision_id = -1;
@@ -1387,6 +1388,10 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->revision_id = strtoul(buf, NULL, 0);
 
+	sprintf(path, "%s/nfit/dirty_shutdown", dimm_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0)
+		dimm->dirty_shutdown = strtoll(buf, NULL, 0);
+
 	sprintf(path, "%s/nfit/subsystem_vendor", dimm_base);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->subsystem_vendor_id = strtoul(buf, NULL, 0);
@@ -1489,6 +1494,11 @@ NDCTL_EXPORT unsigned short ndctl_dimm_get_revision(struct ndctl_dimm *dimm)
 	return dimm->revision_id;
 }
 
+NDCTL_EXPORT long long ndctl_dimm_get_dirty_shutdown(struct ndctl_dimm *dimm)
+{
+	return dimm->dirty_shutdown;
+}
+
 NDCTL_EXPORT unsigned short ndctl_dimm_get_subsystem_vendor(
 		struct ndctl_dimm *dimm)
 {
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index a6849ee1fa4a..f1421a92bb09 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -378,3 +378,8 @@ global:
 	ndctl_region_get_max_available_extent;
 	ndctl_cmd_smart_inject_ctrl_temperature;
 } LIBNDCTL_16;
+
+LIBNDCTL_18 {
+global:
+	ndctl_dimm_get_dirty_shutdown;
+} LIBNDCTL_17;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index b6e438aff8c7..55fcd4e0ad05 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -76,6 +76,7 @@ struct ndctl_dimm {
 	unsigned long cmd_family;
 	unsigned long cmd_mask;
 	unsigned long nfit_dsm_mask;
+	long long dirty_shutdown;
 	char *unique_id;
 	char *dimm_path;
 	char *dimm_buf;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 83d6c6cae18d..7d164f688681 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -150,6 +150,7 @@ unsigned short ndctl_dimm_get_phys_id(struct ndctl_dimm *dimm);
 unsigned short ndctl_dimm_get_vendor(struct ndctl_dimm *dimm);
 unsigned short ndctl_dimm_get_device(struct ndctl_dimm *dimm);
 unsigned short ndctl_dimm_get_revision(struct ndctl_dimm *dimm);
+long long ndctl_dimm_get_dirty_shutdown(struct ndctl_dimm *dimm);
 unsigned short ndctl_dimm_get_subsystem_vendor(struct ndctl_dimm *dimm);
 unsigned short ndctl_dimm_get_subsystem_device(struct ndctl_dimm *dimm);
 unsigned short ndctl_dimm_get_manufacturing_date(struct ndctl_dimm *dimm);
diff --git a/test/libndctl.c b/test/libndctl.c
index e36c1fb5e2e5..50365f00e5fe 100644
--- a/test/libndctl.c
+++ b/test/libndctl.c
@@ -118,6 +118,7 @@ struct dimm {
 	unsigned int subsystem_vendor;
 	unsigned short manufacturing_date;
 	unsigned char manufacturing_location;
+	long long dirty_shutdown;
 	union {
 		unsigned long flags;
 		struct {
@@ -136,15 +137,15 @@ struct dimm {
 	(((n & 0xfff) << 16) | ((s & 0xf) << 12) | ((i & 0xf) << 8) \
 	 | ((c & 0xf) << 4) | (d & 0xf))
 static struct dimm dimms0[] = {
-	{ DIMM_HANDLE(0, 0, 0, 0, 0), 0, 0, 2016, 10, { 0 }, 2, { 0x201, 0x301, }, },
-	{ DIMM_HANDLE(0, 0, 0, 0, 1), 1, 0, 2016, 10, { 0 }, 2, { 0x201, 0x301, }, },
-	{ DIMM_HANDLE(0, 0, 1, 0, 0), 2, 0, 2016, 10, { 0 }, 2, { 0x201, 0x301, }, },
-	{ DIMM_HANDLE(0, 0, 1, 0, 1), 3, 0, 2016, 10, { 0 }, 2, { 0x201, 0x301, }, },
+	{ DIMM_HANDLE(0, 0, 0, 0, 0), 0, 0, 2016, 10, 42, { 0 }, 2, { 0x201, 0x301, }, },
+	{ DIMM_HANDLE(0, 0, 0, 0, 1), 1, 0, 2016, 10, 42, { 0 }, 2, { 0x201, 0x301, }, },
+	{ DIMM_HANDLE(0, 0, 1, 0, 0), 2, 0, 2016, 10, 42, { 0 }, 2, { 0x201, 0x301, }, },
+	{ DIMM_HANDLE(0, 0, 1, 0, 1), 3, 0, 2016, 10, 42, { 0 }, 2, { 0x201, 0x301, }, },
 };
 
 static struct dimm dimms1[] = {
 	{
-		DIMM_HANDLE(0, 0, 0, 0, 0), 0, 0, 2016, 10, {
+		DIMM_HANDLE(0, 0, 0, 0, 0), 0, 0, 2016, 10, 42, {
 			.f_arm = 1,
 			.f_save = 1,
 			.f_flush = 1,
@@ -2195,7 +2196,7 @@ static int check_set_config_data(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
  */
 struct smart {
 	unsigned int flags, health, temperature, spares, alarm_flags,
-		     life_used, shutdown_state, vendor_size;
+		     life_used, shutdown_state, shutdown_count, vendor_size;
 };
 
 static int check_smart(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
@@ -2211,6 +2212,7 @@ static int check_smart(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
 		.alarm_flags = ND_SMART_SPARE_TRIP | ND_SMART_TEMP_TRIP,
 		.life_used = 5,
 		.shutdown_state = 0,
+		.shutdown_count = 42,
 		.vendor_size = 0,
 	};
 	struct ndctl_cmd *cmd = ndctl_dimm_cmd_new_smart(dimm);
@@ -2230,7 +2232,8 @@ static int check_smart(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
 		return rc;
 	}
 
-	__check_smart(dimm, cmd, flags, ~ND_SMART_CTEMP_VALID);
+	__check_smart(dimm, cmd, flags, ~(ND_SMART_CTEMP_VALID
+			| ND_SMART_SHUTDOWN_COUNT_VALID));
 	__check_smart(dimm, cmd, health, -1);
 	__check_smart(dimm, cmd, temperature, -1);
 	__check_smart(dimm, cmd, spares, -1);
@@ -2238,6 +2241,8 @@ static int check_smart(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
 	__check_smart(dimm, cmd, life_used, -1);
 	__check_smart(dimm, cmd, shutdown_state, -1);
 	__check_smart(dimm, cmd, vendor_size, -1);
+	if (ndctl_cmd_smart_get_flags(cmd) & ND_SMART_SHUTDOWN_COUNT_VALID)
+		__check_smart(dimm, cmd, shutdown_count, -1);
 
 	check->cmd = cmd;
 	return 0;
@@ -2468,6 +2473,7 @@ static int check_dimms(struct ndctl_bus *bus, struct dimm *dimms, int n,
 		unsigned long bus_commands, unsigned long dimm_commands,
 		struct ndctl_test *test)
 {
+	long long dsc;
 	int i, j, rc;
 
 	for (i = 0; i < n; i++) {
@@ -2552,6 +2558,15 @@ static int check_dimms(struct ndctl_bus *bus, struct dimm *dimms, int n,
 			}
 		}
 
+		dsc = ndctl_dimm_get_dirty_shutdown(dimm);
+		if (dsc != -ENOENT && dsc != dimms[i].dirty_shutdown) {
+			fprintf(stderr,
+				"dimm%d expected dirty shutdown: %lld got: %lld\n",
+				i, dimms[i].dirty_shutdown,
+				ndctl_dimm_get_dirty_shutdown(dimm));
+			return -ENXIO;
+		}
+
 		rc = check_commands(bus, dimm, bus_commands, dimm_commands, test);
 		if (rc)
 			return rc;

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [ndctl PATCH v3 2/3] ndctl: Revert "ndctl, intel: Fallback to smart cached shutdown_count"
  2018-09-28  0:41 [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count Dan Williams
  2018-09-28  0:41 ` [ndctl PATCH v3 1/3] ndctl, lib: Add dirty-shutdown-count retrieval helper Dan Williams
@ 2018-09-28  0:41 ` Dan Williams
  2018-09-28  0:41 ` [ndctl PATCH v3 3/3] ndctl: Revert "ndctl: Create ndctl udev rules for dirty shutdown" Dan Williams
  2018-10-03  1:08 ` [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count Verma, Vishal L
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2018-09-28  0:41 UTC (permalink / raw)
  To: linux-nvdimm

Static device attribute data, if it needs to be cached and exported to
unprivileged userspace, should live in sysfs.

Cc: Keith Busch <keith.busch@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/intel.c    |   41 -----------------------------------------
 ndctl/lib/libndctl.c |    6 +-----
 ndctl/lib/private.h  |    3 ---
 3 files changed, 1 insertion(+), 49 deletions(-)

diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index 15c5d1a60e34..0abea1e1f5ff 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -56,46 +56,6 @@ static struct ndctl_cmd *alloc_intel_cmd(struct ndctl_dimm *dimm,
 	return cmd;
 }
 
-/*
- * If provided, read the cached shutdown count in case the user does not have
- * access rights to run the smart command, and pretend the command was
- * successful with only the shutdown_count valid.
- */
-static int intel_smart_handle_error(struct ndctl_cmd *cmd)
-{
-	struct ndctl_dimm *dimm = cmd->dimm;
-	struct ndctl_bus *bus = cmd_to_bus(cmd);
-	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
-	char *path = NULL, shutdown_count[16] = {};
-	int fd, rc = cmd->status;
-
-	if (!dimm)
-		goto out;
-
-	if (asprintf(&path, DEF_TMPFS_DIR "/%s/usc",
-		     ndctl_dimm_get_devname(dimm)) < 0)
-		goto out;
-
-	fd = open(path, O_RDONLY);
-	if (fd < 0)
-		goto free_path;
-
-	if (read(fd, shutdown_count, sizeof(shutdown_count)) < 0)
-		goto close;
-
-	cmd->intel->smart.flags = ND_INTEL_SMART_SHUTDOWN_COUNT_VALID;
-	cmd->intel->smart.shutdown_count = strtoull(shutdown_count, NULL, 0);
-	rc = cmd->status = 0;
- close:
-	close (fd);
- free_path:
-	free(path);
- out:
-	if (rc)
-		err(ctx, "failed: %s\n", strerror(errno));
-	return rc;
-}
-
 static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
 {
 	struct ndctl_cmd *cmd;
@@ -107,7 +67,6 @@ static struct ndctl_cmd *intel_dimm_cmd_new_smart(struct ndctl_dimm *dimm)
 	if (!cmd)
 		return NULL;
 	cmd->firmware_status = &cmd->intel->smart.status;
-	cmd->handle_error = intel_smart_handle_error;
 
 	return cmd;
 }
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 737b1de2c930..a7b6cc520085 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2763,9 +2763,7 @@ NDCTL_EXPORT int ndctl_cmd_submit(struct ndctl_cmd *cmd)
 
 	fd = open(path, O_RDWR);
 	if (fd < 0) {
-		if (!cmd->handle_error)
-			err(ctx, "failed to open %s: %s\n", path,
-				strerror(errno));
+		err(ctx, "failed to open %s: %s\n", path, strerror(errno));
 		rc = -errno;
 		goto out;
 	}
@@ -2781,8 +2779,6 @@ NDCTL_EXPORT int ndctl_cmd_submit(struct ndctl_cmd *cmd)
 	close(fd);
  out:
 	cmd->status = rc;
-	if (rc && cmd->handle_error)
-		rc = cmd->handle_error(cmd);
 	return rc;
 }
 
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 55fcd4e0ad05..d3c3e48535e1 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -228,8 +228,6 @@ struct ndctl_namespace {
  * @firmware_status: NFIT command output status code
  * @iter: iterator for multi-xfer commands
  * @source: source cmd of an inherited iter.total_buf
- * @handle_error: function pointer to handle a cmd error and override it to
- * 		  return alternative data (from a cache for example).
  *
  * For dynamically sized commands like 'get_config', 'set_config', or
  * 'vendor', @size encompasses the entire buffer for the command input
@@ -258,7 +256,6 @@ struct ndctl_cmd {
 		int dir;
 	} iter;
 	struct ndctl_cmd *source;
-	int (*handle_error)(struct ndctl_cmd *cmd);
 	union {
 		struct nd_cmd_ars_cap ars_cap[0];
 		struct nd_cmd_ars_start ars_start[0];

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [ndctl PATCH v3 3/3] ndctl: Revert "ndctl: Create ndctl udev rules for dirty shutdown"
  2018-09-28  0:41 [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count Dan Williams
  2018-09-28  0:41 ` [ndctl PATCH v3 1/3] ndctl, lib: Add dirty-shutdown-count retrieval helper Dan Williams
  2018-09-28  0:41 ` [ndctl PATCH v3 2/3] ndctl: Revert "ndctl, intel: Fallback to smart cached shutdown_count" Dan Williams
@ 2018-09-28  0:41 ` Dan Williams
  2018-10-03  1:08 ` [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count Verma, Vishal L
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2018-09-28  0:41 UTC (permalink / raw)
  To: linux-nvdimm

Environments that choose to implement dirty-shutdown mitigation can use
'list --dimm --health nmemX' to retrieve the dirty-shutdown count, or
the new 'nmemX/nfit/dirty_shutdown' attribute in sysfs. The kernel
otherwise takes no default action on that counter rolling. If the
platform advertises support for "Memory Controller Flush to NVDIMM
Durability on Power Loss Capable" then the default Linux policy is to
trust that designation.

Otherwise this usage of libndctl enumeration apis in the udev path
causes the entire udev queue to backup behind libnvdimm sub-system
initialization:

    INFO: task systemd-udevd:1554 blocked for more than 120 seconds.
    "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
    systemd-udevd   D    0  1554   1544 0x80000004
    Call Trace:
     ? __schedule+0x25d/0x870
     schedule+0x28/0x80
     async_synchronize_cookie_domain+0x96/0x140
     ? finish_wait+0x80/0x80
     nvdimm_bus_check_dimm_count+0x31/0xa0 [libnvdimm]
     acpi_nfit_register_dimms+0x4b5/0x800 [nfit]
     acpi_nfit_init+0xca6/0x1300 [nfit]
     ? acpi_nfit_add+0x197/0x210 [nfit]
     acpi_nfit_add+0x197/0x210 [nfit]
     acpi_device_probe+0x48/0x110

Cc: Keith Busch <keith.busch@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 .gitignore             |    1 
 Makefile.am            |    3 -
 configure.ac           |   10 ---
 contrib/80-ndctl.rules |    3 -
 ndctl.spec.in          |    3 -
 ndctl/Makefile.am      |    5 --
 ndctl/ndctl-udev.c     |  150 ------------------------------------------------
 7 files changed, 175 deletions(-)
 delete mode 100644 contrib/80-ndctl.rules
 delete mode 100644 ndctl/ndctl-udev.c

diff --git a/.gitignore b/.gitignore
index f13a7ef5a608..0baace4b457e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,7 +25,6 @@ daxctl/lib/libdaxctl.pc
 *.a
 ndctl/lib/libndctl.pc
 ndctl/ndctl
-ndctl/ndctl-udev
 rhel/
 sles/ndctl.spec
 util/log.lo
diff --git a/Makefile.am b/Makefile.am
index 7880eef1c890..e0c463a3493a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,9 +42,6 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
 dist_bashcompletion_DATA = contrib/ndctl
 endif
 
-udevrulesdir = $(UDEVDIR)/rules.d
-dist_udevrules_DATA = contrib/80-ndctl.rules
-
 noinst_LIBRARIES = libccan.a
 libccan_a_SOURCES = \
 	ccan/str/str.h \
diff --git a/configure.ac b/configure.ac
index 9dc11da194d1..bb6b03324ea8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -153,17 +153,8 @@ fi
 AC_SUBST([systemd_unitdir])
 AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
 
-AC_ARG_WITH([tmpfilesdir],
-	[AS_HELP_STRING([--with-tmpfilesdir=DIR], [Directory for temporary runtime files])],
-	[tmpfilesdir=$withval],
-	[tmpfilesdir="/run"])
-
-UDEVDIR="$(pkg-config udev --variable=udevdir)"
-AC_SUBST([UDEVDIR])
-
 my_CFLAGS="\
 -D DEF_CONF_FILE='\"${sysconfdir}/ndctl/monitor.conf\"' \
--D DEF_TMPFS_DIR='\"${tmpfilesdir}/ndctl\"' \
 -Wall \
 -Wchar-subscripts \
 -Wformat-security \
@@ -204,7 +195,6 @@ AC_MSG_RESULT([
         sysconfdir:             ${sysconfdir}
         libdir:                 ${libdir}
         includedir:             ${includedir}
-        tmpfilesdir:            ${tmpfilesdir}
 
         compiler:               ${CC}
         cflags:                 ${CFLAGS}
diff --git a/contrib/80-ndctl.rules b/contrib/80-ndctl.rules
deleted file mode 100644
index 54788c40ea9d..000000000000
--- a/contrib/80-ndctl.rules
+++ /dev/null
@@ -1,3 +0,0 @@
-# do not edit this file, it will be overwritten on update
-
-ACTION=="add", KERNEL=="nmem*", RUN+="ndctl-udev $kernel"
diff --git a/ndctl.spec.in b/ndctl.spec.in
index 98394a924030..26396d4abad7 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -110,7 +110,6 @@ make check
 %postun -n DAX_LNAME -p /sbin/ldconfig
 
 %define bashcompdir %(pkg-config --variable=completionsdir bash-completion)
-%define udevdir  %(pkg-config --variable=udevdir udev)
 
 %files
 %defattr(-,root,root)
@@ -120,8 +119,6 @@ make check
 %{bashcompdir}/
 %{_sysconfdir}/ndctl/monitor.conf
 %{_unitdir}/ndctl-monitor.service
-%{_udevrulesdir}/80-ndctl.rules
-%{udevdir}/ndctl-udev
 
 %files -n daxctl
 %defattr(-,root,root)
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index d6839e4a9fb6..8a5e5f87e6c5 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -51,8 +51,3 @@ EXTRA_DIST += $(monitor_config_file)
 if ENABLE_SYSTEMD_UNITS
 systemd_unit_DATA = ndctl-monitor.service
 endif
-
-ndctl_udevdir = $(UDEVDIR)
-ndctl_udev_PROGRAMS = ndctl-udev
-ndctl_udev_SOURCES = ndctl-udev.c
-ndctl_udev_LDADD = lib/libndctl.la
diff --git a/ndctl/ndctl-udev.c b/ndctl/ndctl-udev.c
deleted file mode 100644
index 7b174b61d8cd..000000000000
--- a/ndctl/ndctl-udev.c
+++ /dev/null
@@ -1,150 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
-
-#include <ctype.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <stddef.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <ndctl/libndctl.h>
-
-/**
- * mkdir_p
- *
- * Copied from util-linux lib/fileutils.c
- */
-static int mkdir_p(const char *path, mode_t mode)
-{
-	char *p, *dir;
-	int rc = 0;
-
-	if (!path || !*path)
-		return -EINVAL;
-
-	dir = p = strdup(path);
-	if (!dir)
-		return -ENOMEM;
-
-	if (*p == '/')
-		p++;
-
-	while (p && *p) {
-		char *e = strchr(p, '/');
-		if (e)
-			*e = '\0';
-		if (*p) {
-			rc = mkdir(dir, mode);
-			if (rc && errno != EEXIST)
-				break;
-			rc = 0;
-		}
-		if (!e)
-			break;
-		*e = '/';
-		p = e + 1;
-	}
-
-	free(dir);
-	return rc;
-}
-
-static struct ndctl_dimm *find_dimm(struct ndctl_ctx *ctx, const char *devname)
-{
-	struct ndctl_bus *bus;
-	struct ndctl_dimm *dimm;
-
-	ndctl_bus_foreach(ctx, bus) {
-		ndctl_dimm_foreach(bus, dimm) {
-			if (strcmp(ndctl_dimm_get_devname(dimm), devname) == 0)
-				return dimm;
-		}
-	}
-	return NULL;
-}
-
-static void ack_shutdown(struct ndctl_dimm *dimm)
-{
-	struct ndctl_cmd *cmd;
-
-	cmd = ndctl_dimm_cmd_new_ack_shutdown_count(dimm);
-	if (!cmd)
-		return;
-	ndctl_cmd_submit(cmd);
-	ndctl_cmd_unref(cmd);
-}
-
-static void save_unsafe_shutdown_count(struct ndctl_dimm *dimm,
-				       const char *devname)
-{
-	char *path, *usc, count[16];
-	unsigned int shutdown;
-	struct ndctl_cmd *cmd;
-	int fd;
-
-	cmd = ndctl_dimm_cmd_new_smart(dimm);
-	if (!cmd)
-		return;
-
-	if (ndctl_cmd_submit(cmd))
-		goto unref_cmd;
-
-	shutdown = ndctl_cmd_smart_get_shutdown_count(cmd);
-	if (shutdown == UINT_MAX)
-		goto unref_cmd;
-
-	if (asprintf(&path, DEF_TMPFS_DIR "/%s", devname) < 0)
-		goto unref_cmd;
-
-	if (mkdir_p(path, 0755))
-		goto free_path;
-
-	if (asprintf(&usc, "%s/usc", path) < 0)
-		goto free_path;
-
-	fd = open(usc, O_WRONLY | O_CREAT, 0644);
-	if (fd < 0)
-		goto free_usc;
-
-	if (snprintf(count, sizeof(count), "%u\n", shutdown) < 0)
-		goto close_fd;
-
-	if (write(fd, count, strlen(count)) < 0)
-		goto close_fd;
-
- close_fd:
-	close(fd);
- free_usc:
-	free(usc);
- free_path:
-	free(path);
- unref_cmd:
-	ndctl_cmd_unref(cmd);
-}
-
-int main(int argc, char *argv[])
-{
-	struct ndctl_ctx *ctx;
-	struct ndctl_dimm *dimm = NULL;
-	const char *devname;
-
-	if (argc < 2)
-		return EINVAL;
-
-	devname = argv[1];
-	if (ndctl_new(&ctx))
-		return ENOMEM;
-
-	dimm = find_dimm(ctx, devname);
-	if (!dimm)
-		return ENODEV;
-
-	ack_shutdown(dimm);
-	save_unsafe_shutdown_count(dimm, devname);
-
-	ndctl_unref(ctx);
-	return 0;
-}

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count
  2018-09-28  0:41 [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count Dan Williams
                   ` (2 preceding siblings ...)
  2018-09-28  0:41 ` [ndctl PATCH v3 3/3] ndctl: Revert "ndctl: Create ndctl udev rules for dirty shutdown" Dan Williams
@ 2018-10-03  1:08 ` Verma, Vishal L
  3 siblings, 0 replies; 5+ messages in thread
From: Verma, Vishal L @ 2018-10-03  1:08 UTC (permalink / raw)
  To: Williams, Dan J, linux-nvdimm


On Thu, 2018-09-27 at 17:41 -0700, Dan Williams wrote:
> Changes since v2 [1]:
> * Drop the 'dirty-dimm' command
> * Add ndctl_dimm_get_dirty_shutdown() api
> 
> [1]: https://lists.01.org/pipermail/linux-nvdimm/2018-September/017890.html
> 
> ---
> 
> The latch mechanism is awkward especially when all that it needed is a
> rolling count of dirty-shutdown events. The expectation going forward is
> that the platform firmware will handle the latch, if it is present, and
> the OS need only consume the dirty-shutdown count. The ndctl
> implementation called libndctl apis from the udev queue which we
> discovered injects unnecessary udev queue drains / stalls into the boot
> path. Lastly, the userspace caching scheme for non-root users to consume
> the dirty-shutdown-count just isn't as efficient as teaching the kernel
> to cache this value and export it as a standard sysfs attribute.
> 
> ---
> 
> Dan Williams (3):
>       ndctl, lib: Add dirty-shutdown-count retrieval helper
>       ndctl: Revert "ndctl, intel: Fallback to smart cached shutdown_count"
>       ndctl: Revert "ndctl: Create ndctl udev rules for dirty shutdown"

These look good, applied.

> 
> 
>  .gitignore             |    1 
>  Makefile.am            |    3 -
>  configure.ac           |   10 ---
>  contrib/80-ndctl.rules |    3 -
>  ndctl.spec.in          |    3 -
>  ndctl/Makefile.am      |    5 --
>  ndctl/lib/intel.c      |   41 -------------
>  ndctl/lib/libndctl.c   |   16 ++++-
>  ndctl/lib/libndctl.sym |    5 ++
>  ndctl/lib/private.h    |    4 -
>  ndctl/libndctl.h       |    1 
>  ndctl/ndctl-udev.c     |  150 ------------------------------------------------
>  test/libndctl.c        |   29 +++++++--
>  13 files changed, 40 insertions(+), 231 deletions(-)
>  delete mode 100644 contrib/80-ndctl.rules
>  delete mode 100644 ndctl/ndctl-udev.c

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-10-03  1:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-28  0:41 [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count Dan Williams
2018-09-28  0:41 ` [ndctl PATCH v3 1/3] ndctl, lib: Add dirty-shutdown-count retrieval helper Dan Williams
2018-09-28  0:41 ` [ndctl PATCH v3 2/3] ndctl: Revert "ndctl, intel: Fallback to smart cached shutdown_count" Dan Williams
2018-09-28  0:41 ` [ndctl PATCH v3 3/3] ndctl: Revert "ndctl: Create ndctl udev rules for dirty shutdown" Dan Williams
2018-10-03  1:08 ` [ndctl PATCH v3 0/3] Replace udev rule for latch and dirty-shutdown-count Verma, Vishal L

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).