From 2a521a95339aa0e0464add0d32f5a34f0b9c26f2 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Thu, 8 May 2025 10:05:35 +0200 Subject: [PATCH 1/2] util: add SIGINT handler Add a helper for catching Ctrl-C from the user. Signed-off-by: Daniel Wagner --- util/meson.build | 1 + util/sighdl.c | 27 +++++++++++++++++++++++++++ util/sighdl.h | 11 +++++++++++ 3 files changed, 39 insertions(+) create mode 100644 util/sighdl.c create mode 100644 util/sighdl.h diff --git a/util/meson.build b/util/meson.build index f5474cdb38..b76677893a 100644 --- a/util/meson.build +++ b/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' diff --git a/util/sighdl.c b/util/sighdl.c new file mode 100644 index 0000000000..bf2bbc350b --- /dev/null +++ b/util/sighdl.c @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include +#include + +#include "sighdl.h" + +bool nvme_sigint_received; + +static 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; +} diff --git a/util/sighdl.h b/util/sighdl.h new file mode 100644 index 0000000000..8d5d1c1263 --- /dev/null +++ b/util/sighdl.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#ifndef __NVME_SIGHDL +#define __NVME_SIGHDL + +#include + +extern bool nvme_sigint_received; + +int nvme_install_sigint_handler(void); + +#endif // __NVME_SIGHDL From bb582dd260b1618735485e0ea9df9cb4dfa2eac5 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 5 May 2025 11:25:51 +0200 Subject: [PATCH 2/2] 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 --- fabrics.c | 21 +++++++++++++++++++-- nvme.c | 22 ++++++++++------------ util/logging.c | 29 ++++++++++++++++++++++++++--- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/fabrics.c b/fabrics.c index ab17e179c4..1e164228a9 100644 --- a/fabrics.c +++ b/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)) { + printf("nvmf_add_ctrl returned '%s'\n", 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)); diff --git a/nvme.c b/nvme.c index 5aaa838310..c046f1f814 100644 --- a/nvme.c +++ b/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); diff --git a/util/logging.c b/util/logging.c index b2ddcfc3ea..e05483916a 100644 --- a/util/logging.c +++ b/util/logging.c @@ -1,8 +1,10 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include - +#include #include +#include +#include #include #include #include @@ -11,6 +13,7 @@ #include #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);