Skip to content

nvme: apply user-specified timeout to all admin passthru commands#3302

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
darkfiberiru:fix/apply-timeout-to-all-admin-cmds
Apr 22, 2026
Merged

nvme: apply user-specified timeout to all admin passthru commands#3302
igaw merged 1 commit intolinux-nvme:masterfrom
darkfiberiru:fix/apply-timeout-to-all-admin-cmds

Conversation

@darkfiberiru
Copy link
Copy Markdown
Contributor

The --timeout / -t flag is parsed into nvme_args.timeout but was only applied to format-nvm and admin-passthru. All other commands that use nvme_init_*() helpers followed by libnvme_submit_admin_passthru() had their timeout_ms left at 0 (memset by the init functions), silently falling back to the kernel's admin_timeout (60s).

This meant commands like delete-ns, create-ns, attach-ns, detach-ns, sanitize, fw-download, fw-commit, device-self-test, security-send, security-receive, capacity-mgmt, and all identify passthru commands ignored the user-supplied timeout.

Set cmd.timeout_ms = nvme_args.timeout after every nvme_init_*() call and before submission. When -t is not given, nvme_args.timeout is NVME_DEFAULT_IOCTL_TIMEOUT (0), preserving the existing kernel-default behavior.

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Apr 21, 2026

I suggest something simpler, set timeout on the handle and use this value when submitting in ioctl.c:

--- a/nvme.c
+++ b/nvme.c
@@ -381,6 +381,8 @@ int parse_and_open(struct libnvme_global_ctx **ctx,
        libnvme_transport_handle_set_submit_exit(hdl_new , nvme_submit_exit);
        libnvme_transport_handle_set_decide_retry(hdl_new, nvme_decide_retry);
        libnvme_set_dry_run(ctx_new, argconfig_parse_seen(opts, "dry-run"));
+       if (argconfig_parse_seen(opts, "timeout"))
+               libnvme_transport_handle_set_timeout(hdl, nvme_args.timeout);

        *ctx = ctx_new;
        *hdl = hdl_new;

In v1 we passed the timeout value around using always the default value with a few exception. For special cases we can still do the approach you suggest, but I would like to avoid this if possible.

@darkfiberiru darkfiberiru force-pushed the fix/apply-timeout-to-all-admin-cmds branch from 4fc98fe to 22c96c0 Compare April 21, 2026 17:41
@darkfiberiru
Copy link
Copy Markdown
Contributor Author

@igaw yes I see that's much cleaner. Sorry for brute force I will refactor

@darkfiberiru darkfiberiru force-pushed the fix/apply-timeout-to-all-admin-cmds branch from 22c96c0 to 820fa2f Compare April 21, 2026 19:37
@darkfiberiru
Copy link
Copy Markdown
Contributor Author

@igaw Updated

The --timeout / -t flag is parsed into nvme_args.timeout but was only
honored by format-nvm and admin-passthru. All other commands left
cmd.timeout_ms at 0 (set by nvme_init_*() helpers), silently falling
back to the kernel's admin_timeout (60s).

Signed-off-by: Nick Wolff <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@igaw igaw force-pushed the fix/apply-timeout-to-all-admin-cmds branch from 820fa2f to a438889 Compare April 22, 2026 06:52
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Apr 22, 2026

I've updated some formatting (nitpicking), moved linker symbol into the correct section and trimmed the commit message. Thanks!

@igaw igaw merged commit 3fe7c82 into linux-nvme:master Apr 22, 2026
28 of 29 checks passed
@ixhamza
Copy link
Copy Markdown

ixhamza commented Apr 22, 2026

Unrelated to this commit, but in format_cmd() the nvme_args.timeout check runs before_parse_args(), so I believe it never fires.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants