From 13ce7e625a3383004181217985a70d16c3cbe8be Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 13 May 2021 12:59:52 +0100 Subject: nvme: remove redundant initialization of variable ret The variable ret is being initialized with a value that is never read, it is being updated later on. The assignment is redundant and can be removed. Signed-off-by: Colin Ian King Signed-off-by: Christoph Hellwig --- drivers/nvme/host/rdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c index 37943dc4c2c1..74bf2c7f2b80 100644 --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -1088,7 +1088,7 @@ static void nvme_rdma_reconnect_or_remove(struct nvme_rdma_ctrl *ctrl) static int nvme_rdma_setup_ctrl(struct nvme_rdma_ctrl *ctrl, bool new) { - int ret = -EINVAL; + int ret; bool changed; ret = nvme_rdma_configure_admin_queue(ctrl, new); -- cgit From ebd8a93aa4f50e9e013e6aa7fe601b4ce7565c28 Mon Sep 17 00:00:00 2001 From: Alexey Bogoslavsky Date: Wed, 28 Apr 2021 09:27:36 +0000 Subject: nvme: extend and modify the APST configuration algorithm The algorithm that was used until now for building the APST configuration table has been found to produce entries with excessively long ITPT (idle time prior to transition) for devices declaring relatively long entry and exit latencies for non-operational power states. This leads to unnecessary waste of power and, as a result, failure to pass mandatory power consumption tests on Chromebook platforms. The new algorithm is based on two predefined ITPT values and two predefined latency tolerances. Based on these values, as well as on exit and entry latencies reported by the device, the algorithm looks for up to 2 suitable non-operational power states to use as primary and secondary APST transition targets. The predefined values are supplied to the nvme driver as module parameters: - apst_primary_timeout_ms (default: 100) - apst_secondary_timeout_ms (default: 2000) - apst_primary_latency_tol_us (default: 15000) - apst_secondary_latency_tol_us (default: 100000) The algorithm echoes the approach used by Intel's and Microsoft's drivers on Windows. The specific default parameter values are also based on those drivers. Yet, this patch doesn't introduce the ability to dynamically regenerate the APST table in the event of switching the power source from AC to battery and back. Adding this functionality may be considered in the future. In the meantime, the timeouts and tolerances reflect a compromise between values used by Microsoft for AC and battery scenarios. In most NVMe devices the new algorithm causes them to implement a more aggressive power saving policy. While beneficial in most cases, this sometimes comes at the price of a higher IO processing latency in certain scenarios as well as at the price of a potential impact on the drive's endurance (due to more frequent context saving when entering deep non- operational states). So in order to provide a fallback for systems where these regressions cannot be tolerated, the patch allows to revert to the legacy behavior by setting either apst_primary_timeout_ms or apst_primary_latency_tol_us parameter to 0. Eventually (and possibly after fine tuning the default values of the module parameters) the legacy behavior can be removed. TESTING. The new algorithm has been extensively tested. Initially, simulations were used to compare APST tables generated by old and new algorithms for a wide range of devices. After that, power consumption, performance and latencies were measured under different workloads on devices from multiple vendors (WD, Intel, Samsung, Hynix, Kioxia). Below is the description of the tests and the findings. General observations. The effect the patch has on the APST table varies depending on the entry and exit latencies advertised by the devices. For some devices, the effect is negligible (e.g. Kioxia KBG40ZNS), for some significant, making the transitions to PS3 and PS4 much quicker (e.g. WD SN530, Intel 760P), or making the sleep deeper, PS4 rather than PS3 after a similar amount of time (e.g. SK Hynix BC511). For some devices (e.g. Samsung PM991) the effect is mixed: the initial transition happens after a longer idle time, but takes the device to a lower power state. Workflows. In order to evaluate the patch's effect on the power consumption and latency, 7 workflows were used for each device. The workflows were designed to test the scenarios where significant differences between the old and new behaviors are most likely. Each workflow was tested twice: with the new and with the old APST table generation implementation. Power consumption, performance and latency were measured in the process. The following workflows were used: 1) Consecutive write at the maximum rate with IO depth of 2, with no pauses 2) Repeated pattern of 1000 consecutive writes of 4K packets followed by 50ms idle time 3) Repeated pattern of 1000 consecutive writes of 4K packets followed by 150ms idle time 4) Repeated pattern of 1000 consecutive writes of 4K packets followed by 500ms idle time 5) Repeated pattern of 1000 consecutive writes of 4K packets followed by 1.5s idle time 6) Repeated pattern of 1000 consecutive writes of 4K packets followed by 5s idle time 7) Repeated pattern of a single random read of a 4K packet followed by 150ms idle time Power consumption Actual power consumption measurements produced predictable results in accordance with the APST mechanism's theory of operation. Devices with long entry and exit latencies such as WD SN530 showed huge improvement on scenarios 4,5 and 6 of up to 62%. Devices such as Kioxia KBG40ZNS where the resulting APST table looks virtually identical with both legacy and new algorithms, showed little or no change in the average power consumption on all workflows. Devices with extra short latencies such as Samsung PM991 showed moderate increase in power consumption of up to 18% in worst case scenarios. In addition, on Intel and Samsung devices a more complex impact was observed on scenarios 3, 4 and 7. Our understanding is that due to longer stay in deep non-operational states between the writes the devices start performing background operations leading to an increase of power consumption. With the old APST tables part of these operations are delayed until the scenario is over and a longer idle period begins, but eventually this extra power is consumed anyway. Performance. In terms of performance measured on sustained write or read scenarios, the effect of the patch is minimal as in this case the device doesn't enter low power states. Latency As expected, in devices where the patch causes a more aggressive power saving policy (e.g. WD SN530, Intel 760P), an increase in latency was observed in certain scenarios. Workflow number 7, specifically designed to simulate the worst case scenario as far as latency is concerned, indeed shows a sharp increase in average latency (~2ms -> ~53ms on Intel 760P and 0.6 -> 10ms on WD SN530). The latency increase on other workloads and other devices is much milder or non-existent. Signed-off-by: Alexey Bogoslavsky Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 89 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 78 insertions(+), 11 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 762125f2905f..e7441ccaa8db 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -57,6 +57,26 @@ static bool force_apst; module_param(force_apst, bool, 0644); MODULE_PARM_DESC(force_apst, "allow APST for newly enumerated devices even if quirked off"); +static unsigned long apst_primary_timeout_ms = 100; +module_param(apst_primary_timeout_ms, ulong, 0644); +MODULE_PARM_DESC(apst_primary_timeout_ms, + "primary APST timeout in ms"); + +static unsigned long apst_secondary_timeout_ms = 2000; +module_param(apst_secondary_timeout_ms, ulong, 0644); +MODULE_PARM_DESC(apst_secondary_timeout_ms, + "secondary APST timeout in ms"); + +static unsigned long apst_primary_latency_tol_us = 15000; +module_param(apst_primary_latency_tol_us, ulong, 0644); +MODULE_PARM_DESC(apst_primary_latency_tol_us, + "primary APST latency tolerance in us"); + +static unsigned long apst_secondary_latency_tol_us = 100000; +module_param(apst_secondary_latency_tol_us, ulong, 0644); +MODULE_PARM_DESC(apst_secondary_latency_tol_us, + "secondary APST latency tolerance in us"); + static bool streams; module_param(streams, bool, 0644); MODULE_PARM_DESC(streams, "turn on support for Streams write directives"); @@ -2217,14 +2237,54 @@ static int nvme_configure_acre(struct nvme_ctrl *ctrl) return ret; } +/* + * The function checks whether the given total (exlat + enlat) latency of + * a power state allows the latter to be used as an APST transition target. + * It does so by comparing the latency to the primary and secondary latency + * tolerances defined by module params. If there's a match, the corresponding + * timeout value is returned and the matching tolerance index (1 or 2) is + * reported. + */ +static bool nvme_apst_get_transition_time(u64 total_latency, + u64 *transition_time, unsigned *last_index) +{ + if (total_latency <= apst_primary_latency_tol_us) { + if (*last_index == 1) + return false; + *last_index = 1; + *transition_time = apst_primary_timeout_ms; + return true; + } + if (apst_secondary_timeout_ms && + total_latency <= apst_secondary_latency_tol_us) { + if (*last_index <= 2) + return false; + *last_index = 2; + *transition_time = apst_secondary_timeout_ms; + return true; + } + return false; +} + /* * APST (Autonomous Power State Transition) lets us program a table of power * state transitions that the controller will perform automatically. - * We configure it with a simple heuristic: we are willing to spend at most 2% - * of the time transitioning between power states. Therefore, when running in - * any given state, we will enter the next lower-power non-operational state - * after waiting 50 * (enlat + exlat) microseconds, as long as that state's exit - * latency is under the requested maximum latency. + * + * Depending on module params, one of the two supported techniques will be used: + * + * - If the parameters provide explicit timeouts and tolerances, they will be + * used to build a table with up to 2 non-operational states to transition to. + * The default parameter values were selected based on the values used by + * Microsoft's and Intel's NVMe drivers. Yet, since we don't implement dynamic + * regeneration of the APST table in the event of switching between external + * and battery power, the timeouts and tolerances reflect a compromise + * between values used by Microsoft for AC and battery scenarios. + * - If not, we'll configure the table with a simple heuristic: we are willing + * to spend at most 2% of the time transitioning between power states. + * Therefore, when running in any given state, we will enter the next + * lower-power non-operational state after waiting 50 * (enlat + exlat) + * microseconds, as long as that state's exit latency is under the requested + * maximum latency. * * We will not autonomously enter any non-operational state for which the total * latency exceeds ps_max_latency_us. @@ -2240,6 +2300,7 @@ static int nvme_configure_apst(struct nvme_ctrl *ctrl) int max_ps = -1; int state; int ret; + unsigned last_lt_index = UINT_MAX; /* * If APST isn't supported or if we haven't been initialized yet, @@ -2298,13 +2359,19 @@ static int nvme_configure_apst(struct nvme_ctrl *ctrl) le32_to_cpu(ctrl->psd[state].entry_lat); /* - * This state is good. Use it as the APST idle target for - * higher power states. + * This state is good. It can be used as the APST idle target + * for higher power states. */ - transition_ms = total_latency_us + 19; - do_div(transition_ms, 20); - if (transition_ms > (1 << 24) - 1) - transition_ms = (1 << 24) - 1; + if (apst_primary_timeout_ms && apst_primary_latency_tol_us) { + if (!nvme_apst_get_transition_time(total_latency_us, + &transition_ms, &last_lt_index)) + continue; + } else { + transition_ms = total_latency_us + 19; + do_div(transition_ms, 20); + if (transition_ms > (1 << 24) - 1) + transition_ms = (1 << 24) - 1; + } target = cpu_to_le64((state << 3) | (transition_ms << 8)); if (max_ps == -1) -- cgit From e21e0243e7b0f1c2a21d21f4d115f7b37175772a Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Fri, 28 May 2021 11:02:34 -0500 Subject: nvme-pci: look for StorageD3Enable on companion ACPI device instead The documentation around the StorageD3Enable property hints that it should be made on the PCI device. This is where newer AMD systems set the property and it's required for S0i3 support. So rather than look for nodes of the root port only present on Intel systems, switch to the companion ACPI device for all systems. David Box from Intel indicated this should work on Intel as well. Link: https://lore.kernel.org/linux-nvme/YK6gmAWqaRmvpJXb@google.com/T/#m900552229fa455867ee29c33b854845fce80ba70 Link: https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/power-management-for-storage-hardware-devices-intro Fixes: df4f9bc4fb9c ("nvme-pci: add support for ACPI StorageD3Enable property") Suggested-by: Liang Prike Acked-by: Raul E Rangel Signed-off-by: Mario Limonciello Reviewed-by: David E. Box Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index a29b170701fc..3aa7245a505f 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2831,10 +2831,7 @@ static unsigned long check_vendor_combination_bug(struct pci_dev *pdev) #ifdef CONFIG_ACPI static bool nvme_acpi_storage_d3(struct pci_dev *dev) { - struct acpi_device *adev; - struct pci_dev *root; - acpi_handle handle; - acpi_status status; + struct acpi_device *adev = ACPI_COMPANION(&dev->dev); u8 val; /* @@ -2842,28 +2839,9 @@ static bool nvme_acpi_storage_d3(struct pci_dev *dev) * must use D3 to support deep platform power savings during * suspend-to-idle. */ - root = pcie_find_root_port(dev); - if (!root) - return false; - adev = ACPI_COMPANION(&root->dev); if (!adev) return false; - - /* - * The property is defined in the PXSX device for South complex ports - * and in the PEGP device for North complex ports. - */ - status = acpi_get_handle(adev->handle, "PXSX", &handle); - if (ACPI_FAILURE(status)) { - status = acpi_get_handle(adev->handle, "PEGP", &handle); - if (ACPI_FAILURE(status)) - return false; - } - - if (acpi_bus_get_device(handle, &adev)) - return false; - if (fwnode_property_read_u8(acpi_fwnode_handle(adev), "StorageD3Enable", &val)) return false; -- cgit From 3ede8f72a9a2825efca23a3552e80a1202ea88fd Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Thu, 20 May 2021 15:09:34 -0400 Subject: nvme-tcp: allow selecting the network interface for connections In our application, we need a way to force TCP connections to go out a specific IP interface instead of letting Linux select the interface based on the routing tables. Add the 'host-iface' option to allow specifying the interface to use. When the option host-iface is specified, the driver uses the specified interface to set the option SO_BINDTODEVICE on the TCP socket before connecting. This new option is needed in addtion to the existing host-traddr for the following reasons: Specifying an IP interface by its associated IP address is less intuitive than specifying the actual interface name and, in some cases, simply doesn't work. That's because the association between interfaces and IP addresses is not predictable. IP addresses can be changed or can change by themselves over time (e.g. DHCP). Interface names are predictable [1] and will persist over time. Consider the following configuration. 1: lo: mtu 65536 qdisc noqueue state ... link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 inet 100.0.0.100/24 scope global lo valid_lft forever preferred_lft forever 2: enp0s3: mtu 1500 qdisc ... link/ether 08:00:27:21:65:ec brd ff:ff:ff:ff:ff:ff inet 100.0.0.100/24 scope global enp0s3 valid_lft forever preferred_lft forever 3: enp0s8: mtu 1500 qdisc ... link/ether 08:00:27:4f:95:5c brd ff:ff:ff:ff:ff:ff inet 100.0.0.100/24 scope global enp0s8 valid_lft forever preferred_lft forever The above is a VM that I configured with the same IP address (100.0.0.100) on all interfaces. Doing a reverse lookup to identify the unique interface associated with 100.0.0.100 does not work here. And this is why the option host_iface is required. I understand that the above config does not represent a standard host system, but I'm using this to prove a point: "We can never know how users will configure their systems". By te way, The above configuration is perfectly fine by Linux. The current TCP implementation for host_traddr performs a bind()-before-connect(). This is a common construct to set the source IP address on a TCP socket before connecting. This has no effect on how Linux selects the interface for the connection. That's because Linux uses the Weak End System model as described in RFC1122 [2]. On the other hand, setting the Source IP Address has benefits and should be supported by linux-nvme. In fact, setting the Source IP Address is a mandatory FedGov requirement (e.g. connection to a RADIUS/TACACS+ server). Consider the following configuration. $ ip addr list dev enp0s8 3: enp0s8: mtu 1500 qdisc ... link/ether 08:00:27:4f:95:5c brd ff:ff:ff:ff:ff:ff inet 192.168.56.101/24 brd 192.168.56.255 scope global enp0s8 valid_lft 426sec preferred_lft 426sec inet 192.168.56.102/24 scope global secondary enp0s8 valid_lft forever preferred_lft forever inet 192.168.56.103/24 scope global secondary enp0s8 valid_lft forever preferred_lft forever inet 192.168.56.104/24 scope global secondary enp0s8 valid_lft forever preferred_lft forever Here we can see that several addresses are associated with interface enp0s8. By default, Linux always selects the default IP address, 192.168.56.101, as the source address when connecting over interface enp0s8. Some users, however, want the ability to specify a different source address (e.g., 192.168.56.102, 192.168.56.103, ...). The option host_traddr can be used as-is to perform this function. In conclusion, I believe that we need 2 options for TCP connections. One that can be used to specify an interface (host-iface). And one that can be used to set the source address (host-traddr). Users should be allowed to use one or the other, or both, or none. Of course, the documentation for host_traddr will need some clarification. It should state that when used for TCP connection, this option only sets the source address. And the documentation for host_iface should say that this option is only available for TCP connections. References: [1] https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/ [2] https://tools.ietf.org/html/rfc1122 Tested both IPv4 and IPv6 connections. Signed-off-by: Martin Belanger Reviewed-by: Sagi Grimberg Reviewed-by: Hannes Reinecke Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 5 +++++ drivers/nvme/host/fabrics.c | 14 ++++++++++++++ drivers/nvme/host/fabrics.h | 6 +++++- drivers/nvme/host/tcp.c | 27 ++++++++++++++++++++++++++- 4 files changed, 50 insertions(+), 2 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index e7441ccaa8db..bb8b242594f9 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4134,6 +4134,11 @@ static int nvme_class_uevent(struct device *dev, struct kobj_uevent_env *env) ret = add_uevent_var(env, "NVME_HOST_TRADDR=%s", opts->host_traddr ?: "none"); + if (ret) + return ret; + + ret = add_uevent_var(env, "NVME_HOST_IFACE=%s", + opts->host_iface ?: "none"); } return ret; } diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index a2bb7fc63a73..76dc3eaf46f3 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -112,6 +112,9 @@ int nvmf_get_address(struct nvme_ctrl *ctrl, char *buf, int size) if (ctrl->opts->mask & NVMF_OPT_HOST_TRADDR) len += scnprintf(buf + len, size - len, "%shost_traddr=%s", (len) ? "," : "", ctrl->opts->host_traddr); + if (ctrl->opts->mask & NVMF_OPT_HOST_IFACE) + len += scnprintf(buf + len, size - len, "%shost_iface=%s", + (len) ? "," : "", ctrl->opts->host_iface); len += scnprintf(buf + len, size - len, "\n"); return len; @@ -545,6 +548,7 @@ static const match_table_t opt_tokens = { { NVMF_OPT_KATO, "keep_alive_tmo=%d" }, { NVMF_OPT_HOSTNQN, "hostnqn=%s" }, { NVMF_OPT_HOST_TRADDR, "host_traddr=%s" }, + { NVMF_OPT_HOST_IFACE, "host_iface=%s" }, { NVMF_OPT_HOST_ID, "hostid=%s" }, { NVMF_OPT_DUP_CONNECT, "duplicate_connect" }, { NVMF_OPT_DISABLE_SQFLOW, "disable_sqflow" }, @@ -754,6 +758,15 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts, kfree(opts->host_traddr); opts->host_traddr = p; break; + case NVMF_OPT_HOST_IFACE: + p = match_strdup(args); + if (!p) { + ret = -ENOMEM; + goto out; + } + kfree(opts->host_iface); + opts->host_iface = p; + break; case NVMF_OPT_HOST_ID: p = match_strdup(args); if (!p) { @@ -938,6 +951,7 @@ void nvmf_free_options(struct nvmf_ctrl_options *opts) kfree(opts->trsvcid); kfree(opts->subsysnqn); kfree(opts->host_traddr); + kfree(opts->host_iface); kfree(opts); } EXPORT_SYMBOL_GPL(nvmf_free_options); diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h index d7f7974dc208..c31dad69a773 100644 --- a/drivers/nvme/host/fabrics.h +++ b/drivers/nvme/host/fabrics.h @@ -66,6 +66,7 @@ enum { NVMF_OPT_NR_POLL_QUEUES = 1 << 18, NVMF_OPT_TOS = 1 << 19, NVMF_OPT_FAIL_FAST_TMO = 1 << 20, + NVMF_OPT_HOST_IFACE = 1 << 21, }; /** @@ -83,7 +84,9 @@ enum { * @trsvcid: The transport-specific TRSVCID field for a port on the * subsystem which is adding a controller. * @host_traddr: A transport-specific field identifying the NVME host port - * to use for the connection to the controller. + * to use for the connection to the controller. + * @host_iface: A transport-specific field identifying the NVME host + * interface to use for the connection to the controller. * @queue_size: Number of IO queue elements. * @nr_io_queues: Number of controller IO queues that will be established. * @reconnect_delay: Time between two consecutive reconnect attempts. @@ -108,6 +111,7 @@ struct nvmf_ctrl_options { char *traddr; char *trsvcid; char *host_traddr; + char *host_iface; size_t queue_size; unsigned int nr_io_queues; unsigned int reconnect_delay; diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 34f4b3402f7c..5fc6c568c626 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -123,6 +123,7 @@ struct nvme_tcp_ctrl { struct blk_mq_tag_set admin_tag_set; struct sockaddr_storage addr; struct sockaddr_storage src_addr; + struct net_device *ndev; struct nvme_ctrl ctrl; struct work_struct err_work; @@ -1455,6 +1456,20 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, } } + if (nctrl->opts->mask & NVMF_OPT_HOST_IFACE) { + char *iface = nctrl->opts->host_iface; + sockptr_t optval = KERNEL_SOCKPTR(iface); + + ret = sock_setsockopt(queue->sock, SOL_SOCKET, SO_BINDTODEVICE, + optval, strlen(iface)); + if (ret) { + dev_err(nctrl->device, + "failed to bind to interface %s queue %d err %d\n", + iface, qid, ret); + goto err_sock; + } + } + queue->hdr_digest = nctrl->opts->hdr_digest; queue->data_digest = nctrl->opts->data_digest; if (queue->hdr_digest || queue->data_digest) { @@ -2515,6 +2530,16 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev, } } + if (opts->mask & NVMF_OPT_HOST_IFACE) { + ctrl->ndev = dev_get_by_name(&init_net, opts->host_iface); + if (!ctrl->ndev) { + pr_err("invalid interface passed: %s\n", + opts->host_iface); + ret = -ENODEV; + goto out_free_ctrl; + } + } + if (!opts->duplicate_connect && nvme_tcp_existing_controller(opts)) { ret = -EALREADY; goto out_free_ctrl; @@ -2571,7 +2596,7 @@ static struct nvmf_transport_ops nvme_tcp_transport = { NVMF_OPT_HOST_TRADDR | NVMF_OPT_CTRL_LOSS_TMO | NVMF_OPT_HDR_DIGEST | NVMF_OPT_DATA_DIGEST | NVMF_OPT_NR_WRITE_QUEUES | NVMF_OPT_NR_POLL_QUEUES | - NVMF_OPT_TOS, + NVMF_OPT_TOS | NVMF_OPT_HOST_IFACE, .create_ctrl = nvme_tcp_create_ctrl, }; -- cgit From 25e1de8c40c57bb6be4ecd601641691cfd8a7923 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Fri, 21 May 2021 15:41:57 -0700 Subject: nvme-fabrics: fix the kerneldco comment for nvmf_log_connect_error() Fix the comment style that matches existing code. No functionality change in this patch. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 76dc3eaf46f3..1d20105bb283 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -257,19 +257,15 @@ int nvmf_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val) EXPORT_SYMBOL_GPL(nvmf_reg_write32); /** - * nvmf_log_connect_error() - Error-parsing-diagnostic print - * out function for connect() errors. - * - * @ctrl: the specific /dev/nvmeX device that had the error. - * - * @errval: Error code to be decoded in a more human-friendly - * printout. - * - * @offset: For use with the NVMe error code NVME_SC_CONNECT_INVALID_PARAM. - * - * @cmd: This is the SQE portion of a submission capsule. - * - * @data: This is the "Data" portion of a submission capsule. + * nvmf_log_connect_error() - Error-parsing-diagnostic print out function for + * connect() errors. + * @ctrl: The specific /dev/nvmeX device that had the error. + * @errval: Error code to be decoded in a more human-friendly + * printout. + * @offset: For use with the NVMe error code + * NVME_SC_CONNECT_INVALID_PARAM. + * @cmd: This is the SQE portion of a submission capsule. + * @data: This is the "Data" portion of a submission capsule. */ static void nvmf_log_connect_error(struct nvme_ctrl *ctrl, int errval, int offset, struct nvme_command *cmd, -- cgit From 63d20f54a3d0cff17145716caff03a0d161abf44 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Thu, 3 Jun 2021 10:28:03 +0300 Subject: nvme-fabrics: remove extra new lines in the switch Remove the extra lines in the switch block that is not common practice in the kernel code. No functionality change in this patch. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 1d20105bb283..d71ffcbc3296 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -274,7 +274,6 @@ static void nvmf_log_connect_error(struct nvme_ctrl *ctrl, int err_sctype = errval & (~NVME_SC_DNR); switch (err_sctype) { - case (NVME_SC_CONNECT_INVALID_PARAM): if (offset >> 16) { char *inv_data = "Connect Invalid Data Parameter"; @@ -317,24 +316,24 @@ static void nvmf_log_connect_error(struct nvme_ctrl *ctrl, } } break; - case NVME_SC_CONNECT_INVALID_HOST: dev_err(ctrl->device, "Connect for subsystem %s is not allowed, hostnqn: %s\n", data->subsysnqn, data->hostnqn); break; - case NVME_SC_CONNECT_CTRL_BUSY: dev_err(ctrl->device, "Connect command failed: controller is busy or not available\n"); break; - case NVME_SC_CONNECT_FORMAT: dev_err(ctrl->device, "Connect incompatible format: %d", cmd->connect.recfmt); break; - + case NVME_SC_HOST_PATH_ERROR: + dev_err(ctrl->device, + "Connect command failed: host path error\n"); + break; default: dev_err(ctrl->device, "Connect command failed, error wo/DNR bit: %d\n", -- cgit From 6f860c922532afaae33a968b0d1df3ddf9a8d8a7 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Fri, 21 May 2021 15:41:59 -0700 Subject: nvme-fabrics: remove an extra comment Remove the comment at the end of the switch that is not needed as function is small enough. No functionality change in this patch. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index d71ffcbc3296..78527690c947 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -339,7 +339,7 @@ static void nvmf_log_connect_error(struct nvme_ctrl *ctrl, "Connect command failed, error wo/DNR bit: %d\n", err_sctype); break; - } /* switch (err_sctype) */ + } } /** -- cgit From 97ba6931ba881ea23f3758bbbde7a07a98bff4f9 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Fri, 21 May 2021 15:42:00 -0700 Subject: nvme-fabrics: remove extra braces No need to use the braces around ~ operator. No functionality change in this patch. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 78527690c947..1239a63e3ac2 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -271,7 +271,7 @@ static void nvmf_log_connect_error(struct nvme_ctrl *ctrl, int errval, int offset, struct nvme_command *cmd, struct nvmf_connect_data *data) { - int err_sctype = errval & (~NVME_SC_DNR); + int err_sctype = errval & ~NVME_SC_DNR; switch (err_sctype) { case (NVME_SC_CONNECT_INVALID_PARAM): -- cgit From f423c85cd392241f1521887b1396038cd1e4c68e Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 19 May 2021 09:02:59 +0200 Subject: nvme: open code nvme_put_ns_from_disk in nvme_ns_head_chr_ioctl nvme_ns_head_chr_ioctl is always used on multipath nodes, so just call srcu_read_unlock and consolidate the two unlock paths. Signed-off-by: Christoph Hellwig Reviewed-by: Keith Busch Reviewed-by: Sagi Grimberg --- drivers/nvme/host/ioctl.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 9557ead02de1..0341767ff2e7 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c @@ -419,21 +419,19 @@ long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd, container_of(cdev, struct nvme_ns_head, cdev); void __user *argp = (void __user *)arg; struct nvme_ns *ns; - int srcu_idx, ret; + int srcu_idx, ret = -EWOULDBLOCK; srcu_idx = srcu_read_lock(&head->srcu); ns = nvme_find_path(head); - if (!ns) { - srcu_read_unlock(&head->srcu, srcu_idx); - return -EWOULDBLOCK; - } + if (!ns) + goto out_unlock; if (is_ctrl_ioctl(cmd)) return nvme_ns_head_ctrl_ioctl(ns, cmd, argp, head, srcu_idx); ret = nvme_ns_ioctl(ns, cmd, argp); - nvme_put_ns_from_disk(head, srcu_idx); - +out_unlock: + srcu_read_unlock(&head->srcu, srcu_idx); return ret; } #endif /* CONFIG_NVME_MULTIPATH */ -- cgit From 86b4284d98d6a47033b7bfc5b029a4fc45e4d370 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 19 May 2021 09:04:26 +0200 Subject: nvme: open code nvme_{get,put}_ns_from_disk in nvme_ns_head_ioctl nvme_ns_head_ioctl is always used on multipath nodes, no need to deal with the de-multiplexers. Signed-off-by: Christoph Hellwig Reviewed-by: Keith Busch Reviewed-by: Sagi Grimberg --- drivers/nvme/host/ioctl.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 0341767ff2e7..3f84bd3b9259 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c @@ -387,14 +387,15 @@ static int nvme_ns_head_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd, int nvme_ns_head_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg) { - struct nvme_ns_head *head = NULL; + struct nvme_ns_head *head = bdev->bd_disk->private_data; void __user *argp = (void __user *)arg; struct nvme_ns *ns; - int srcu_idx, ret; + int srcu_idx, ret = -EWOULDBLOCK; - ns = nvme_get_ns_from_disk(bdev->bd_disk, &head, &srcu_idx); - if (unlikely(!ns)) - return -EWOULDBLOCK; + srcu_idx = srcu_read_lock(&head->srcu); + ns = nvme_find_path(head); + if (!ns) + goto out_unlock; /* * Handle ioctls that apply to the controller instead of the namespace @@ -402,12 +403,11 @@ int nvme_ns_head_ioctl(struct block_device *bdev, fmode_t mode, * deadlock when deleting namespaces using the passthrough interface. */ if (is_ctrl_ioctl(cmd)) - ret = nvme_ns_head_ctrl_ioctl(ns, cmd, argp, head, srcu_idx); - else { - ret = nvme_ns_ioctl(ns, cmd, argp); - nvme_put_ns_from_disk(head, srcu_idx); - } + return nvme_ns_head_ctrl_ioctl(ns, cmd, argp, head, srcu_idx); + ret = nvme_ns_ioctl(ns, cmd, argp); +out_unlock: + srcu_read_unlock(&head->srcu, srcu_idx); return ret; } -- cgit From 3e7d1a55165bdce2aaf1139ee8889e68eb29c263 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 19 May 2021 09:08:41 +0200 Subject: nvme: open code nvme_put_ns_from_disk in nvme_ns_head_ctrl_ioctl nvme_ns_head_ctrl_ioctl is always used on multipath nodes, so just call srcu_read_unlock directly. Signed-off-by: Christoph Hellwig Reviewed-by: Keith Busch Reviewed-by: Sagi Grimberg --- drivers/nvme/host/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 3f84bd3b9259..2c6969ffe85c 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c @@ -377,7 +377,7 @@ static int nvme_ns_head_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd, int ret; nvme_get_ctrl(ns->ctrl); - nvme_put_ns_from_disk(head, srcu_idx); + srcu_read_unlock(&head->srcu, srcu_idx); ret = nvme_ctrl_ioctl(ns->ctrl, cmd, argp); nvme_put_ctrl(ctrl); -- cgit From 85b790a7ae0518dd745bbb97d532b83840d2db04 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 19 May 2021 09:09:56 +0200 Subject: nvme: add a sparse annotation to nvme_ns_head_ctrl_ioctl Add the __releases annotation to tell sparse that nvme_ns_head_ctrl_ioctl is expected to unlock head->srcu. Signed-off-by: Christoph Hellwig Reviewed-by: Keith Busch Reviewed-by: Sagi Grimberg --- drivers/nvme/host/ioctl.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 2c6969ffe85c..2e7780ea0354 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c @@ -372,6 +372,7 @@ long nvme_ns_chr_ioctl(struct file *file, unsigned int cmd, unsigned long arg) #ifdef CONFIG_NVME_MULTIPATH static int nvme_ns_head_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd, void __user *argp, struct nvme_ns_head *head, int srcu_idx) + __releases(&head->srcu) { struct nvme_ctrl *ctrl = ns->ctrl; int ret; -- cgit From d8ca66e82191a9a95926f7f129028bd362202d5d Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 19 May 2021 09:11:54 +0200 Subject: nvme: move the CSI sanity check into nvme_ns_report_zones Move the CSI check into nvme_ns_report_zones to clean up the code a little bit and prepare for further refactoring. Signed-off-by: Christoph Hellwig Reviewed-by: Keith Busch Reviewed-by: Sagi Grimberg --- drivers/nvme/host/zns.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c index 475dd45c3db4..31e789ecd940 100644 --- a/drivers/nvme/host/zns.c +++ b/drivers/nvme/host/zns.c @@ -180,6 +180,9 @@ static int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector, unsigned int nz, i; size_t buflen; + if (ns->head->ids.csi != NVME_CSI_ZNS) + return -EINVAL; + report = nvme_zns_alloc_report_buffer(ns, nr_zones, &buflen); if (!report) return -ENOMEM; @@ -237,11 +240,7 @@ int nvme_report_zones(struct gendisk *disk, sector_t sector, ns = nvme_get_ns_from_disk(disk, &head, &srcu_idx); if (unlikely(!ns)) return -EWOULDBLOCK; - - if (ns->head->ids.csi == NVME_CSI_ZNS) - ret = nvme_ns_report_zones(ns, sector, nr_zones, cb, data); - else - ret = -EINVAL; + ret = nvme_ns_report_zones(ns, sector, nr_zones, cb, data); nvme_put_ns_from_disk(head, srcu_idx); return ret; -- cgit From 8b4fb0f968ffe73f619c06cb4040ecaa60327098 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 19 May 2021 09:17:06 +0200 Subject: nvme: split nvme_report_zones Split multipath support out of nvme_report_zones into a separate helper and simplify the non-multipath version as a result. Signed-off-by: Christoph Hellwig Reviewed-by: Keith Busch Reviewed-by: Sagi Grimberg --- drivers/nvme/host/core.c | 11 +++++++++++ drivers/nvme/host/multipath.c | 21 ++++++++++++++++++++- drivers/nvme/host/nvme.h | 7 ++----- drivers/nvme/host/zns.c | 20 ++------------------ 4 files changed, 35 insertions(+), 24 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index bb8b242594f9..47cfc8a28e45 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2073,6 +2073,17 @@ int nvme_sec_submit(void *data, u16 spsp, u8 secp, void *buffer, size_t len, EXPORT_SYMBOL_GPL(nvme_sec_submit); #endif /* CONFIG_BLK_SED_OPAL */ +#ifdef CONFIG_BLK_DEV_ZONED +static int nvme_report_zones(struct gendisk *disk, sector_t sector, + unsigned int nr_zones, report_zones_cb cb, void *data) +{ + return nvme_ns_report_zones(disk->private_data, sector, nr_zones, cb, + data); +} +#else +#define nvme_report_zones NULL +#endif /* CONFIG_BLK_DEV_ZONED */ + static const struct block_device_operations nvme_bdev_ops = { .owner = THIS_MODULE, .ioctl = nvme_ioctl, diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index f81871c7128a..127a17b4c13d 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -349,6 +349,25 @@ static void nvme_ns_head_release(struct gendisk *disk, fmode_t mode) nvme_put_ns_head(disk->private_data); } +#ifdef CONFIG_BLK_DEV_ZONED +static int nvme_ns_head_report_zones(struct gendisk *disk, sector_t sector, + unsigned int nr_zones, report_zones_cb cb, void *data) +{ + struct nvme_ns_head *head = disk->private_data; + struct nvme_ns *ns; + int srcu_idx, ret = -EWOULDBLOCK; + + srcu_idx = srcu_read_lock(&head->srcu); + ns = nvme_find_path(head); + if (ns) + ret = nvme_ns_report_zones(ns, sector, nr_zones, cb, data); + srcu_read_unlock(&head->srcu, srcu_idx); + return ret; +} +#else +#define nvme_ns_head_report_zones NULL +#endif /* CONFIG_BLK_DEV_ZONED */ + const struct block_device_operations nvme_ns_head_ops = { .owner = THIS_MODULE, .submit_bio = nvme_ns_head_submit_bio, @@ -356,7 +375,7 @@ const struct block_device_operations nvme_ns_head_ops = { .release = nvme_ns_head_release, .ioctl = nvme_ns_head_ioctl, .getgeo = nvme_getgeo, - .report_zones = nvme_report_zones, + .report_zones = nvme_ns_head_report_zones, .pr_ops = &nvme_pr_ops, }; diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 0015860ec12b..01f41b2bf915 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -810,17 +810,14 @@ static inline void nvme_mpath_start_freeze(struct nvme_subsystem *subsys) #endif /* CONFIG_NVME_MULTIPATH */ int nvme_revalidate_zones(struct nvme_ns *ns); +int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector, + unsigned int nr_zones, report_zones_cb cb, void *data); #ifdef CONFIG_BLK_DEV_ZONED int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf); -int nvme_report_zones(struct gendisk *disk, sector_t sector, - unsigned int nr_zones, report_zones_cb cb, void *data); - blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns, struct request *req, struct nvme_command *cmnd, enum nvme_zone_mgmt_action action); #else -#define nvme_report_zones NULL - static inline blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns, struct request *req, struct nvme_command *cmnd, enum nvme_zone_mgmt_action action) diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c index 31e789ecd940..d95010481fce 100644 --- a/drivers/nvme/host/zns.c +++ b/drivers/nvme/host/zns.c @@ -171,8 +171,8 @@ static int nvme_zone_parse_entry(struct nvme_ns *ns, return cb(&zone, idx, data); } -static int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector, - unsigned int nr_zones, report_zones_cb cb, void *data) +int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector, + unsigned int nr_zones, report_zones_cb cb, void *data) { struct nvme_zone_report *report; struct nvme_command c = { }; @@ -230,22 +230,6 @@ out_free: return ret; } -int nvme_report_zones(struct gendisk *disk, sector_t sector, - unsigned int nr_zones, report_zones_cb cb, void *data) -{ - struct nvme_ns_head *head = NULL; - struct nvme_ns *ns; - int srcu_idx, ret; - - ns = nvme_get_ns_from_disk(disk, &head, &srcu_idx); - if (unlikely(!ns)) - return -EWOULDBLOCK; - ret = nvme_ns_report_zones(ns, sector, nr_zones, cb, data); - nvme_put_ns_from_disk(head, srcu_idx); - - return ret; -} - blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns, struct request *req, struct nvme_command *c, enum nvme_zone_mgmt_action action) { -- cgit From f1cf35e17ec308c0e76f55c6bccf84fff1a2d71a Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 19 May 2021 09:22:35 +0200 Subject: nvme: remove nvme_{get,put}_ns_from_disk Now that only one caller is left remove the helpers by restructuring nvme_pr_command so that it has two helpers for sending a command of to a given nsid using either the ns_head for multipath, or the namespace stored in the gendisk. Signed-off-by: Christoph Hellwig Reviewed-by: Keith Busch Reviewed-by: Sagi Grimberg --- drivers/nvme/host/core.c | 68 +++++++++++++++++++----------------------------- drivers/nvme/host/nvme.h | 5 +--- 2 files changed, 28 insertions(+), 45 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 47cfc8a28e45..177cae44b612 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1542,36 +1542,6 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl) queue_work(nvme_wq, &ctrl->async_event_work); } -/* - * Issue ioctl requests on the first available path. Note that unlike normal - * block layer requests we will not retry failed request on another controller. - */ -struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk, - struct nvme_ns_head **head, int *srcu_idx) -{ -#ifdef CONFIG_NVME_MULTIPATH - if (disk->fops == &nvme_ns_head_ops) { - struct nvme_ns *ns; - - *head = disk->private_data; - *srcu_idx = srcu_read_lock(&(*head)->srcu); - ns = nvme_find_path(*head); - if (!ns) - srcu_read_unlock(&(*head)->srcu, *srcu_idx); - return ns; - } -#endif - *head = NULL; - *srcu_idx = -1; - return disk->private_data; -} - -void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx) -{ - if (head) - srcu_read_unlock(&head->srcu, idx); -} - static int nvme_ns_open(struct nvme_ns *ns) { @@ -1968,30 +1938,46 @@ static char nvme_pr_type(enum pr_type type) } }; +static int nvme_send_ns_head_pr_command(struct block_device *bdev, + struct nvme_command *c, u8 data[16]) +{ + struct nvme_ns_head *head = bdev->bd_disk->private_data; + int srcu_idx = srcu_read_lock(&head->srcu); + struct nvme_ns *ns = nvme_find_path(head); + int ret = -EWOULDBLOCK; + + if (ns) { + c->common.nsid = cpu_to_le32(ns->head->ns_id); + ret = nvme_submit_sync_cmd(ns->queue, c, data, 16); + } + srcu_read_unlock(&head->srcu, srcu_idx); + return ret; +} + +static int nvme_send_ns_pr_command(struct nvme_ns *ns, struct nvme_command *c, + u8 data[16]) +{ + c->common.nsid = cpu_to_le32(ns->head->ns_id); + return nvme_submit_sync_cmd(ns->queue, c, data, 16); +} + static int nvme_pr_command(struct block_device *bdev, u32 cdw10, u64 key, u64 sa_key, u8 op) { - struct nvme_ns_head *head = NULL; - struct nvme_ns *ns; struct nvme_command c; - int srcu_idx, ret; u8 data[16] = { 0, }; - ns = nvme_get_ns_from_disk(bdev->bd_disk, &head, &srcu_idx); - if (unlikely(!ns)) - return -EWOULDBLOCK; - put_unaligned_le64(key, &data[0]); put_unaligned_le64(sa_key, &data[8]); memset(&c, 0, sizeof(c)); c.common.opcode = op; - c.common.nsid = cpu_to_le32(ns->head->ns_id); c.common.cdw10 = cpu_to_le32(cdw10); - ret = nvme_submit_sync_cmd(ns->queue, &c, data, 16); - nvme_put_ns_from_disk(head, srcu_idx); - return ret; + if (IS_ENABLED(CONFIG_NVME_MULTIPATH) && + bdev->bd_disk->fops == &nvme_ns_head_ops) + return nvme_send_ns_head_pr_command(bdev, &c, data); + return nvme_send_ns_pr_command(bdev->bd_disk->private_data, &c, data); } static int nvme_pr_register(struct block_device *bdev, u64 old, diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 01f41b2bf915..1f397ecba16c 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -674,9 +674,6 @@ int nvme_delete_ctrl(struct nvme_ctrl *ctrl); void nvme_queue_scan(struct nvme_ctrl *ctrl); int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi, void *log, size_t size, u64 offset); -struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk, - struct nvme_ns_head **head, int *srcu_idx); -void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx); bool nvme_tryget_ns_head(struct nvme_ns_head *head); void nvme_put_ns_head(struct nvme_ns_head *head); int nvme_cdev_add(struct cdev *cdev, struct device *cdev_device, @@ -697,6 +694,7 @@ extern const struct attribute_group *nvme_ns_id_attr_groups[]; extern const struct pr_ops nvme_pr_ops; extern const struct block_device_operations nvme_ns_head_ops; +struct nvme_ns *nvme_find_path(struct nvme_ns_head *head); #ifdef CONFIG_NVME_MULTIPATH static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl) { @@ -718,7 +716,6 @@ void nvme_mpath_uninit(struct nvme_ctrl *ctrl); void nvme_mpath_stop(struct nvme_ctrl *ctrl); bool nvme_mpath_clear_current_path(struct nvme_ns *ns); void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl); -struct nvme_ns *nvme_find_path(struct nvme_ns_head *head); static inline void nvme_mpath_check_last_path(struct nvme_ns *ns) { -- cgit From 2744d7a0733503931b71c00d156119ced002f22c Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Wed, 9 Jun 2021 13:40:17 -0500 Subject: ACPI: Check StorageD3Enable _DSD property in ACPI code Although first implemented for NVME, this check may be usable by other drivers as well. Microsoft's specification explicitly mentions that is may be usable by SATA and AHCI devices. Google also indicates that they have used this with SDHCI in a downstream kernel tree that a user can plug a storage device into. Link: https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/power-management-for-storage-hardware-devices-intro Suggested-by: Keith Busch CC: Shyam-sundar S-k CC: Alexander Deucher CC: Rafael J. Wysocki CC: Prike Liang Signed-off-by: Mario Limonciello Reviewed-by: Rafael J. Wysocki Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 3aa7245a505f..8fbc4c87a0d8 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2828,32 +2828,6 @@ static unsigned long check_vendor_combination_bug(struct pci_dev *pdev) return 0; } -#ifdef CONFIG_ACPI -static bool nvme_acpi_storage_d3(struct pci_dev *dev) -{ - struct acpi_device *adev = ACPI_COMPANION(&dev->dev); - u8 val; - - /* - * Look for _DSD property specifying that the storage device on the port - * must use D3 to support deep platform power savings during - * suspend-to-idle. - */ - - if (!adev) - return false; - if (fwnode_property_read_u8(acpi_fwnode_handle(adev), "StorageD3Enable", - &val)) - return false; - return val == 1; -} -#else -static inline bool nvme_acpi_storage_d3(struct pci_dev *dev) -{ - return false; -} -#endif /* CONFIG_ACPI */ - static void nvme_async_probe(void *data, async_cookie_t cookie) { struct nvme_dev *dev = data; @@ -2903,7 +2877,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) quirks |= check_vendor_combination_bug(pdev); - if (!noacpi && nvme_acpi_storage_d3(pdev)) { + if (!noacpi && acpi_storage_d3(&pdev->dev)) { /* * Some systems use a bios work around to ask for D3 on * platforms that support kernel managed suspend. -- cgit From 120bb3624d55d65145f7c1bf12a839fd323cde29 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 7 Jun 2021 10:56:56 +0200 Subject: nvme: verify MNAN value if ANA is enabled The controller is required to have a non-zero MNAN value if it supports ANA: If the controller supports Asymmetric Namespace Access Reporting, then this field shall be set to a non-zero value that is less than or equal to the NN value. Reviewed-by: Hannes Reinecke Reviewed-by: Chaitanya Kulkarni Signed-off-by: Daniel Wagner Signed-off-by: Christoph Hellwig --- drivers/nvme/host/multipath.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 127a17b4c13d..98426234d416 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -818,6 +818,13 @@ int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id) !(ctrl->subsys->cmic & NVME_CTRL_CMIC_ANA)) return 0; + if (!ctrl->max_namespaces || + ctrl->max_namespaces > le32_to_cpu(id->nn)) { + dev_err(ctrl->device, + "Invalid MNAN value %u\n", ctrl->max_namespaces); + return -EINVAL; + } + ctrl->anacap = id->anacap; ctrl->anatt = id->anatt; ctrl->nanagrpid = le32_to_cpu(id->nanagrpid); -- cgit From 2411424143bdfad3027e82fe6a66c5aadce271ee Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 7 Jun 2021 10:46:51 +0200 Subject: nvme: remove superfluous bio_set_dev in nvme_requeue_work Commit ce86dad222e9 ("nvme-multipath: reset bdev to ns head when failover") moved the reset code where the bio is added to the requeue_list for the failover path. But it left the original bio_set_dev in nvme_requeue_work. There is a second path to nvme_requee_work. It is via nvme_ns_head_submit_bio. Though we don't have to set bio->bi_bdev for this path either, as it points to the correct bdev already. Let's remove the bio_set_dev. It's updating the bio->bi_bdev with the same pointer and thus it's unnecessary. Signed-off-by: Daniel Wagner Signed-off-by: Christoph Hellwig --- drivers/nvme/host/multipath.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 98426234d416..23573fe3fc7d 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -435,11 +435,6 @@ static void nvme_requeue_work(struct work_struct *work) next = bio->bi_next; bio->bi_next = NULL; - /* - * Reset disk to the mpath node and resubmit to select a new - * path. - */ - bio_set_dev(bio, head->disk->part0); submit_bio_noacct(bio); } } -- cgit From d399742cd02dca6d1ed17ae7db7a366192516591 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 14 Jun 2021 16:16:07 +0200 Subject: nvme: fix grammar in the CONFIG_NVME_MULTIPATH kconfig help text Fix a singular/plural mismatch in the CONFIG_NVME_MULTIPATH help text. Signed-off-by: Geert Uytterhoeven Signed-off-by: Christoph Hellwig --- drivers/nvme/host/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig index a44d49d63968..102292289cdf 100644 --- a/drivers/nvme/host/Kconfig +++ b/drivers/nvme/host/Kconfig @@ -21,7 +21,7 @@ config NVME_MULTIPATH help This option enables support for multipath access to NVMe subsystems. If this option is enabled only a single - /dev/nvmeXnY device will show up for each NVMe namespaces, + /dev/nvmeXnY device will show up for each NVMe namespace, even if it is accessible through multiple controllers. config NVME_HWMON -- cgit From e7d4b5493a2d5a6225fc572e01167e12f89c6a42 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Mon, 7 Jun 2021 12:54:50 -0700 Subject: nvme: factor out a nvme_validate_passthru_nsid helper Add a helper nvme_validate_passthru_nsid() to validate the nsid that removes the nsid validation and error message print code from nvme_user_cmd() and nvme_user_cmd64(). Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/ioctl.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c index 2e7780ea0354..d93928d1e5bd 100644 --- a/drivers/nvme/host/ioctl.c +++ b/drivers/nvme/host/ioctl.c @@ -177,6 +177,20 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio) metadata, meta_len, lower_32_bits(io.slba), NULL, 0); } +static bool nvme_validate_passthru_nsid(struct nvme_ctrl *ctrl, + struct nvme_ns *ns, __u32 nsid) +{ + if (ns && nsid != ns->head->ns_id) { + dev_err(ctrl->device, + "%s: nsid (%u) in cmd does not match nsid (%u)" + "of namespace\n", + current->comm, nsid, ns->head->ns_id); + return false; + } + + return true; +} + static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns, struct nvme_passthru_cmd __user *ucmd) { @@ -192,12 +206,8 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns, return -EFAULT; if (cmd.flags) return -EINVAL; - if (ns && cmd.nsid != ns->head->ns_id) { - dev_err(ctrl->device, - "%s: nsid (%u) in cmd does not match nsid (%u) of namespace\n", - current->comm, cmd.nsid, ns->head->ns_id); + if (!nvme_validate_passthru_nsid(ctrl, ns, cmd.nsid)) return -EINVAL; - } memset(&c, 0, sizeof(c)); c.common.opcode = cmd.opcode; @@ -242,12 +252,8 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns, return -EFAULT; if (cmd.flags) return -EINVAL; - if (ns && cmd.nsid != ns->head->ns_id) { - dev_err(ctrl->device, - "%s: nsid (%u) in cmd does not match nsid (%u) of namespace\n", - current->comm, cmd.nsid, ns->head->ns_id); + if (!nvme_validate_passthru_nsid(ctrl, ns, cmd.nsid)) return -EINVAL; - } memset(&c, 0, sizeof(c)); c.common.opcode = cmd.opcode; -- cgit From 522af60cb2f8e3658bda1902fb7f200dcf888a5c Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 5 Jun 2021 15:48:16 +0300 Subject: nvme-tcp: fix error codes in nvme_tcp_setup_ctrl() These error paths currently return success but they should return -EOPNOTSUPP. Fixes: 73ffcefcfca0 ("nvme-tcp: check sgl supported by target") Fixes: 3f2304f8c6d6 ("nvme-tcp: add NVMe over TCP host driver") Signed-off-by: Dan Carpenter Reviewed-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/tcp.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 5fc6c568c626..6a65b0516180 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -1988,11 +1988,13 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new) return ret; if (ctrl->icdoff) { + ret = -EOPNOTSUPP; dev_err(ctrl->device, "icdoff is not supported!\n"); goto destroy_admin; } if (!(ctrl->sgls & ((1 << 0) | (1 << 1)))) { + ret = -EOPNOTSUPP; dev_err(ctrl->device, "Mandatory sgls are not supported!\n"); goto destroy_admin; } -- cgit From a0aac973a26d1ac814b9e131e209eb39472a67ce Mon Sep 17 00:00:00 2001 From: JK Kim Date: Thu, 17 Jun 2021 15:02:17 +0900 Subject: nvme-pci: fix var. type for increasing cq_head nvmeq->cq_head is compared with nvmeq->q_depth and changed the value and cq_phase for handling the next cq db. but, nvmeq->q_depth's type is u32 and max. value is 0x10000 when CQP.MSQE is 0xffff and io_queue_depth is 0x10000. current temp. variable for comparing with nvmeq->q_depth is overflowed when previous nvmeq->cq_head is 0xffff. in this case, nvmeq->cq_phase is not updated. so, fix data type for temp. variable to u32. Signed-off-by: JK Kim Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 8fbc4c87a0d8..5a72bdf5ad03 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -1032,7 +1032,7 @@ static inline void nvme_handle_cqe(struct nvme_queue *nvmeq, u16 idx) static inline void nvme_update_cq_head(struct nvme_queue *nvmeq) { - u16 tmp = nvmeq->cq_head + 1; + u32 tmp = nvmeq->cq_head + 1; if (tmp == nvmeq->q_depth) { nvmeq->cq_head = 0; -- cgit From cb1b10e7ac6c1438247ee3c7e4a2f2332a77ba07 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Mon, 7 Jun 2021 12:54:54 -0700 Subject: nvme-pci: remove trailing lines for helpers Remove the extra white line at the end of the functions. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 5a72bdf5ad03..138e7e7453dd 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -559,7 +559,6 @@ static void nvme_free_prps(struct nvme_dev *dev, struct request *req) dma_pool_free(dev->prp_page_pool, prp_list, dma_addr); dma_addr = next_dma_addr; } - } static void nvme_free_sgls(struct nvme_dev *dev, struct request *req) @@ -576,7 +575,6 @@ static void nvme_free_sgls(struct nvme_dev *dev, struct request *req) dma_pool_free(dev->prp_page_pool, sg_list, dma_addr); dma_addr = next_dma_addr; } - } static void nvme_unmap_sg(struct nvme_dev *dev, struct request *req) -- cgit From 73eefc270afa1f27d82c42fdb34562d07a834b40 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Wed, 9 Jun 2021 18:28:23 -0700 Subject: nvme: add a helper to check ctrl sgl support For various transports such as fc/tcp/pci it is common to check if NVMe SGLs are supported or not by the controller. In this preparation patch we add a helper to avoid the open coding of such checks in the various transport. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/nvme.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 1f397ecba16c..75420ceacc10 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -869,6 +869,11 @@ static inline void nvme_hwmon_exit(struct nvme_ctrl *ctrl) } #endif +static inline bool nvme_ctrl_sgl_supported(struct nvme_ctrl *ctrl) +{ + return ctrl->sgls & ((1 << 0) | (1 << 1)); +} + u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode); void nvme_execute_passthru_rq(struct request *rq); -- cgit From b61678bcd43c6686a6d0cf965934a54b4225821d Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Wed, 9 Jun 2021 18:28:24 -0700 Subject: nvme-fc: use ctrl sgl check helper Use the helper to check NVMe controller's SGL support. Reviewed-by: James Smart Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c index 256e87721a01..8a3c4814d21b 100644 --- a/drivers/nvme/host/fc.c +++ b/drivers/nvme/host/fc.c @@ -3111,7 +3111,7 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl) } /* FC-NVME supports normal SGL Data Block Descriptors */ - if (!(ctrl->ctrl.sgls & ((1 << 0) | (1 << 1)))) { + if (!nvme_ctrl_sgl_supported(&ctrl->ctrl)) { dev_err(ctrl->ctrl.device, "Mandatory sgls are not supported!\n"); goto out_disconnect_admin_queue; -- cgit From 253a0b76a12a4cce14095b3d74004e67a6434d79 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Wed, 9 Jun 2021 18:28:25 -0700 Subject: nvme-pci: use ctrl sgl check helper Use the helper to check NVMe controller's SGL support. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 138e7e7453dd..12ffd58c27b1 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -536,7 +536,7 @@ static inline bool nvme_pci_use_sgls(struct nvme_dev *dev, struct request *req) avg_seg_size = DIV_ROUND_UP(blk_rq_payload_bytes(req), nseg); - if (!(dev->ctrl.sgls & ((1 << 0) | (1 << 1)))) + if (!nvme_ctrl_sgl_supported(&dev->ctrl)) return false; if (!iod->nvmeq->qid) return false; @@ -853,7 +853,7 @@ static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req, &cmnd->rw, &bv); if (iod->nvmeq->qid && sgl_threshold && - dev->ctrl.sgls & ((1 << 0) | (1 << 1))) + nvme_ctrl_sgl_supported(&dev->ctrl)) return nvme_setup_sgl_simple(dev, req, &cmnd->rw, &bv); } -- cgit From 3b54064fbce73a4dada6019dd400f0ce28ab5eb9 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Wed, 9 Jun 2021 18:28:26 -0700 Subject: nvme-tcp: use ctrl sgl check helper Use the helper to check NVMe controller's SGL support. Reviewed-by: Sagi Grimberg Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/tcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 6a65b0516180..c7bd37103cf4 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -1993,7 +1993,7 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new) goto destroy_admin; } - if (!(ctrl->sgls & ((1 << 0) | (1 << 1)))) { + if (!nvme_ctrl_sgl_supported(ctrl)) { ret = -EOPNOTSUPP; dev_err(ctrl->device, "Mandatory sgls are not supported!\n"); goto destroy_admin; -- cgit From 2796a8e409429a67daeb813ed270eb645f56f817 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Mon, 14 Jun 2021 19:45:51 -0700 Subject: nvme-fabrics: remove memset in nvmf_reg_read64() Declare and initialize structure variable to the zero values so that we can get rid of the zeroout memset call. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 1239a63e3ac2..4753f1e5505e 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -190,11 +190,10 @@ EXPORT_SYMBOL_GPL(nvmf_reg_read32); */ int nvmf_reg_read64(struct nvme_ctrl *ctrl, u32 off, u64 *val) { - struct nvme_command cmd; + struct nvme_command cmd = { }; union nvme_result res; int ret; - memset(&cmd, 0, sizeof(cmd)); cmd.prop_get.opcode = nvme_fabrics_command; cmd.prop_get.fctype = nvme_fabrics_type_property_get; cmd.prop_get.attrib = 1; -- cgit From c22c2720133d51d95da608a77cd703f29d29747e Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Mon, 14 Jun 2021 19:45:52 -0700 Subject: nvme-fabrics: remove memset in nvmf_reg_write32() Declare and initialize structure variable to the zero values so that we can get rid of the zeroout memset call. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 4753f1e5505e..09fe3d97bf44 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -235,10 +235,9 @@ EXPORT_SYMBOL_GPL(nvmf_reg_read64); */ int nvmf_reg_write32(struct nvme_ctrl *ctrl, u32 off, u32 val) { - struct nvme_command cmd; + struct nvme_command cmd = { }; int ret; - memset(&cmd, 0, sizeof(cmd)); cmd.prop_set.opcode = nvme_fabrics_command; cmd.prop_set.fctype = nvme_fabrics_type_property_set; cmd.prop_set.attrib = 0; -- cgit From bfa9d1222d6185a4aea603ebc7d74d75c747087c Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Mon, 14 Jun 2021 19:45:53 -0700 Subject: nvme-fabrics: remove memset in connect admin q Declare and initialize structure variable to the zero values so that we can get rid of the zeroout memset call. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 09fe3d97bf44..43c797e3380b 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -362,12 +362,11 @@ static void nvmf_log_connect_error(struct nvme_ctrl *ctrl, */ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl) { - struct nvme_command cmd; + struct nvme_command cmd = { }; union nvme_result res; struct nvmf_connect_data *data; int ret; - memset(&cmd, 0, sizeof(cmd)); cmd.connect.opcode = nvme_fabrics_command; cmd.connect.fctype = nvme_fabrics_type_connect; cmd.connect.qid = 0; -- cgit From eff4423ec0b03fedb8b7b420549ed8e424d246f1 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Mon, 14 Jun 2021 19:45:54 -0700 Subject: nvme-fabrics: remove memset in connect io q Declare and initialize structure variable to the zero values so that we can get rid of the zeroout memset call. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/fabrics.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 43c797e3380b..1e6a7cc056ca 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -429,12 +429,11 @@ EXPORT_SYMBOL_GPL(nvmf_connect_admin_queue); */ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid, bool poll) { - struct nvme_command cmd; + struct nvme_command cmd = { }; struct nvmf_connect_data *data; union nvme_result res; int ret; - memset(&cmd, 0, sizeof(cmd)); cmd.connect.opcode = nvme_fabrics_command; cmd.connect.fctype = nvme_fabrics_type_connect; cmd.connect.qid = cpu_to_le16(qid); -- cgit From f66e2804d61aef690bb428d8de6a127f844bb240 Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Wed, 16 Jun 2021 15:15:53 -0700 Subject: nvme-pci: remove zeroout memset call for struct Declare and initialize structure variables to zero values so that we can remove zeroout memset calls in the host/pci.c. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 12ffd58c27b1..d3c5086673bc 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -307,13 +307,12 @@ static void nvme_dbbuf_free(struct nvme_queue *nvmeq) static void nvme_dbbuf_set(struct nvme_dev *dev) { - struct nvme_command c; + struct nvme_command c = { }; unsigned int i; if (!dev->dbbuf_dbs) return; - memset(&c, 0, sizeof(c)); c.dbbuf.opcode = nvme_admin_dbbuf; c.dbbuf.prp1 = cpu_to_le64(dev->dbbuf_dbs_dma_addr); c.dbbuf.prp2 = cpu_to_le64(dev->dbbuf_eis_dma_addr); @@ -1112,9 +1111,8 @@ static void nvme_pci_submit_async_event(struct nvme_ctrl *ctrl) { struct nvme_dev *dev = to_nvme_dev(ctrl); struct nvme_queue *nvmeq = &dev->queues[0]; - struct nvme_command c; + struct nvme_command c = { }; - memset(&c, 0, sizeof(c)); c.common.opcode = nvme_admin_async_event; c.common.command_id = NVME_AQ_BLK_MQ_DEPTH; nvme_submit_cmd(nvmeq, &c, true); @@ -1122,9 +1120,8 @@ static void nvme_pci_submit_async_event(struct nvme_ctrl *ctrl) static int adapter_delete_queue(struct nvme_dev *dev, u8 opcode, u16 id) { - struct nvme_command c; + struct nvme_command c = { }; - memset(&c, 0, sizeof(c)); c.delete_queue.opcode = opcode; c.delete_queue.qid = cpu_to_le16(id); @@ -1134,7 +1131,7 @@ static int adapter_delete_queue(struct nvme_dev *dev, u8 opcode, u16 id) static int adapter_alloc_cq(struct nvme_dev *dev, u16 qid, struct nvme_queue *nvmeq, s16 vector) { - struct nvme_command c; + struct nvme_command c = { }; int flags = NVME_QUEUE_PHYS_CONTIG; if (!test_bit(NVMEQ_POLLED, &nvmeq->flags)) @@ -1144,7 +1141,6 @@ static int adapter_alloc_cq(struct nvme_dev *dev, u16 qid, * Note: we (ab)use the fact that the prp fields survive if no data * is attached to the request. */ - memset(&c, 0, sizeof(c)); c.create_cq.opcode = nvme_admin_create_cq; c.create_cq.prp1 = cpu_to_le64(nvmeq->cq_dma_addr); c.create_cq.cqid = cpu_to_le16(qid); @@ -1159,7 +1155,7 @@ static int adapter_alloc_sq(struct nvme_dev *dev, u16 qid, struct nvme_queue *nvmeq) { struct nvme_ctrl *ctrl = &dev->ctrl; - struct nvme_command c; + struct nvme_command c = { }; int flags = NVME_QUEUE_PHYS_CONTIG; /* @@ -1174,7 +1170,6 @@ static int adapter_alloc_sq(struct nvme_dev *dev, u16 qid, * Note: we (ab)use the fact that the prp fields survive if no data * is attached to the request. */ - memset(&c, 0, sizeof(c)); c.create_sq.opcode = nvme_admin_create_sq; c.create_sq.prp1 = cpu_to_le64(nvmeq->sq_dma_addr); c.create_sq.sqid = cpu_to_le16(qid); @@ -1255,7 +1250,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved) struct nvme_queue *nvmeq = iod->nvmeq; struct nvme_dev *dev = nvmeq->dev; struct request *abort_req; - struct nvme_command cmd; + struct nvme_command cmd = { }; u32 csts = readl(dev->bar + NVME_REG_CSTS); /* If PCI error recovery process is happening, we cannot reset or @@ -1335,7 +1330,6 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved) } iod->aborted = 1; - memset(&cmd, 0, sizeof(cmd)); cmd.abort.opcode = nvme_admin_abort_cmd; cmd.abort.cid = req->tag; cmd.abort.sqid = cpu_to_le16(nvmeq->qid); @@ -1886,10 +1880,9 @@ static int nvme_set_host_mem(struct nvme_dev *dev, u32 bits) { u32 host_mem_size = dev->host_mem_size >> NVME_CTRL_PAGE_SHIFT; u64 dma_addr = dev->host_mem_descs_dma; - struct nvme_command c; + struct nvme_command c = { }; int ret; - memset(&c, 0, sizeof(c)); c.features.opcode = nvme_admin_set_features; c.features.fid = cpu_to_le32(NVME_FEAT_HOST_MEM_BUF); c.features.dword11 = cpu_to_le32(bits); @@ -2263,9 +2256,8 @@ static int nvme_delete_queue(struct nvme_queue *nvmeq, u8 opcode) { struct request_queue *q = nvmeq->dev->ctrl.admin_q; struct request *req; - struct nvme_command cmd; + struct nvme_command cmd = { }; - memset(&cmd, 0, sizeof(cmd)); cmd.delete_queue.opcode = opcode; cmd.delete_queue.qid = cpu_to_le16(nvmeq->qid); -- cgit From cc72c4426764d1716839e9ec591ee8e161ed5cbc Mon Sep 17 00:00:00 2001 From: Chaitanya Kulkarni Date: Wed, 16 Jun 2021 15:15:52 -0700 Subject: nvme: remove zeroout memset call for struct Declare and initialize structure variables to zero values so that we can remove zeroout memset calls in the host/core.c. Signed-off-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) (limited to 'drivers/nvme/host') diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 177cae44b612..c7ef0b6684b5 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -721,9 +721,7 @@ EXPORT_SYMBOL_GPL(__nvme_check_ready); static int nvme_toggle_streams(struct nvme_ctrl *ctrl, bool enable) { - struct nvme_command c; - - memset(&c, 0, sizeof(c)); + struct nvme_command c = { }; c.directive.opcode = nvme_admin_directive_send; c.directive.nsid = cpu_to_le32(NVME_NSID_ALL); @@ -748,9 +746,8 @@ static int nvme_enable_streams(struct nvme_ctrl *ctrl) static int nvme_get_stream_params(struct nvme_ctrl *ctrl, struct streams_directive_params *s, u32 nsid) { - struct nvme_command c; + struct nvme_command c = { }; - memset(&c, 0, sizeof(c)); memset(s, 0, sizeof(*s)); c.directive.opcode = nvme_admin_directive_recv; @@ -1460,10 +1457,9 @@ static int nvme_features(struct nvme_ctrl *dev, u8 op, unsigned int fid, unsigned int dword11, void *buffer, size_t buflen, u32 *result) { union nvme_result res = { 0 }; - struct nvme_command c; + struct nvme_command c = { }; int ret; - memset(&c, 0, sizeof(c)); c.features.opcode = op; c.features.fid = cpu_to_le32(fid); c.features.dword11 = cpu_to_le32(dword11); @@ -1591,9 +1587,8 @@ int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo) static void nvme_init_integrity(struct gendisk *disk, u16 ms, u8 pi_type, u32 max_integrity_segments) { - struct blk_integrity integrity; + struct blk_integrity integrity = { }; - memset(&integrity, 0, sizeof(integrity)); switch (pi_type) { case NVME_NS_DPS_PI_TYPE3: integrity.profile = &t10_pi_type3_crc; @@ -1964,13 +1959,12 @@ static int nvme_send_ns_pr_command(struct nvme_ns *ns, struct nvme_command *c, static int nvme_pr_command(struct block_device *bdev, u32 cdw10, u64 key, u64 sa_key, u8 op) { - struct nvme_command c; + struct nvme_command c = { }; u8 data[16] = { 0, }; put_unaligned_le64(key, &data[0]); put_unaligned_le64(sa_key, &data[8]); - memset(&c, 0, sizeof(c)); c.common.opcode = op; c.common.cdw10 = cpu_to_le32(cdw10); @@ -2042,9 +2036,8 @@ int nvme_sec_submit(void *data, u16 spsp, u8 secp, void *buffer, size_t len, bool send) { struct nvme_ctrl *ctrl = data; - struct nvme_command cmd; + struct nvme_command cmd = { }; - memset(&cmd, 0, sizeof(cmd)); if (send) cmd.common.opcode = nvme_admin_security_send; else -- cgit