nvme: retry commands on EAGAIN and EINTR

The kernel started to return EAGAIN and EINTR. It is quiet likely that
this was possible before but it happened not so often. Thus for users
of nvme-cli it is a behavior change.

The first attempt was to handle this in libnvme itself. But it turns out
we can't just blindly retry. It is necessary to check if the user has
pressed Ctrl-C to terminate the program. Adding a signal handler inside
the library is a no go. Thus add the retry logic to nvme-cli itself.

Signed-off-by: Daniel Wagner <[email protected]>

Author: igaw

fabrics.c

@@ -47,6 +47,7 @@
 #include "fabrics.h"
 #include "util/cleanup.h"
 #include "util/logging.h"
+#include "util/sighdl.h"
 
 #define PATH_NVMF_DISC		SYSCONFDIR "/nvme/discovery.conf"
 #define PATH_NVMF_CONFIG	SYSCONFDIR "/nvme/config.json"
@@ -170,6 +171,22 @@ static int set_discovery_kato(struct nvme_fabrics_config *cfg)
 	return tmo;
 }
 
+
+static int nvme_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
+			 struct nvme_fabrics_config *cfg)
+{
+	int ret;
+
+retry:
+	ret = nvmf_add_ctrl(h, c, cfg);
+	if (ret == EAGAIN || (ret == EINTR && !nvme_sigint_received)) {
+		print_debug("nvmf_add_ctrl returned '%s'\n", nvme_strerror(ret));
+		goto retry;
+	}
+
+	return ret;
+}
+
 static nvme_ctrl_t __create_discover_ctrl(nvme_root_t r, nvme_host_t h,
 					  struct nvme_fabrics_config *cfg,
 					  struct tr_config *trcfg)
@@ -189,7 +206,7 @@ static nvme_ctrl_t __create_discover_ctrl(nvme_root_t r, nvme_host_t h,
 	tmo = set_discovery_kato(cfg);
 
 	errno = 0;
-	ret = nvmf_add_ctrl(h, c, cfg);
+	ret = nvme_add_ctrl(h, c, cfg);
 
 	cfg->keep_alive_tmo = tmo;
 	if (ret) {
@@ -1103,7 +1120,7 @@ int nvmf_connect(const char *desc, int argc, char **argv)
 	nvme_parse_tls_args(keyring, tls_key, tls_key_identity, &cfg, c);
 
 	errno = 0;
-	ret = nvmf_add_ctrl(h, c, &cfg);
+	ret = nvme_add_ctrl(h, c, &cfg);
 	if (ret)
 		fprintf(stderr, "could not add new controller: %s\n",
 			nvme_strerror(errno));

nvme.c

@@ -65,6 +65,7 @@
 #include "util/argconfig.h"
 #include "util/suffix.h"
 #include "util/logging.h"
+#include "util/sighdl.h"
 #include "fabrics.h"
 #define CREATE_CMD
 #include "nvme-builtin.h"
@@ -4437,21 +4438,16 @@ static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struc
 	return err;
 }
 
-static void intr_self_test(int signum)
-{
-	printf("\nInterrupted device self-test operation by %s\n", strsignal(signum));
-
-	errno = EINTR;
-}
-
 static int sleep_self_test(unsigned int seconds)
 {
-	errno = 0;
+	nvme_sigint_received = false;
 
 	sleep(seconds);
 
-	if (errno)
-		return -errno;
+	if (nvme_sigint_received) {
+		printf("\nInterrupted device self-test operation by SIGINT\n");
+		return -SIGINT;
+	}
 
 	return 0;
 }
@@ -4464,8 +4460,6 @@ static int wait_self_test(struct nvme_dev *dev)
 	int err, i = 0, p = 0, cnt = 0;
 	int wthr;
 
-	signal(SIGINT, intr_self_test);
-
 	ctrl = nvme_alloc(sizeof(*ctrl));
 	if (!ctrl)
 		return -ENOMEM;
@@ -10929,6 +10923,10 @@ int main(int argc, char **argv)
 	}
 	setlocale(LC_ALL, "");
 
+	err = nvme_install_sigint_handler();
+	if (err)
+		return err;
+
 	err = handle_plugin(argc - 1, &argv[1], nvme.extensions);
 	if (err == -ENOTTY)
 		general_help(&builtin);

util/logging.c

@@ -1,8 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #include <inttypes.h>
-
+#include <signal.h>
 #include <stdint.h>
+#include <string.h>
+#include <errno.h>
 #include <sys/ioctl.h>
 #include <sys/syslog.h>
 #include <sys/time.h>
@@ -11,6 +13,7 @@
 #include <libnvme.h>
 
 #include "logging.h"
+#include "sighdl.h"
 
 int log_level;
 static bool dry_run;
@@ -92,6 +95,14 @@ static void nvme_show_latency(struct timeval start, struct timeval end)
 				    (end.tv_usec - start.tv_usec)));
 }
 
+static void nvme_log_retry(int errnum)
+{
+	if (log_level < LOG_DEBUG)
+		return;
+
+	printf("passthru command returned '%s'\n", strerror(errnum));
+}
+
 int nvme_submit_passthru(int fd, unsigned long ioctl_cmd,
 			 struct nvme_passthru_cmd *cmd, __u32 *result)
 {
@@ -102,8 +113,14 @@ int nvme_submit_passthru(int fd, unsigned long ioctl_cmd,
 	if (log_level >= LOG_DEBUG)
 		gettimeofday(&start, NULL);
 
-	if (!dry_run)
+	if (!dry_run) {
+	retry:
 		err = ioctl(fd, ioctl_cmd, cmd);
+		if (err == EAGAIN || (err == EINTR && !nvme_sigint_received)) {
+			nvme_log_retry(err);
+			goto retry;
+		}
+	}
 
 	if (log_level >= LOG_DEBUG) {
 		gettimeofday(&end, NULL);
@@ -128,8 +145,14 @@ int nvme_submit_passthru64(int fd, unsigned long ioctl_cmd,
 	if (log_level >= LOG_DEBUG)
 		gettimeofday(&start, NULL);
 
-	if (!dry_run)
+	if (!dry_run) {
+	retry:
 		err = ioctl(fd, ioctl_cmd, cmd);
+		if (err == EAGAIN || (err == EINTR && !nvme_sigint_received)) {
+			nvme_log_retry(err);
+			goto retry;
+		}
+	}
 
 	if (log_level >= LOG_DEBUG) {
 		gettimeofday(&end, NULL);

util/meson.build

@@ -6,6 +6,7 @@ sources += [
   'util/crc32.c',
   'util/logging.c',
   'util/mem.c',
+  'util/sighdl.c',
   'util/suffix.c',
   'util/types.c',
   'util/utils.c'

util/sighdl.c

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <signal.h>
+#include <errno.h>
+
+#include "sighdl.h"
+
+bool nvme_sigint_received;
+
+void nvme_sigint_handler(int signum)
+{
+	nvme_sigint_received = true;
+}
+
+int nvme_install_sigint_handler(void)
+{
+	struct sigaction act;
+
+	sigemptyset(&act.sa_mask);
+	act.sa_handler = nvme_sigint_handler;
+	act.sa_flags = 0;
+
+	nvme_sigint_received = false;
+	if (sigaction(SIGINT, &act, NULL) == -1)
+		return errno;
+
+	return 0;
+}

util/sighdl.h

@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __NVME_SIGHDL
+#define __NVME_SIGHDL
+
+#include <stdbool.h>
+
+extern bool nvme_sigint_received;
+
+int nvme_install_sigint_handler(void);
+
+#endif // __NVME_SIGHDL