Skip to content

Commit a7a2c54

Browse files
committed
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]>
1 parent c70f633 commit a7a2c54

3 files changed

Lines changed: 55 additions & 17 deletions

File tree

fabrics.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include "fabrics.h"
4848
#include "util/cleanup.h"
4949
#include "util/logging.h"
50+
#include "util/sighdl.h"
5051

5152
#define PATH_NVMF_DISC SYSCONFDIR "/nvme/discovery.conf"
5253
#define PATH_NVMF_CONFIG SYSCONFDIR "/nvme/config.json"
@@ -170,6 +171,22 @@ static int set_discovery_kato(struct nvme_fabrics_config *cfg)
170171
return tmo;
171172
}
172173

174+
175+
static int nvme_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
176+
struct nvme_fabrics_config *cfg)
177+
{
178+
int ret;
179+
180+
retry:
181+
ret = nvmf_add_ctrl(h, c, cfg);
182+
if (ret == EAGAIN || (ret == EINTR && !nvme_sigint_received)) {
183+
printf("nvmf_add_ctrl returned '%s'\n", strerror(ret));
184+
goto retry;
185+
}
186+
187+
return ret;
188+
}
189+
173190
static nvme_ctrl_t __create_discover_ctrl(nvme_root_t r, nvme_host_t h,
174191
struct nvme_fabrics_config *cfg,
175192
struct tr_config *trcfg)
@@ -189,7 +206,7 @@ static nvme_ctrl_t __create_discover_ctrl(nvme_root_t r, nvme_host_t h,
189206
tmo = set_discovery_kato(cfg);
190207

191208
errno = 0;
192-
ret = nvmf_add_ctrl(h, c, cfg);
209+
ret = nvme_add_ctrl(h, c, cfg);
193210

194211
cfg->keep_alive_tmo = tmo;
195212
if (ret) {
@@ -1103,7 +1120,7 @@ int nvmf_connect(const char *desc, int argc, char **argv)
11031120
nvme_parse_tls_args(keyring, tls_key, tls_key_identity, &cfg, c);
11041121

11051122
errno = 0;
1106-
ret = nvmf_add_ctrl(h, c, &cfg);
1123+
ret = nvme_add_ctrl(h, c, &cfg);
11071124
if (ret)
11081125
fprintf(stderr, "could not add new controller: %s\n",
11091126
nvme_strerror(errno));

nvme.c

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
#include "util/argconfig.h"
6666
#include "util/suffix.h"
6767
#include "util/logging.h"
68+
#include "util/sighdl.h"
6869
#include "fabrics.h"
6970
#define CREATE_CMD
7071
#include "nvme-builtin.h"
@@ -4437,21 +4438,16 @@ static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struc
44374438
return err;
44384439
}
44394440

4440-
static void intr_self_test(int signum)
4441-
{
4442-
printf("\nInterrupted device self-test operation by %s\n", strsignal(signum));
4443-
4444-
errno = EINTR;
4445-
}
4446-
44474441
static int sleep_self_test(unsigned int seconds)
44484442
{
4449-
errno = 0;
4443+
nvme_sigint_received = false;
44504444

44514445
sleep(seconds);
44524446

4453-
if (errno)
4454-
return -errno;
4447+
if (nvme_sigint_received) {
4448+
printf("\nInterrupted device self-test operation by SIGINT\n");
4449+
return -SIGINT;
4450+
}
44554451

44564452
return 0;
44574453
}
@@ -4464,8 +4460,6 @@ static int wait_self_test(struct nvme_dev *dev)
44644460
int err, i = 0, p = 0, cnt = 0;
44654461
int wthr;
44664462

4467-
signal(SIGINT, intr_self_test);
4468-
44694463
ctrl = nvme_alloc(sizeof(*ctrl));
44704464
if (!ctrl)
44714465
return -ENOMEM;
@@ -10935,6 +10929,10 @@ int main(int argc, char **argv)
1093510929
}
1093610930
setlocale(LC_ALL, "");
1093710931

10932+
err = nvme_install_sigint_handler();
10933+
if (err)
10934+
return err;
10935+
1093810936
err = handle_plugin(argc - 1, &argv[1], nvme.extensions);
1093910937
if (err == -ENOTTY)
1094010938
general_help(&builtin, NULL);

util/logging.c

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
// SPDX-License-Identifier: GPL-2.0-or-later
22

33
#include <inttypes.h>
4-
4+
#include <signal.h>
55
#include <stdint.h>
6+
#include <string.h>
7+
#include <errno.h>
68
#include <sys/ioctl.h>
79
#include <sys/syslog.h>
810
#include <sys/time.h>
@@ -11,6 +13,7 @@
1113
#include <libnvme.h>
1214

1315
#include "logging.h"
16+
#include "sighdl.h"
1417

1518
int log_level;
1619
static bool dry_run;
@@ -92,6 +95,14 @@ static void nvme_show_latency(struct timeval start, struct timeval end)
9295
(end.tv_usec - start.tv_usec)));
9396
}
9497

98+
static void nvme_log_retry(int errnum)
99+
{
100+
if (log_level < LOG_DEBUG)
101+
return;
102+
103+
printf("passthru command returned '%s'\n", strerror(errnum));
104+
}
105+
95106
int nvme_submit_passthru(int fd, unsigned long ioctl_cmd,
96107
struct nvme_passthru_cmd *cmd, __u32 *result)
97108
{
@@ -102,8 +113,14 @@ int nvme_submit_passthru(int fd, unsigned long ioctl_cmd,
102113
if (log_level >= LOG_DEBUG)
103114
gettimeofday(&start, NULL);
104115

105-
if (!dry_run)
116+
if (!dry_run) {
117+
retry:
106118
err = ioctl(fd, ioctl_cmd, cmd);
119+
if (err == EAGAIN || (err == EINTR && !nvme_sigint_received)) {
120+
nvme_log_retry(err);
121+
goto retry;
122+
}
123+
}
107124

108125
if (log_level >= LOG_DEBUG) {
109126
gettimeofday(&end, NULL);
@@ -128,8 +145,14 @@ int nvme_submit_passthru64(int fd, unsigned long ioctl_cmd,
128145
if (log_level >= LOG_DEBUG)
129146
gettimeofday(&start, NULL);
130147

131-
if (!dry_run)
148+
if (!dry_run) {
149+
retry:
132150
err = ioctl(fd, ioctl_cmd, cmd);
151+
if (err == EAGAIN || (err == EINTR && !nvme_sigint_received)) {
152+
nvme_log_retry(err);
153+
goto retry;
154+
}
155+
}
133156

134157
if (log_level >= LOG_DEBUG) {
135158
gettimeofday(&end, NULL);

0 commit comments

Comments
 (0)