nvme: Remove SCSI translations
PATCH v2 1/1 nvme: Remove SCSI translations
The SCSI-to-NVMe translations were added to assist storage applications utilizing SG_IO transitioning to NVMe. It was always recommended, however, to use native NVMe for device management as too much is lost in translation and the maintenance burden in keeping this kludgey layer around has been neglected such that much of the translations are completely broken.
This patch removes SG_IO handling from NVMe to avoid any confusion regarding maintenance support for this interface. The config option for NVMe SCSI emulation has been disabled by default since 4.5. The driver has supported native nvme user commands since the beginning, and native tooling is publicly available for use or as reference for anyone writing their own tools, so there's no excuse for hanging onto a broken crutch.
---
v1 -> v2:
Updated change log, and added v1's acks and reviews.
Removed all functions that only scsi used, and make the now local functions static.
drivers/nvme/host/Kconfig | 12 - drivers/nvme/host/core.c | 58 +- drivers/nvme/host/nvme.h | 14 - drivers/nvme/host/scsi.c | 2460 --------------------------------------------- 4 files changed, 3 insertions(+), 2541 deletions(-) delete mode 100644 drivers/nvme/host/scsi.c
drivers/nvme/host/Kconfig +0 -12
@@ -13,18 +13,6 @@ config BLK_DEV_NVME To compile this driver as a module, choose M here: the module will be called nvme. -config BLK_DEV_NVME_SCSI - bool "SCSI emulation for NVMe device nodes" - depends on NVME_CORE - ---help--- - This adds support for the SG_IO ioctl on the NVMe character - and block devices nodes, as well as a translation for a small - number of selected SCSI commands to NVMe commands to the NVMe - driver. If you don't know what this means you probably want - to say N here, unless you run a distro that abuses the SCSI - emulation to provide stable device names for mount by id, like - some OpenSuSE and SLES versions. - config NVME_FABRICS tristate
drivers/nvme/host/core.c +3 -55
100 lines not on this page · raw message
drivers/nvme/host/nvme.h +0 -14
@@ -310,24 +310,10 @@ int __nvme_submit_user_cmd(struct request_queue *q, struct nvme_command *cmd, void __user *ubuffer, unsigned bufflen, void __user *meta_buffer, unsigned meta_len, u32 meta_seed, u32 *result, unsigned timeout); -int nvme_identify_ctrl (struct nvme_ctrl *dev,struct nvme_id_ctrl **id); -int nvme_identify_ns (struct nvme_ctrl *dev,unsigned nsid, -struct nvme_id_ns **id); -int nvme_get_log_page (struct nvme_ctrl *dev,struct nvme_smart_log **log); -int nvme_get_features (struct nvme_ctrl *dev,unsigned fid,unsigned nsid, -void *buffer,size_t buflen,u32 *result); -int nvme_set_features (struct nvme_ctrl *dev,unsigned fid,unsigned dword11, -void *buffer,size_t buflen,u32 *result);int nvme_set_queue_count (struct nvme_ctrl *ctrl,int *count);void nvme_start_keep_alive (struct nvme_ctrl *ctrl);void nvme_stop_keep_alive (struct nvme_ctrl *ctrl); -struct sg_io_hdr ; - -int nvme_sg_io (struct nvme_ns *ns,struct sg_io_hdr __user *u_hdr); -int nvme_sg_io32 (struct nvme_ns *ns,unsigned long arg); -int nvme_sg_get_version_num (int __user *ip); -#ifdef CONFIG_NVM int nvme_nvm_ns_supported (struct nvme_ns *ns,struct nvme_id_ns *id);int nvme_nvm_register (struct nvme_ns *ns,char *disk_name,int node);
drivers/nvme/host/scsi.c deleted +0 -2460
2,460 lines not on this page · raw message
Signature · 1 line
2.5.5
PATCH v2 1/1 nvme: Remove SCSI translations
Quoted 14 lines · Keith Busch
The SCSI-to-NVMe translations were added to assist storage applications utilizing
SG_IOtransitioning to NVMe. It was always recommended, however, to use native NVMe for device management as too much is lost in translation and the maintenance burden in keeping this kludgey layer around has been neglected such that much of the translations are completely broken.This patch removes
SG_IOhandling from NVMe to avoid any confusion regarding maintenance support for this interface. The config option for NVMe SCSI emulation has been disabled by default since 4.5. The driver has supported native nvme user commands since the beginning, and native tooling is publicly available for use or as reference for anyone writing their own tools, so there's no excuse for hanging onto a broken crutch.
Quoted 6 lines
Signed-off-by Keith Busch <keith.busch at intel.com>Acked-by Jens Axboe <axboe at kernel.dk>Reviewed-by Martin K. Petersen <martin.petersen at oracle.com>Reviewed-by Sagi Grimberg <sagi at grimberg.me>Reviewed-by Max Gurtovoy <maxg at mellanox.com>---
Signature · 6 lines
Johannes Thumshirn Storage jthumshirn at suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: Felix Imend?rffer, Jane Smithard, Graham Norton HRB 21284 (AG N?rnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
PATCH v2 1/1 nvme: Remove SCSI translations
In reply to Keith BuschQuoted 14 lines · Keith Busch
The SCSI-to-NVMe translations were added to assist storage applications utilizing
SG_IOtransitioning to NVMe. It was always recommended, however, to use native NVMe for device management as too much is lost in translation and the maintenance burden in keeping this kludgey layer around has been neglected such that much of the translations are completely broken.This patch removes
SG_IOhandling from NVMe to avoid any confusion regarding maintenance support for this interface. The config option for NVMe SCSI emulation has been disabled by default since 4.5. The driver has supported native nvme user commands since the beginning, and native tooling is publicly available for use or as reference for anyone writing their own tools, so there's no excuse for hanging onto a broken crutch.
Quoted 6 lines
Signed-off-by Keith Busch <keith.busch at intel.com>Acked-by Jens Axboe <axboe at kernel.dk>Reviewed-by Martin K. Petersen <martin.petersen at oracle.com>Reviewed-by Sagi Grimberg <sagi at grimberg.me>Reviewed-by Max Gurtovoy <maxg at mellanox.com>---
.
PATCH v2 1/1 nvme: Remove SCSI translations
In reply to Keith BuschNice patch!
And the sg ioctl used for multipath-tools should be considered. Regards!
Reviewed-by: Yang Feng <philip.yang at huawei.com> -Yang
PATCH v2 1/1 nvme: Remove SCSI translations
Quoted 2 lines · Yang Feng
And the sg ioctl used for multipath-tools should be considered. Regards!
Yep, these are already updated for native nvme identification.
PATCH v2 1/1 nvme: Remove SCSI translations
In reply to Keith BuschQuoted 9 lines
Subject: [PATCHv2] nvme: Remove SCSI translations
The SCSI-to-NVMe translations were added to assist storage applications utilizing
SG_IOtransitioning to NVMe. It was always recommended, however, to use native NVMe for device management as too much is lost in translation and the maintenance burden in keeping this kludgey layer around has been neglected such that much of the translations are completely broken.
Quoted 6 lines
This patch removes
SG_IOhandling from NVMe to avoid any confusion regarding maintenance support for this interface. The config option for NVMe SCSI emulation has been disabled by default since 4.5. The driver has supported native nvme user commands since the beginning, and native tooling is publicly available for use or as reference for anyone writing their own tools, so there's no excuse for hanging onto a broken crutch.
As Linda and Micah mentioned, this violates a cardinal linux kernel rule - it breaks user space.
Please:
1. Incorporate the TEST UNIT READY bug fix now.
2. Add a WARN_ONCE printing a message that the interface that was just used is deprecated and will go away, and modify the CONFIG description to include that warning too.
3. Wait a few kernel cycles before really removing it.
---
Robert Elliott, HPE Persistent Memory
PATCH v2 1/1 nvme: Remove SCSI translations
Quoted 10 lines · Robert (Persistent Memory)
As Linda and Micah mentioned, this violates a cardinal linux kernel rule - it breaks user space.
Please:
1. Incorporate the TEST UNIT READY bug fix now.
2. Add aWARN_ONCEprinting a message that the interface that was just used is deprecated and will go away, and modify the CONFIG description to include that warning too.
3. Wait a few kernel cycles before really removing it.
I really want to emphasize a few points here:
The linux nvme driver support for SG_IO was never really working in the first place. We can't break what's broken.
We're not making any nvme features unreachable by deleting broken interfaces. The driver has supported a real NVMe management interface since day 1, and open source tooling for it is freely available. These tools are also part of many distribution's package repositories.
The NVMe translation for TUR has been broken for years and you didn't notice. Your WARN proposal won't do you any good in that case, but it certainly puts a burden on the developers trying to progress and enhance NVMe.
The distributions supporting SCSI translations for NVMe are already applying out of tree patches to enable them.
PATCH v2 1/1 nvme: Remove SCSI translations
Quoted 12 lines · Keith Busch
Quoted 10 lines · Robert (Persistent Memory)
As Linda and Micah mentioned, this violates a cardinal linux kernel rule - it breaks user space.
Please:
1. Incorporate the TEST UNIT READY bug fix now.
2. Add aWARN_ONCEprinting a message that the interface that was just used is deprecated and will go away, and modify the CONFIG description to include that warning too.
3. Wait a few kernel cycles before really removing it.
Quoted 4 lines
I really want to emphasize a few points here:
The linux nvme driver support for
SG_IOwas never really working in the first place. We can't break what's broken.
It was working well enough that people are using it.
Quoted 7 lines
We're not making any nvme features unreachable by deleting broken interfaces. The driver has supported a real NVMe management interface since day 1, and open source tooling for it is freely available. These tools are also part of many distribution's package repositories.
The NVMe translation for TUR has been broken for years and you didn't notice.
As I said, it was working well enough until recently. We noticed the problem when the recent code was pulled into an enterprise distro where we do most of our production/customer testing. David Milburn posted a simple patch to fix at least the part that we're using. I can't speak for other people who may be using other parts.
Quoted 3 lines
Your WARN proposal won't do you any good in that case, but it certainly puts a burden on the developers trying to progress and enhance NVMe.
I don't see how this prevents enhancing NVMe. It seems like a reasonable transition.
Quoted 2 lines
The distributions supporting SCSI translations for NVMe are already applying out of tree patches to enable them.
All of them? Are you sure?
-- ljk
Quoted 6 lines
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme