1. 15 Jun, 2021 2 commits
  2. 14 Jun, 2021 1 commit
  3. 09 Jun, 2021 6 commits
  4. 08 Jun, 2021 1 commit
    • Jens Axboe's avatar
      Merge tag 'nvme-5.14-2021-06-08' of git://git.infradead.org/nvme into for-5.14/drivers · 600abd34
      Jens Axboe authored
      Pull NVMe updates from Christoph:
      
      "nvme updates for Linux 5.14
      
       - improve the APST configuration algorithm (Alexey Bogoslavsky)
       - look for StorageD3Enable on companion ACPI device (Mario Limonciello)
       - allow selecting the network interface for TCP connections
         (Martin Belanger)
       - misc cleanups (Amit Engel, Chaitanya Kulkarni, Colin Ian King, me)"
      
      * tag 'nvme-5.14-2021-06-08' of git://git.infradead.org/nvme:
        nvmet: remove a superfluous variable
        nvmet: move ka_work initialization to nvmet_alloc_ctrl
        nvme: remove nvme_{get,put}_ns_from_disk
        nvme: split nvme_report_zones
        nvme: move the CSI sanity check into nvme_ns_report_zones
        nvme: add a sparse annotation to nvme_ns_head_ctrl_ioctl
        nvme: open code nvme_put_ns_from_disk in nvme_ns_head_ctrl_ioctl
        nvme: open code nvme_{get,put}_ns_from_disk in nvme_ns_head_ioctl
        nvme: open code nvme_put_ns_from_disk in nvme_ns_head_chr_ioctl
        nvme-fabrics: remove extra braces
        nvme-fabrics: remove an extra comment
        nvme-fabrics: remove extra new lines in the switch
        nvme-fabrics: fix the kerneldco comment for nvmf_log_connect_error()
        nvme-tcp: allow selecting the network interface for connections
        nvme-pci: look for StorageD3Enable on companion ACPI device instead
        nvme: extend and modify the APST configuration algorithm
        nvme: remove redundant initialization of variable ret
      600abd34
  5. 03 Jun, 2021 17 commits
    • Chaitanya Kulkarni's avatar
      nvmet: remove a superfluous variable · 346ac785
      Chaitanya Kulkarni authored
      Remove the superfluous variable "bdev" that is only used once in the
      nvmet_bdev_alloc_bip() and use req->ns->bdev that is used everywhere in
      the code to access the nvmet request's bdev.
      Signed-off-by: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      346ac785
    • Amit Engel's avatar
      nvmet: move ka_work initialization to nvmet_alloc_ctrl · f6e8bd59
      Amit Engel authored
      Initialize keep-alive work only once, as part of alloc_ctrl
      and not each time that nvmet_start_keep_alive_timer is being called
      Signed-off-by: default avatarAmit Engel <amit.engel@dell.com>
      Reviewed-by: default avatarHou Pu <houpu.main@gmail.com>
      f6e8bd59
    • Christoph Hellwig's avatar
      nvme: remove nvme_{get,put}_ns_from_disk · f1cf35e1
      Christoph Hellwig authored
      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: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarKeith Busch <kbusch@kernel.org>
      Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
      f1cf35e1
    • Christoph Hellwig's avatar
      nvme: split nvme_report_zones · 8b4fb0f9
      Christoph Hellwig authored
      Split multipath support out of nvme_report_zones into a separate helper
      and simplify the non-multipath version as a result.
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarKeith Busch <kbusch@kernel.org>
      Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
      8b4fb0f9
    • Christoph Hellwig's avatar
      nvme: move the CSI sanity check into nvme_ns_report_zones · d8ca66e8
      Christoph Hellwig authored
      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: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarKeith Busch <kbusch@kernel.org>
      Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
      d8ca66e8
    • Christoph Hellwig's avatar
      nvme: add a sparse annotation to nvme_ns_head_ctrl_ioctl · 85b790a7
      Christoph Hellwig authored
      Add the __releases annotation to tell sparse that nvme_ns_head_ctrl_ioctl
      is expected to unlock head->srcu.
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarKeith Busch <kbusch@kernel.org>
      Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
      85b790a7
    • Christoph Hellwig's avatar
      nvme: open code nvme_put_ns_from_disk in nvme_ns_head_ctrl_ioctl · 3e7d1a55
      Christoph Hellwig authored
      nvme_ns_head_ctrl_ioctl is always used on multipath nodes, so just call
      srcu_read_unlock directly.
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarKeith Busch <kbusch@kernel.org>
      Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
      3e7d1a55
    • Christoph Hellwig's avatar
      nvme: open code nvme_{get,put}_ns_from_disk in nvme_ns_head_ioctl · 86b4284d
      Christoph Hellwig authored
      nvme_ns_head_ioctl is always used on multipath nodes, no need to
      deal with the de-multiplexers.
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarKeith Busch <kbusch@kernel.org>
      Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
      86b4284d
    • Christoph Hellwig's avatar
      nvme: open code nvme_put_ns_from_disk in nvme_ns_head_chr_ioctl · f423c85c
      Christoph Hellwig authored
      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: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarKeith Busch <kbusch@kernel.org>
      Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
      f423c85c
    • Chaitanya Kulkarni's avatar
      nvme-fabrics: remove extra braces · 97ba6931
      Chaitanya Kulkarni authored
      No need to use the braces around ~ operator.
      
      No functionality change in this patch.
      Signed-off-by: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      97ba6931
    • Chaitanya Kulkarni's avatar
      nvme-fabrics: remove an extra comment · 6f860c92
      Chaitanya Kulkarni authored
      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: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      6f860c92
    • Chaitanya Kulkarni's avatar
      nvme-fabrics: remove extra new lines in the switch · 63d20f54
      Chaitanya Kulkarni authored
      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: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      63d20f54
    • Chaitanya Kulkarni's avatar
      nvme-fabrics: fix the kerneldco comment for nvmf_log_connect_error() · 25e1de8c
      Chaitanya Kulkarni authored
      Fix the comment style that matches existing code.
      
      No functionality change in this patch.
      Signed-off-by: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      25e1de8c
    • Martin Belanger's avatar
      nvme-tcp: allow selecting the network interface for connections · 3ede8f72
      Martin Belanger authored
      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: <LOOPBACK,UP,LOWER_UP> 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: <BROADCAST,MULTICAST,UP,LOWER_UP> 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: <BROADCAST,MULTICAST,UP,LOWER_UP> 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: <BROADCAST,MULTICAST,UP,LOWER_UP> 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: default avatarMartin Belanger <martin.belanger@dell.com>
      Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      3ede8f72
    • Mario Limonciello's avatar
      nvme-pci: look for StorageD3Enable on companion ACPI device instead · e21e0243
      Mario Limonciello authored
      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: df4f9bc4 ("nvme-pci: add support for ACPI StorageD3Enable property")
      Suggested-by: default avatarLiang Prike <Prike.Liang@amd.com>
      Acked-by: default avatarRaul E Rangel <rrangel@chromium.org>
      Signed-off-by: default avatarMario Limonciello <mario.limonciello@amd.com>
      Reviewed-by: default avatarDavid E. Box <david.e.box@linux.intel.com>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      e21e0243
    • Alexey Bogoslavsky's avatar
      nvme: extend and modify the APST configuration algorithm · ebd8a93a
      Alexey Bogoslavsky authored
      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: default avatarAlexey Bogoslavsky <alexey.bogoslavsky@wdc.com>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      ebd8a93a
    • Colin Ian King's avatar
      nvme: remove redundant initialization of variable ret · 13ce7e62
      Colin Ian King authored
      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: default avatarColin Ian King <colin.king@canonical.com>
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      13ce7e62
  6. 24 May, 2021 13 commits