Skip to content

nvme: add support for abort command#2825

Closed
ikegami-t wants to merge 2 commits intolinux-nvme:masterfrom
ikegami-t:nvme-abort
Closed

nvme: add support for abort command#2825
ikegami-t wants to merge 2 commits intolinux-nvme:masterfrom
ikegami-t:nvme-abort

Conversation

@ikegami-t
Copy link
Copy Markdown
Contributor

The abort command can be submitted by the admin-passthru command. But better to support by the nvme abort command directly then add.

ikegami-t added 2 commits May 25, 2025 02:00
The abort command can be submitted by the admin-passthru command.
But better to support by the nvme abort command directly then add.

Signed-off-by: Tokunori Ikegami <[email protected]>
Since actually the message output by the error case.

Signed-off-by: Tokunori Ikegami <[email protected]>
@ikegami-t
Copy link
Copy Markdown
Contributor Author

The changes depended on the PR: linux-nvme/libnvme#1017.

@keithbusch
Copy link
Copy Markdown
Contributor

No no no! Userspace doesn't know anything about the command ids or queue contexts that are used, there is no way this interface could be safely or consistently used!

@ikegami-t
Copy link
Copy Markdown
Contributor Author

Just sent the commit below to get the SQID and CID informtion from the driver before the PR but is this still not safe? By the way just thought to change the driver to abort a command with the opcode or etc., used by the userspace also.
https://lore.kernel.org/linux-nvme/[email protected]/T/#u

@keithbusch
Copy link
Copy Markdown
Contributor

Just sent the commit below to get the SQID and CID informtion from the driver before the PR but is this still not safe?

Absolutely not. Outside broken controllers, that sysfs interface proposal just displays a racy snapshot, and I don't think it is appropriate for sysfs anyway as it looks like a debug interface rather than a device attribute. Nvme devices do millions of IOPs, there's no way the output from reading a sysfs file is accurate before the text even hits the output steam.

@keithbusch
Copy link
Copy Markdown
Contributor

The only reason the kernel is able to do this at all is because it holds a reference on the targeted qid:cid pair that locks the context of the outstanding IO. It can't possibly be a different context in this path, we know exactly what we're aborting. You can't do that from user space because it doesn't own the hardware abstractions. There is zero reason to make this an easily reachable command, just like we don't support create/delete cq/sq.

@ikegami-t
Copy link
Copy Markdown
Contributor Author

I could understood well so let me close the PRs. Thank you for your explanation.

Outside broken controllers, that sysfs interface proposal just displays a racy snapshot, and I don't think it is appropriate for sysfs anyway as it looks like a debug interface rather than a device attribute.

Yes the sysfs interface is for the debugging purpose so I will do consider more later if any other way to confirm the command not completed.

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.

2 participants