Copy Offload in NVMe Fabrics with P2P PCI Memory
Subsystems (11)
Logan Gunthorpe
maintainerv9 cover Copy Offload in NVMe Fabrics with P2P PCI Memory
This is v9 for the p2pdma patch set. It has some substantial changes from the previous version. Essentially, the last patch has changed based on information from Sagi that the P2P memory can be assigned per namespace instead of globally. To that end, I've added a radix tree that stores the p2p devices to use for each namespace and a lookup will be done on that before allocating the memory.
This has some knock on effects of simplifying the requirements from the p2pdma code and so we drop all the client list code seeing we don't need to worry about selecting a P2P device that works with every namespace at once; only each namespace independantly. Thus, Patch 1, 6 and 13 have significant changes. However, I think the removal of the client list code will be generally appreciated based on the feedback I've gotten from Christian.
As usual, a git repo of the branch is available here:
https://github.com/sbates130272/linux-p2pmem pci-p2p-v9
Thanks,
Logan
--
Changes in v9:
* Rebased on v4.19-rc6
* Droped pci_p2pdma_add_client(), pci_p2pdma_remove_client() and pci_p2pdma_client_list_free(), and reworked pci_p2pdma_distance() and pci_p2pmem_find() to take simple arrays instead of list_heads.
* Updated the documentation to match
* Reworked the nvmet code to have a configfs attribute per namespace instead of per port. Then each namespace has it's own P2P device stored in a radix-tree in the controller (it can't be stored in the nvme_ns structure seeing it needs to be unique per controller).
* Dropped the 'sq' argument in nvmet_req_alloc_sgl() seeing I've noticed it is now available in the nvmet_req structure.
* Collected Keith's reviewed-by tags (however, I have not applied his tag to the last patch seeing I think it has changed too much since his review).
Changes in v8:
* Added a couple of comments to address Bart's feedback and removed the bogus call to percpu_ref_switch_to_atomic_sync()
Changes in v7:
* Rebased on v4.19-rc5
* Fixed up commit message of patch 7 that was no longer accurate. (as pointed out by Jens)
* Change the configfs to not use "auto" or "none" and instead just use a 0/1/<pci_dev> (or boolean). This matches the existing nvme-target configfs booleans. (Per Bjorn)
* A handful of other minor changes and edits that were noticed by Bjorn
* Collected Acks from Bjorn
Changes in v6:
* Rebased on v4.19-rc3
* Remove the changes to the common submit_bio() path and instead set REQ_NOMERGE in the NVME target driver, when appropriate. Per discussions with Jens and Christoph.
* Some minor grammar changes in the documentation as spotted by Randy.
Changes in v5:
* Rebased on v4.19-rc1
* Drop changing ACS settings in this patchset. Now, the code will only allow P2P transactions between devices whos downstream ports do not restrict P2P TLPs.
* Drop the REQ_PCI_P2PDMA block flag and instead use is_pci_p2pdma_page() to tell if a request is P2P or not. In that case we check for queue support and enforce using REQ_NOMERGE. Per feedback from Christoph.
* Drop the pci_p2pdma_unmap_sg() function as it was empty and only there for symmetry and compatibility with dma_unmap_sg. Per feedback from Christoph.
* Split off the logic to handle enabling P2P in NVMe fabrics' configfs into specific helpers in the p2pdma code. Per feedback from Christoph.
* A number of other minor cleanups and fixes as pointed out by Christoph and others.
Changes in v4:
* Change the original upstream_bridges_match() function to upstream_bridge_distance() which calculates the distance between two devices as long as they are behind the same root port. This should address Bjorn's concerns that the code was to focused on being behind a single switch.
* The disable ACS function now disables ACS for all bridge ports instead of switch ports (ie. those that had two upstream_bridge ports).
* Change the pci_p2pmem_alloc_sgl() and pci_p2pmem_free_sgl() API to be more like sgl_alloc() in that the alloc function returns the allocated scatterlist and nents is not required bythe free function.
* Moved the new documentation into the driver-api tree as requested by Jonathan
* Add SGL alloc and free helpers in the nvmet code so that the individual drivers can share the code that allocates P2P memory. As requested by Christoph.
* Cleanup the nvmet_p2pmem_store() function as Christoph thought my first attempt was ugly.
* Numerous commit message and comment fix-ups
Changes in v3:
* Many more fixes and minor cleanups that were spotted by Bjorn
* Additional explanation of the ACS change in both the commit message and Kconfig doc. Also, the code that disables the ACS bits is surrounded explicitly by an #ifdef
* Removed the flag we added to rdma_rw_ctx() in favour of using is_pci_p2pdma_page(), as suggested by Sagi.
* Adjust pci_p2pmem_find() so that it prefers P2P providers that are closest to (or the same as) the clients using them. In cases of ties, the provider is randomly chosen.
* Modify the NVMe Target code so that the PCI device name of the provider may be explicitly specified, bypassing the logic in pci_p2pmem_find(). (Note: it's still enforced that the provider must be behind the same switch as the clients).
* As requested by Bjorn, added documentation for driver writers.
Changes in v2:
* Renamed everything to 'p2pdma' per the suggestion from Bjorn as well as a bunch of cleanup and spelling fixes he pointed out in the last series.
* To address Alex's ACS concerns, we change to a simpler method of just disabling ACS behind switches for any kernel that has CONFIG_PCI_P2PDMA.
* We also reject using devices that employ 'dma_virt_ops' which should fairly simply handle Jason's concerns that this work might break with the HFI, QIB and rxe drivers that use the virtual ops to implement their own special DMA operations.
--
This is a continuation of our work to enable using Peer-to-Peer PCI memory in the kernel with initial support for the NVMe fabrics target subsystem. Many thanks go to Christoph Hellwig who provided valuable feedback to get these patches to where they are today.
The concept here is to use memory that's exposed on a PCI BAR as data buffers in the NVMe target code such that data can be transferred from an RDMA NIC to the special memory and then directly to an NVMe device avoiding system memory entirely. The upside of this is better QoS for applications running on the CPU utilizing memory and lower PCI bandwidth required to the CPU (such that systems could be designed with fewer lanes connected to the CPU).
Due to these trade-offs we've designed the system to only enable using the PCI memory in cases where the NIC, NVMe devices and memory are all behind the same PCI switch hierarchy. This will mean many setups that could likely work well will not be supported so that we can be more confident it will work and not place any responsibility on the user to understand their topology. (We chose to go this route based on feedback we received at the last LSF). Future work may enable these transfers using a white list of known good root complexes. However, at this time, there is no reliable way to ensure that Peer-to-Peer transactions are permitted between PCI Root Ports.
For PCI P2P DMA transfers to work in this situation the ACS bits must be disabled on the downstream ports (DSPs) for all devices involved in the transfer. This can be done using the "disable_acs_redir" PCI kernel command line option which was introduced in v4.19.
In order to enable PCI P2P functionality, we introduce a few new PCI functions such that a driver can register P2P memory with the system. Struct pages are created for this memory using devm_memremap_pages() and the PCI bus offset is stored in the corresponding pagemap structure.
Another set of functions allow a client driver to create a list of client devices that will be used in a given P2P transactions and then use that list to find any P2P memory that is supported by all the client devices.
In the block layer, we also introduce a flag for a request queue to indicate a given queue supports targeting P2P memory. The driver submitting bios must ensure that the queue supports P2P before attempting to submit BIOs backed by P2P memory. Also, P2P requests are marked to not be merged seeing a non-homogenous request would complicate the DMA mapping requirements.
In the PCI NVMe driver, we modify the existing CMB support to utilize the new PCI P2P memory infrastructure and also add support for P2P memory in its request queue. When a P2P request is received it uses the pci_p2pmem_map_sg() function which applies the necessary transformation to get the corrent pci_bus_addr_t for the DMA transactions.
In the RDMA core, we also adjust rdma_rw_ctx_init() and rdma_rw_ctx_destroy() to take a flags argument which indicates whether to use the PCI P2P mapping functions or not. To avoid odd RDMA devices that don't use the proper DMA infrastructure this code rejects using any device that employs the virt_dma_ops implementation.
Finally, in the NVMe fabrics target port we introduce a new configuration attribute: 'p2pmem'. When set to a true boolean, the port will attempt to find P2P memory supported by the RDMA NIC and all namespaces. It may also be set to a PCI device name to select a specific P2P memory to use. If supported memory is found, it will be used in all IO transfers. And if a port is using P2P memory, adding new namespaces that are not supported by that memory will fail.
These patches have been tested on a number of Intel based systems and for a variety of RDMA NICs (Mellanox, Broadcomm, Chelsio) and NVMe SSDs (Intel, Seagate, Samsung) and p2pdma devices (Eideticom, Microsemi, Chelsio and Everspin) using switches from both Microsemi and Broadcomm.
--
Logan Gunthorpe (13):
PCI/P2PDMA: Support peer-to-peer memory
PCI/P2PDMA: Add sysfs group to display p2pmem stats PCI/P2PDMA: Add PCI p2pmem DMA mappings to adjust the bus offset PCI/P2PDMA: Introduce configfs/sysfs enable attribute helpers
docs-rst: Add a new directory for PCI documentation PCI/P2PDMA: Add P2P DMA driver writer's documentation
block: Add PCI P2P flag for request queue and check support for
requests
IB/core: Ensure we map P2P memory correctly in
rdma_rw_ctx_[init|destroy]()
nvme-pci: Use PCI p2pmem subsystem to manage the CMB
nvme-pci: Add support for P2P memory in requests
nvme-pci: Add a quirk for a pseudo CMB
nvmet: Introduce helper functions to allocate and free request SGLs
nvmet: Optionally use PCI P2P memory
Documentation/ABI/testing/sysfs-bus-pci | 24 +
Documentation/driver-api/index.rst | 2 +-
Documentation/driver-api/pci/index.rst | 21 +
Documentation/driver-api/pci/p2pdma.rst | 145 ++++
Documentation/driver-api/{ => pci}/pci.rst | 0
drivers/infiniband/core/rw.c | 11 +-
drivers/nvme/host/core.c | 4 +
drivers/nvme/host/nvme.h | 8 +
drivers/nvme/host/pci.c | 121 +--
drivers/nvme/target/configfs.c | 45 ++
drivers/nvme/target/core.c | 180 +++++
drivers/nvme/target/io-cmd-bdev.c | 3 +
drivers/nvme/target/nvmet.h | 17 +
drivers/nvme/target/rdma.c | 22 +-
drivers/pci/Kconfig | 17 +
drivers/pci/Makefile | 1 +
drivers/pci/p2pdma.c | 809 +++++++++++++++++++++
include/linux/blkdev.h | 3 +
include/linux/memremap.h | 6 +
include/linux/mm.h | 18 +
include/linux/pci-p2pdma.h | 114 +++
include/linux/pci.h | 4 +
22 files changed, 1521 insertions(+), 54 deletions(-)
create mode 100644 Documentation/driver-api/pci/index.rst
create mode 100644 Documentation/driver-api/pci/p2pdma.rst
rename Documentation/driver-api/{ => pci}/pci.rst (100%)
create mode 100644 drivers/pci/p2pdma.c
create mode 100644 include/linux/pci-p2pdma.h--
2.19.0
Logan Gunthorpe
maintainerPATCH v9 1/13 PCI/P2PDMA: Support peer-to-peer memory
Some PCI devices may have memory mapped in a BAR space that's intended for use in peer-to-peer transactions. In order to enable such transactions the memory must be registered with ZONE_DEVICE pages so it can be used by DMA interfaces in existing drivers.
Add an interface for other subsystems to find and allocate chunks of P2P memory as necessary to facilitate transfers between two PCI peers:
struct pci_dev *pci_p2pmem_find[_many]();
int pci_p2pdma_distance[_many]();
void *pci_alloc_p2pmem();
The new interface requires a driver to collect a list of client devices involved in the transaction then call pci_p2pmem_find() to obtain any suitable P2P memory. Alternatively, if the caller knows a device which provides P2P memory, they can use pci_p2pdma_distance() to determine if it is usable. With a suitable p2pmem device, memory can then be allocated with pci_alloc_p2pmem() for use in DMA transactions.
Depending on hardware, using peer-to-peer memory may reduce the bandwidth of the transfer but can significantly reduce pressure on system memory. This may be desirable in many cases: for example a system could be designed with a small CPU connected to a PCIe switch by a small number of lanes which would maximize the number of lanes available to connect to NVMe devices.
The code is designed to only utilize the p2pmem device if all the devices involved in a transfer are behind the same PCI bridge. This is because we have no way of knowing whether peer-to-peer routing between PCIe Root Ports is supported (PCIe r4.0, sec 1.3.1). Additionally, the benefits of P2P transfers that go through the RC is limited to only reducing DRAM usage and, in some cases, coding convenience. The PCI-SIG may be exploring adding a new capability bit to advertise whether this is possible for future hardware.
This commit includes significant rework and feedback from Christoph Hellwig.
--- drivers/pci/Kconfig | 17 + drivers/pci/Makefile | 1 + drivers/pci/p2pdma.c | 630 +++++++++++++++++++++++++++++++++++++ include/linux/memremap.h | 5 + include/linux/mm.h | 18 ++ include/linux/pci-p2pdma.h | 92 ++++++ include/linux/pci.h | 4 + 7 files changed, 767 insertions(+) create mode 100644 drivers/pci/p2pdma.c create mode 100644 include/linux/pci-p2pdma.h
drivers/pci/Kconfig +17 -0
@@ -132,6 +132,23 @@ config PCI_PASID If unsure, say N. +config PCI_P2PDMA + bool "PCI peer-to-peer transfer support" + depends on PCI && ZONE_DEVICE + select GENERIC_ALLOCATOR + help + Enableѕ drivers to do PCI peer-to-peer transactions to and from + BARs that are exposed in other devices that are the part of + the hierarchy where peer-to-peer DMA is guaranteed by the PCI + specification to work (ie. anything below a single PCI bridge). + + Many PCIe root complexes do not support P2P transactions and + it's hard to tell which support it at all, so at this time, + P2P DMA transations must be between devices behind the same root + port. + + If unsure, say N. + config PCI_LABEL def_bool y if (DMI || ACPI) depends on PCI
drivers/pci/Makefile +1 -0
@@ -26,6 +26,7 @@ obj-$(CONFIG_PCI_SYSCALL) += syscall.o obj-$(CONFIG_PCI_STUB) += pci-stub.o obj-$(CONFIG_PCI_PF_STUB) += pci-pf-stub.o obj-$(CONFIG_PCI_ECAM) += ecam.o +obj-$(CONFIG_PCI_P2PDMA) += p2pdma.o obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o # Endpoint library must be initialized before its users
drivers/pci/p2pdma.c new file +630 -0
630 lines not on this page · raw message
include/linux/memremap.h +5 -0
@@ -53,11 +53,16 @@ struct vmem_altmap {* wakeup event whenever a page is unpinned and becomes idle. This * wakeup is used to coordinate physical address space management (ex: * fs truncate/hole punch) vs pinned pages (ex: device dma). +* +* MEMORY_DEVICE_PCI_P2PDMA: +* Device memory residing in a PCI BAR intended for use with Peer-to-Peer +* transactions. */ enum memory_type {MEMORY_DEVICE_PRIVATE =1 ,MEMORY_DEVICE_PUBLIC ,MEMORY_DEVICE_FS_DAX , +MEMORY_DEVICE_PCI_P2PDMA , };/*
include/linux/mm.h +18 -0
@@ -890,6 +890,19 @@ static inline bool is_device_public_page(const struct page *page) page->pgmap->type == MEMORY_DEVICE_PUBLIC; } +#ifdef CONFIG_PCI_P2PDMA +static inline bool is_pci_p2pdma_page (const struct page *page) +{ +return is_zone_device_page (page) && + page->pgmap->type ==MEMORY_DEVICE_PCI_P2PDMA ; +} +#else /* CONFIG_PCI_P2PDMA */ +static inline bool is_pci_p2pdma_page (const struct page *page) +{ +return false; +} +#endif /* CONFIG_PCI_P2PDMA */ +#else /* CONFIG_DEV_PAGEMAP_OPS */ static inline void dev_pagemap_get_ops (void ) {
@@ -913,6 +926,11 @@ static inline bool is_device_public_page(const struct page *page) {return false; } + +static inline bool is_pci_p2pdma_page (const struct page *page) +{ +return false; +}#endif /* CONFIG_DEV_PAGEMAP_OPS */ static inline void get_page (struct page *page)
include/linux/pci-p2pdma.h new file +92 -0
92 lines not on this page · raw message
include/linux/pci.h +4 -0
@@ -281,6 +281,7 @@ struct pcie_link_state;struct pci_vpd ;struct pci_sriov ;struct pci_ats ; +struct pci_p2pdma ;/* The pci_dev structure describes PCI devices */ struct pci_dev {
@@ -438,6 +439,9 @@ struct pci_dev {#endif #ifdef CONFIG_PCI_PASID u16 pasid_features; +#endif +#ifdef CONFIG_PCI_P2PDMA +struct pci_p2pdma *p2pdma;#endif phys_addr_t rom;/* Physical address if not from BAR */ size_t romlen;/* Length if not from BAR */
Signature · 1 line
2.19.0
Logan Gunthorpe
maintainerPATCH v9 2/13 PCI/P2PDMA: Add sysfs group to display p2pmem stats
In reply to Logan GunthorpeAdd a sysfs group to display statistics about P2P memory that is registered in each PCI device.
Attributes in the group display the total amount of P2P memory, the amount available and whether it is published or not.
--- Documentation/ABI/testing/sysfs-bus-pci | 24 +++++++++++ drivers/pci/p2pdma.c | 54 +++++++++++++++++++++++++ 2 files changed, 78 insertions(+)
Documentation/ABI/testing/sysfs-bus-pci +24 -0
@@ -323,3 +323,27 @@ Description: This is similar to /sys/bus/pci/drivers_autoprobe, but affects only the VFs associated with a specific PF. + +What: /sys/bus/pci/devices/.../p2pmem/size +Date: November 2017 +Contact: Logan Gunthorpe <[email protected]> +Description: + If the device has any Peer-to-Peer memory registered, this + file contains the total amount of memory that the device + provides (in decimal). + +What: /sys/bus/pci/devices/.../p2pmem/available +Date: November 2017 +Contact: Logan Gunthorpe <[email protected]> +Description: + If the device has any Peer-to-Peer memory registered, this + file contains the amount of memory that has not been + allocated (in decimal). + +What: /sys/bus/pci/devices/.../p2pmem/published +Date: November 2017 +Contact: Logan Gunthorpe <[email protected]> +Description: + If the device has any Peer-to-Peer memory registered, this + file contains a '1' if the memory has been published for + use outside the driver that owns the device.
drivers/pci/p2pdma.c +54 -0
75 lines not on this page · raw message
Signature · 1 line
2.19.0
Re: [PATCH v9 02/13] PCI/P2PDMA: Add sysfs group to display p2pmem stats
Looks good,
Logan Gunthorpe
maintainerPATCH v9 3/13 PCI/P2PDMA: Add PCI p2pmem DMA mappings to adjust the bus offset
In reply to Logan GunthorpeThe DMA address used when mapping PCI P2P memory must be the PCI bus address. Thus, introduce pci_p2pmem_map_sg() to map the correct addresses when using P2P memory.
Memory mapped in this way does not need to be unmapped and thus if we provided pci_p2pmem_unmap_sg() it would be empty. This breaks the expected balance between map/unmap but was left out as an empty function doesn't really provide any benefit. In the future, if this call becomes necessary it can be added without much difficulty.
For this, we assume that an SGL passed to these functions contain all P2P memory or no P2P memory.
--- drivers/pci/p2pdma.c | 43 ++++++++++++++++++++++++++++++++++++++ include/linux/memremap.h | 1 + include/linux/pci-p2pdma.h | 7 +++++++ 3 files changed, 51 insertions(+)
drivers/pci/p2pdma.c +43 -0
@@ -194,6 +194,8 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size, pgmap->res.flags =pci_resource_flags (pdev, bar); pgmap->ref = &pdev->p2pdma->devmap_ref; pgmap->type =MEMORY_DEVICE_PCI_P2PDMA ; + pgmap->pci_p2pdma_bus_offset =pci_bus_address (pdev, bar) - +pci_resource_start (pdev, bar); addr =devm_memremap_pages (&pdev->dev, pgmap);if (IS_ERR (addr)) {
@@ -682,3 +684,44 @@ void pci_p2pmem_publish(struct pci_dev *pdev, bool publish) pdev->p2pdma->p2pmem_published = publish; }EXPORT_SYMBOL_GPL (pci_p2pmem_publish); + +/** +* pci_p2pdma_map_sg - map a PCI peer-to-peer scatterlist for DMA +* @dev: device doing the DMA request +* @sg: scatter list to map +* @nents: elements in the scatterlist +* @dir: DMA direction +* +* Scatterlists mapped with this function should not be unmapped in any way. +* +* Returns the number of SG entries mapped or 0 on error. +*/ +int pci_p2pdma_map_sg (struct device *dev,struct scatterlist *sg,int nents, +enum dma_data_direction dir) +{ +struct dev_pagemap *pgmap; +struct scatterlist *s; +phys_addr_t paddr; +int i; + +/* +* p2pdma mappings are not compatible with devices that use +* dma_virt_ops. If the upper layers do the right thing +* this should never happen because it will be prevented +* by the check in pci_p2pdma_add_client() +*/ +if (WARN_ON_ONCE (IS_ENABLED (CONFIG_DMA_VIRT_OPS ) && + dev->dma_ops == &dma_virt_ops)) +return 0 ; + +for_each_sg (sg, s, nents, i) { + pgmap =sg_page (s)->pgmap; + paddr =sg_phys (s); + + s->dma_address = paddr - pgmap->pci_p2pdma_bus_offset; +sg_dma_len (s) = s->length; + } + +return nents; +} +EXPORT_SYMBOL_GPL (pci_p2pdma_map_sg);
include/linux/memremap.h +1 -0
@@ -125,6 +125,7 @@ struct dev_pagemap {struct device *dev;void *data;enum memory_type type; +u64 pci_p2pdma_bus_offset; };#ifdef CONFIG_ZONE_DEVICE
include/linux/pci-p2pdma.h +7 -0
@@ -30,6 +30,8 @@ struct scatterlist *pci_p2pmem_alloc_sgl(struct pci_dev *pdev, unsigned int *nents, u32 length);void pci_p2pmem_free_sgl (struct pci_dev *pdev,struct scatterlist *sgl);void pci_p2pmem_publish (struct pci_dev *pdev,bool publish); +int pci_p2pdma_map_sg (struct device *dev,struct scatterlist *sg,int nents, +enum dma_data_direction dir);#else /* CONFIG_PCI_P2PDMA */ static inline int pci_p2pdma_add_resource (struct pci_dev *pdev,int bar,size_t size,u64 offset)
@@ -75,6 +77,11 @@ static inline void pci_p2pmem_free_sgl(struct pci_dev *pdev,static inline void pci_p2pmem_publish (struct pci_dev *pdev,bool publish) { } +static inline int pci_p2pdma_map_sg (struct device *dev, +struct scatterlist *sg,int nents,enum dma_data_direction dir) +{ +return 0 ; +}#endif /* CONFIG_PCI_P2PDMA */
Signature · 1 line
2.19.0
Re: [PATCH v9 03/13] PCI/P2PDMA: Add PCI p2pmem DMA mappings to adjust the bus offset
Looks good,
Logan Gunthorpe
maintainerPATCH v9 4/13 PCI/P2PDMA: Introduce configfs/sysfs enable attribute helpers
In reply to Logan GunthorpeUsers of the P2PDMA infrastructure will typically need a way for the user to tell the kernel to use P2P resources. Typically this will be a simple on/off boolean operation but sometimes it may be desirable for the user to specify the exact device to use for the P2P operation.
Add new helpers for attributes which take a boolean or a PCI device. Any boolean as accepted by strtobool() turn P2P on or off (such as 'y', 'n', '1', '0', etc). Specifying a full PCI device name/BDF will select the specific device.
--- drivers/pci/p2pdma.c | 82 ++++++++++++++++++++++++++++++++++++++ include/linux/pci-p2pdma.h | 15 +++++++ 2 files changed, 97 insertions(+)
drivers/pci/p2pdma.c +82 -0
@@ -8,6 +8,8 @@* Copyright (c) 2018, Eideticom Inc. */ +#define pr_fmt (fmt) "pci-p2pdma: " fmt +#include <linux/ctype.h> #include <linux/pci-p2pdma.h> #include <linux/module.h> #include <linux/slab.h>
@@ -725,3 +727,83 @@ int pci_p2pdma_map_sg(struct device *dev, struct scatterlist *sg, int nents,return nents; }EXPORT_SYMBOL_GPL (pci_p2pdma_map_sg); + +/** +* pci_p2pdma_enable_store - parse a configfs/sysfs attribute store +* to enable p2pdma +* @page: contents of the value to be stored +* @p2p_dev: returns the PCI device that was selected to be used +* (if one was specified in the stored value) +* @use_p2pdma: returns whether to enable p2pdma or not +* +* Parses an attribute value to decide whether to enable p2pdma. +* The value can select a PCI device (using it's full BDF device +* name) or a boolean (in any format strtobool() accepts). A false +* value disables p2pdma, a true value expects the caller +* to automatically find a compatible device and specifying a PCI device +* expects the caller to use the specific provider. +* +* pci_p2pdma_enable_show() should be used as the show operation for +* the attribute. +* +* Returns 0 on success +*/ +int pci_p2pdma_enable_store (const char *page,struct pci_dev **p2p_dev, +bool *use_p2pdma) +{ +struct device *dev; + + dev =bus_find_device_by_name (&pci_bus_type,NULL , page); +if (dev) { + *use_p2pdma = true; + *p2p_dev =to_pci_dev (dev); + +if (!pci_has_p2pmem (*p2p_dev)) { +pci_err (*p2p_dev, +"PCI device has no peer-to-peer memory: %s\n" , + page); +pci_dev_put (*p2p_dev); +return -ENODEV ; + } + +return 0 ; + }else if ((page[0 ] =='0' || page[0 ] =='1' ) && !iscntrl (page[1 ])) { +/* +* If the user enters a PCI device that doesn't exist +* like "0000:01:00.1", we don't want strtobool to think +* it's a '0' when it's clearly not what the user wanted. +* So we require 0's and 1's to be exactly one character. +*/ + }else if (!strtobool (page, use_p2pdma)) { +return 0 ; + } + +pr_err ("No such PCI device: %.*s\n" , (int )strcspn (page,"\n" ), page); +return -ENODEV ; +} +EXPORT_SYMBOL_GPL (pci_p2pdma_enable_store); + +/** +* pci_p2pdma_enable_show - show a configfs/sysfs attribute indicating +* whether p2pdma is enabled +* @page: contents of the stored value +* @p2p_dev: the selected p2p device (NULL if no device is selected) +* @use_p2pdma: whether p2pdme has been enabled +* +* Attributes that use pci_p2pdma_enable_store() should use this function +* to show the value of the attribute. +* +* Returns 0 on success +*/ +ssize_t pci_p2pdma_enable_show (char *page,struct pci_dev *p2p_dev, +bool use_p2pdma) +{ +if (!use_p2pdma) +return sprintf (page,"0\n" ); + +if (!p2p_dev) +return sprintf (page,"1\n" ); + +return sprintf (page,"%s\n" ,pci_name (p2p_dev)); +} +EXPORT_SYMBOL_GPL (pci_p2pdma_enable_show);
include/linux/pci-p2pdma.h +15 -0
@@ -32,6 +32,10 @@ void pci_p2pmem_free_sgl(struct pci_dev *pdev, struct scatterlist *sgl);void pci_p2pmem_publish (struct pci_dev *pdev,bool publish);int pci_p2pdma_map_sg (struct device *dev,struct scatterlist *sg,int nents,enum dma_data_direction dir); +int pci_p2pdma_enable_store (const char *page,struct pci_dev **p2p_dev, +bool *use_p2pdma); +ssize_t pci_p2pdma_enable_show (char *page,struct pci_dev *p2p_dev, +bool use_p2pdma);#else /* CONFIG_PCI_P2PDMA */ static inline int pci_p2pdma_add_resource (struct pci_dev *pdev,int bar,size_t size,u64 offset)
@@ -82,6 +86,17 @@ static inline int pci_p2pdma_map_sg(struct device *dev, {return 0 ; } +static inline int pci_p2pdma_enable_store (const char *page, +struct pci_dev **p2p_dev,bool *use_p2pdma) +{ + *use_p2pdma = false; +return 0 ; +} +static inline ssize_t pci_p2pdma_enable_show (char *page, +struct pci_dev *p2p_dev,bool use_p2pdma) +{ +return sprintf (page,"none\n" ); +}#endif /* CONFIG_PCI_P2PDMA */
Signature · 1 line
2.19.0
Re: [PATCH v9 04/13] PCI/P2PDMA: Introduce configfs/sysfs enable attribute helpers
Looks good,
Logan Gunthorpe
maintainerPATCH v9 5/13 docs-rst: Add a new directory for PCI documentation
In reply to Logan GunthorpeAdd a new directory in the driver API guide for PCI specific documentation.
This is in preparation for adding a new PCI P2P DMA driver writers guide which will go in this directory.
---
Documentation/driver-api/index.rst | 2 +-
Documentation/driver-api/pci/index.rst | 20 ++++++++++++++++++++
Documentation/driver-api/{ => pci}/pci.rst | 0
3 files changed, 21 insertions(+), 1 deletion(-)
create mode 100644 Documentation/driver-api/pci/index.rst
rename Documentation/driver-api/{ => pci}/pci.rst (100%)Documentation/driver-api/index.rst +1 -1
@@ -29,7 +29,7 @@ available subsections can be seen below. iio/index input usb/index - pci + pci/index spi i2c hsi
Documentation/driver-api/pci/index.rst new file +20 -0
@@ -0,0 +1,20 @@ +.. SPDX-License-Identifier: GPL-2.0 +============================================ +The Linux PCI driver implementer's API guide +============================================ + +.. class:: toc-title + + Table of contents + +.. toctree:: + :maxdepth: 2 + + pci + +.. only:: subproject and html + + Indices + ======= + + * :ref:`genindex`
Documentation/driver-api/pci/pci.rst renamed from Documentation/driver-api/pci.rst +0 -0
Signature · 1 line
2.19.0
Logan Gunthorpe
maintainerPATCH v9 6/13 PCI/P2PDMA: Add P2P DMA driver writer's documentation
In reply to Logan GunthorpeAdd a restructured text file describing how to write drivers with support for P2P DMA transactions. The document describes how to use the APIs that were added in the previous few commits.
Also adds an index for the PCI documentation tree even though this is the only PCI document that has been converted to restructured text at this time.
--- Documentation/driver-api/pci/index.rst | 1 + Documentation/driver-api/pci/p2pdma.rst | 145 ++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 Documentation/driver-api/pci/p2pdma.rst
Documentation/driver-api/pci/index.rst +1 -0
@@ -11,6 +11,7 @@ The Linux PCI driver implementer's API guide :maxdepth: 2 pci + p2pdma .. only:: subproject and html
Documentation/driver-api/pci/p2pdma.rst new file +145 -0
145 lines not on this page · raw message
Signature · 1 line
2.19.0
Logan Gunthorpe
maintainerPATCH v9 7/13 block: Add PCI P2P flag for request queue and check support for requests
In reply to Logan GunthorpeQUEUE_FLAG_PCI_P2P is introduced meaning a driver's request queue supports targeting P2P memory. This will be used by P2P providers and orchestrators (in subsequent patches) to ensure block devices can support P2P memory before submitting P2P backed pages to submit_bio().
--- include/linux/blkdev.h | 3 +++ 1 file changed, 3 insertions(+)
include/linux/blkdev.h +3 -0
@@ -699,6 +699,7 @@ struct request_queue {#define QUEUE_FLAG_SCSI_PASSTHROUGH 27/* queue supports SCSI commands */ #define QUEUE_FLAG_QUIESCED 28/* queue has been quiesced */ #define QUEUE_FLAG_PREEMPT_ONLY 29/* only process REQ_PREEMPT requests */ +#define QUEUE_FLAG_PCI_P2PDMA 30/* device supports pci p2p requests */ #define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \ (1 << QUEUE_FLAG_SAME_COMP) | \
@@ -731,6 +732,8 @@ bool blk_queue_flag_test_and_clear(unsigned int flag, struct request_queue *q);#define blk_queue_dax (q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)#define blk_queue_scsi_passthrough (q) \ test_bit(QUEUE_FLAG_SCSI_PASSTHROUGH, &(q)->queue_flags) +#define blk_queue_pci_p2pdma (q) \ + test_bit(QUEUE_FLAG_PCI_P2PDMA, &(q)->queue_flags)#define blk_noretry_request (rq) \ ((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
Signature · 1 line
2.19.0
Re: [PATCH v9 07/13] block: Add PCI P2P flag for request queue and check support for requests
Quoted 6 lines · Logan Gunthorpe
QUEUE_FLAG_PCI_P2Pis introduced meaning a driver's request queue supports targeting P2P memory. This will be used by P2P providers and orchestrators (in subsequent patches) to ensure block devices can support P2P memory before submitting P2P backed pages tosubmit_bio().Signed-off-by Logan Gunthorpe <[email protected]>
Jens Axboe
maintainerRe: [PATCH v9 07/13] block: Add PCI P2P flag for request queue and check support for requests
In reply to Logan GunthorpeQuoted 4 lines · Logan Gunthorpe
QUEUE_FLAG_PCI_P2Pis introduced meaning a driver's request queue supports targeting P2P memory. This will be used by P2P providers and orchestrators (in subsequent patches) to ensure block devices can support P2P memory before submitting P2P backed pages tosubmit_bio().
Nit pick, but the subject line still says that it checks support for requests. This patch just adds the ability to flag support in the queue.
Signature · 1 line
Jens Axboe
Bjorn Helgaas
Reviewed-by Christoph HellwigRe: [PATCH v9 07/13] block: Add PCI P2P flag for request queue and check support for requests
Quoted 9 lines · Jens Axboe
Quoted 4 lines · Logan Gunthorpe
QUEUE_FLAG_PCI_P2Pis introduced meaning a driver's request queue supports targeting P2P memory. This will be used by P2P providers and orchestrators (in subsequent patches) to ensure block devices can support P2P memory before submitting P2P backed pages tosubmit_bio().Nit pick, but the subject line still says that it checks support for requests. This patch just adds the ability to flag support in the queue.
What about the following? MAINTAINERS doesn't list a specific maintainer for include/linux/blkdev.h (maybe the "BLOCK LAYER" entry should have an F: pattern for it?), but I'd really like your ack before merging this.
commit 7e647ae1eda290786851c3dff4f38189b982386d
Author: Logan Gunthorpe <[email protected]> Date: Thu Oct 4 15:27:41 2018 -0600
block: Add PCI P2P flag for request queue
Add QUEUE_FLAG_PCI_P2P, meaning a driver's request queue supports targeting P2P memory. This will be used by P2P providers and orchestrators (in subsequent patches) to ensure block devices can support P2P memory before submitting P2P-backed pages to submit_bio().
include/linux/blkdev.h +3 -0
@@ -699,6 +699,7 @@ struct request_queue {#define QUEUE_FLAG_SCSI_PASSTHROUGH 27/* queue supports SCSI commands */ #define QUEUE_FLAG_QUIESCED 28/* queue has been quiesced */ #define QUEUE_FLAG_PREEMPT_ONLY 29/* only process REQ_PREEMPT requests */ +#define QUEUE_FLAG_PCI_P2PDMA 30/* device supports PCI p2p requests */ #define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \ (1 << QUEUE_FLAG_SAME_COMP) | \
@@ -731,6 +732,8 @@ bool blk_queue_flag_test_and_clear(unsigned int flag, struct request_queue *q);#define blk_queue_dax (q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)#define blk_queue_scsi_passthrough (q) \ test_bit(QUEUE_FLAG_SCSI_PASSTHROUGH, &(q)->queue_flags) +#define blk_queue_pci_p2pdma (q) \ + test_bit(QUEUE_FLAG_PCI_P2PDMA, &(q)->queue_flags)#define blk_noretry_request (rq) \ ((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
Jens Axboe
maintainerRe: [PATCH v9 07/13] block: Add PCI P2P flag for request queue and check support for requests
Quoted 22 lines · Bjorn Helgaas
Quoted 9 lines · Jens Axboe
Quoted 4 lines · Logan Gunthorpe
QUEUE_FLAG_PCI_P2Pis introduced meaning a driver's request queue supports targeting P2P memory. This will be used by P2P providers and orchestrators (in subsequent patches) to ensure block devices can support P2P memory before submitting P2P backed pages tosubmit_bio().Nit pick, but the subject line still says that it checks support for requests. This patch just adds the ability to flag support in the queue.
What about the following?
MAINTAINERSdoesn't list a specific maintainer forinclude/linux/blkdev.h(maybe the "BLOCK LAYER" entry should have an F: pattern for it?), but I'd really like your ack before merging this.commit
7e647ae1eda290786851c3dff4f38189b982386dAuthor: Logan Gunthorpe <[email protected]> Date: Thu Oct 4 15:27:41 2018 -0600block: Add PCI P2P flag for request queue
Quoted 8 lines
Add
QUEUE_FLAG_PCI_P2P, meaning a driver's request queue supports targeting P2P memory. This will be used by P2P providers and orchestrators (in subsequent patches) to ensure block devices can support P2P memory before submitting P2P-backed pages tosubmit_bio().Signed-off-by Logan Gunthorpe <[email protected]>Signed-off-by Bjorn Helgaas <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>
You can add my acked-by to this one.
Signature · 1 line
Jens Axboe
Logan Gunthorpe
maintainerPATCH v9 8/13 IB/core: Ensure we map P2P memory correctly in rdma_rw_ctx_[init|destroy]()
In reply to Logan GunthorpeIn order to use PCI P2P memory the pci_p2pmem_map_sg() function must be called to map the correct PCI bus address.
To do this, check the first page in the scatter list to see if it is P2P memory or not. At the moment, scatter lists that contain P2P memory must be homogeneous so if the first page is P2P the entire SGL should be P2P.
--- drivers/infiniband/core/rw.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/infiniband/core/rw.c +9 -2
@@ -12,6 +12,7 @@*/ #include <linux/moduleparam.h> #include <linux/slab.h> +#include <linux/pci-p2pdma.h> #include <rdma/mr_pool.h> #include <rdma/rw.h>
@@ -280,7 +281,11 @@ int rdma_rw_ctx_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp, u8 port_num,struct ib_device *dev = qp->pd->device;int ret; - ret =ib_dma_map_sg (dev, sg, sg_cnt, dir); +if (is_pci_p2pdma_page (sg_page (sg))) + ret =pci_p2pdma_map_sg (dev->dma_device, sg, sg_cnt, dir); +else + ret =ib_dma_map_sg (dev, sg, sg_cnt, dir); +if (!ret)return -ENOMEM ; sg_cnt = ret;
@@ -602,7 +607,9 @@ void rdma_rw_ctx_destroy(struct rdma_rw_ctx *ctx, struct ib_qp *qp, u8 port_num,break ; } -ib_dma_unmap_sg (qp->pd->device, sg, sg_cnt, dir); +/* P2PDMA contexts do not need to be unmapped */ +if (!is_pci_p2pdma_page (sg_page (sg))) +ib_dma_unmap_sg (qp->pd->device, sg, sg_cnt, dir); }EXPORT_SYMBOL (rdma_rw_ctx_destroy);
Signature · 1 line
2.19.0
Re: [PATCH v9 08/13] IB/core: Ensure we map P2P memory correctly in rdma_rw_ctx_[init|destroy]()
Logan Gunthorpe
maintainerPATCH v9 9/13 nvme-pci: Use PCI p2pmem subsystem to manage the CMB
In reply to Logan GunthorpeRegister the CMB buffer as p2pmem and use the appropriate allocation functions to create and destroy the IO submission queues.
If the CMB supports WDS and RDS, publish it for use as P2P memory by other devices.
Kernels without CONFIG_PCI_P2PDMA will also no longer support NVMe CMB. However, seeing the main use-cases for the CMB is P2P operations, this seems like a reasonable dependency.
We drop the __iomem safety on the buffer seeing that, by convention, it's safe to directly access memory mapped by memremap()/devm_memremap_pages(). Architectures where this is not safe will not be supported by memremap() and therefore will not be support PCI P2P and have no support for CMB.
--- drivers/nvme/host/pci.c | 80 +++++++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 35 deletions(-)
drivers/nvme/host/pci.c +45 -35
@@ -30,6 +30,7 @@#include <linux/types.h> #include <linux/io-64-nonatomic-lo-hi.h> #include <linux/sed-opal.h> +#include <linux/pci-p2pdma.h> #include "nvme.h"
@@ -99,9 +100,8 @@ struct nvme_dev {struct work_struct remove_work;struct mutex shutdown_lock;bool subsystem; -void __iomem *cmb; -pci_bus_addr_t cmb_bus_addr;u64 cmb_size; +bool cmb_use_sqes;u32 cmbsz;u32 cmbloc;struct nvme_ctrl ctrl;
@@ -158,7 +158,7 @@ struct nvme_queue {struct nvme_dev *dev;spinlock_t sq_lock;struct nvme_command *sq_cmds; -struct nvme_command __iomem *sq_cmds_io; +bool sq_cmds_is_io;spinlock_t cq_lock____cacheline_aligned_in_smp ;volatile struct nvme_completion *cqes;struct blk_mq_tags **tags;
@@ -447,11 +447,8 @@ static int nvme_pci_map_queues(struct blk_mq_tag_set *set)static void nvme_submit_cmd (struct nvme_queue *nvmeq,struct nvme_command *cmd) {spin_lock (&nvmeq->sq_lock); -if (nvmeq->sq_cmds_io) -memcpy_toio (&nvmeq->sq_cmds_io[nvmeq->sq_tail], cmd, -sizeof (*cmd)); -else -memcpy (&nvmeq->sq_cmds[nvmeq->sq_tail], cmd,sizeof (*cmd)); + +memcpy (&nvmeq->sq_cmds[nvmeq->sq_tail], cmd,sizeof (*cmd));if (++nvmeq->sq_tail == nvmeq->q_depth) nvmeq->sq_tail =0 ;
@@ -1232,9 +1229,18 @@ static void nvme_free_queue(struct nvme_queue *nvmeq) {dma_free_coherent (nvmeq->q_dmadev,CQ_SIZE (nvmeq->q_depth), (void *)nvmeq->cqes, nvmeq->cq_dma_addr); -if (nvmeq->sq_cmds) -dma_free_coherent (nvmeq->q_dmadev,SQ_SIZE (nvmeq->q_depth), - nvmeq->sq_cmds, nvmeq->sq_dma_addr); + +if (nvmeq->sq_cmds) { +if (nvmeq->sq_cmds_is_io) +pci_free_p2pmem (to_pci_dev (nvmeq->q_dmadev), + nvmeq->sq_cmds, +SQ_SIZE (nvmeq->q_depth)); +else +dma_free_coherent (nvmeq->q_dmadev, +SQ_SIZE (nvmeq->q_depth), + nvmeq->sq_cmds, + nvmeq->sq_dma_addr); + } }static void nvme_free_queues (struct nvme_dev *dev,int lowest)
@@ -1323,12 +1329,21 @@ static int nvme_cmb_qdepth(struct nvme_dev *dev, int nr_io_queues,static int nvme_alloc_sq_cmds (struct nvme_dev *dev,struct nvme_queue *nvmeq,int qid,int depth) { -/* CMB SQEs will be mapped before creation */ -if (qid && dev->cmb && use_cmb_sqes && (dev->cmbsz &NVME_CMBSZ_SQS )) -return 0 ; +struct pci_dev *pdev =to_pci_dev (dev->dev); + +if (qid && dev->cmb_use_sqes && (dev->cmbsz &NVME_CMBSZ_SQS )) { + nvmeq->sq_cmds =pci_alloc_p2pmem (pdev,SQ_SIZE (depth)); + nvmeq->sq_dma_addr =pci_p2pmem_virt_to_bus (pdev, + nvmeq->sq_cmds); + nvmeq->sq_cmds_is_io = true; + } + +if (!nvmeq->sq_cmds) { + nvmeq->sq_cmds =dma_alloc_coherent (dev->dev,SQ_SIZE (depth), + &nvmeq->sq_dma_addr,GFP_KERNEL ); + nvmeq->sq_cmds_is_io = false; + } - nvmeq->sq_cmds =dma_alloc_coherent (dev->dev,SQ_SIZE (depth), - &nvmeq->sq_dma_addr,GFP_KERNEL );if (!nvmeq->sq_cmds)return -ENOMEM ;return 0 ;
@@ -1405,13 +1420,6 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid)int result;s16 vector; -if (dev->cmb && use_cmb_sqes && (dev->cmbsz &NVME_CMBSZ_SQS )) { -unsigned offset = (qid -1 ) *roundup (SQ_SIZE (nvmeq->q_depth), - dev->ctrl.page_size); - nvmeq->sq_dma_addr = dev->cmb_bus_addr + offset; - nvmeq->sq_cmds_io = dev->cmb + offset; - } -/* * A queue's vector matches the queue identifier unless the controller * has only one vector available.
@@ -1652,9 +1660,6 @@ static void nvme_map_cmb(struct nvme_dev *dev)return ; dev->cmbloc =readl (dev->bar +NVME_REG_CMBLOC ); -if (!use_cmb_sqes) -return ; - size =nvme_cmb_size_unit (dev) *nvme_cmb_size (dev); offset =nvme_cmb_size_unit (dev) *NVME_CMB_OFST (dev->cmbloc); bar =NVME_CMB_BIR (dev->cmbloc);
@@ -1671,11 +1676,18 @@ static void nvme_map_cmb(struct nvme_dev *dev)if (size > bar_size - offset) size = bar_size - offset; - dev->cmb =ioremap_wc (pci_resource_start (pdev, bar) + offset, size); -if (!dev->cmb) +if (pci_p2pdma_add_resource (pdev, bar, size, offset)) { +dev_warn (dev->ctrl.device, +"failed to register the CMB\n" );return ; - dev->cmb_bus_addr =pci_bus_address (pdev, bar) + offset; + } + dev->cmb_size = size; + dev->cmb_use_sqes = use_cmb_sqes && (dev->cmbsz &NVME_CMBSZ_SQS ); + +if ((dev->cmbsz & (NVME_CMBSZ_WDS |NVME_CMBSZ_RDS )) == + (NVME_CMBSZ_WDS |NVME_CMBSZ_RDS )) +pci_p2pmem_publish (pdev, true);if (sysfs_add_file_to_group (&dev->ctrl.device->kobj, &dev_attr_cmb.attr,NULL ))
@@ -1685,12 +1697,10 @@ static void nvme_map_cmb(struct nvme_dev *dev)static inline void nvme_release_cmb (struct nvme_dev *dev) { -if (dev->cmb) { -iounmap (dev->cmb); - dev->cmb =NULL ; +if (dev->cmb_size) {sysfs_remove_file_from_group (&dev->ctrl.device->kobj, &dev_attr_cmb.attr,NULL ); - dev->cmbsz =0 ; + dev->cmb_size =0 ; } }
@@ -1889,13 +1899,13 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)if (nr_io_queues ==0 )return 0 ; -if (dev->cmb && (dev->cmbsz &NVME_CMBSZ_SQS )) { +if (dev->cmb_use_sqes) { result =nvme_cmb_qdepth (dev, nr_io_queues,sizeof (struct nvme_command ));if (result >0 ) dev->q_depth = result;else -nvme_release_cmb (dev); + dev->cmb_use_sqes = false; } do {
Signature · 1 line
2.19.0
Re: [PATCH v9 09/13] nvme-pci: Use PCI p2pmem subsystem to manage the CMB
Looks good,
Logan Gunthorpe
maintainerPATCH v9 10/13 nvme-pci: Add support for P2P memory in requests
In reply to Logan GunthorpeFor P2P requests, we must use the pci_p2pmem_map_sg() function instead of the dma_map_sg functions.
With that, we can then indicate PCI_P2P support in the request queue. For this, we create an NVME_F_PCI_P2P flag which tells the core to set QUEUE_FLAG_PCI_P2P in the request queue.
--- drivers/nvme/host/core.c | 4 ++++ drivers/nvme/host/nvme.h | 1 + drivers/nvme/host/pci.c | 17 +++++++++++++---- 3 files changed, 18 insertions(+), 4 deletions(-)
drivers/nvme/host/core.c +4 -0
@@ -3051,7 +3051,11 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) ns->queue =blk_mq_init_queue (ctrl->tagset);if (IS_ERR (ns->queue)) goto out_free_ns; +blk_queue_flag_set (QUEUE_FLAG_NONROT , ns->queue); +if (ctrl->ops->flags &NVME_F_PCI_P2PDMA ) +blk_queue_flag_set (QUEUE_FLAG_PCI_P2PDMA , ns->queue); + ns->queue->queuedata = ns; ns->ctrl = ctrl;
drivers/nvme/host/nvme.h +1 -0
@@ -343,6 +343,7 @@ struct nvme_ctrl_ops {unsigned int flags;#define NVME_F_FABRICS (1 << 0)#define NVME_F_METADATA_SUPPORTED (1 << 1) +#define NVME_F_PCI_P2PDMA (1 << 2)int (*reg_read32)(struct nvme_ctrl *ctrl,u32 off,u32 *val);int (*reg_write32)(struct nvme_ctrl *ctrl,u32 off,u32 val);int (*reg_read64)(struct nvme_ctrl *ctrl,u32 off,u64 *val);
drivers/nvme/host/pci.c +13 -4
@@ -745,8 +745,13 @@ static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req, goto out; ret =BLK_STS_RESOURCE ; - nr_mapped =dma_map_sg_attrs (dev->dev, iod->sg, iod->nents, dma_dir, -DMA_ATTR_NO_WARN ); + +if (is_pci_p2pdma_page (sg_page (iod->sg))) + nr_mapped =pci_p2pdma_map_sg (dev->dev, iod->sg, iod->nents, + dma_dir); +else + nr_mapped =dma_map_sg_attrs (dev->dev, iod->sg, iod->nents, + dma_dir,DMA_ATTR_NO_WARN );if (!nr_mapped) goto out;
@@ -788,7 +793,10 @@ static void nvme_unmap_data(struct nvme_dev *dev, struct request *req) DMA_TO_DEVICE :DMA_FROM_DEVICE ;if (iod->nents) { -dma_unmap_sg (dev->dev, iod->sg, iod->nents, dma_dir); +/* P2PDMA requests do not need to be unmapped */ +if (!is_pci_p2pdma_page (sg_page (iod->sg))) +dma_unmap_sg (dev->dev, iod->sg, iod->nents, dma_dir); +if (blk_integrity_rq (req))dma_unmap_sg (dev->dev, &iod->meta_sg,1 , dma_dir); }
@@ -2400,7 +2408,8 @@ static int nvme_pci_get_address(struct nvme_ctrl *ctrl, char *buf, int size)static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = { .name ="pcie" , .module =THIS_MODULE , - .flags =NVME_F_METADATA_SUPPORTED , + .flags =NVME_F_METADATA_SUPPORTED | +NVME_F_PCI_P2PDMA , .reg_read32 = nvme_pci_reg_read32, .reg_write32 = nvme_pci_reg_write32, .reg_read64 = nvme_pci_reg_read64,
Signature · 1 line
2.19.0
Logan Gunthorpe
maintainerPATCH v9 11/13 nvme-pci: Add a quirk for a pseudo CMB
In reply to Logan GunthorpeIntroduce a quirk to use CMB-like memory on older devices that have an exposed BAR but do not advertise support for using CMBLOC and CMBSIZE.
We'd like to use some of these older cards to test P2P memory.
--- drivers/nvme/host/nvme.h | 7 +++++++ drivers/nvme/host/pci.c | 24 ++++++++++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-)
drivers/nvme/host/nvme.h +7 -0
@@ -90,6 +90,13 @@ enum nvme_quirks {* Set MEDIUM priority on SQ creation */ NVME_QUIRK_MEDIUM_PRIO_SQ = (1 <<7 ), + +/* +* Pseudo CMB Support on BAR 4. For adapters like the Microsemi +* NVRAM that have CMB-like memory on a BAR but does not set +* CMBLOC or CMBSZ. +*/ + NVME_QUIRK_PSEUDO_CMB_BAR4 = (1 << 8), };/*
drivers/nvme/host/pci.c +20 -4
@@ -1644,6 +1644,13 @@ static ssize_t nvme_cmb_show(struct device *dev, } static DEVICE_ATTR(cmb, S_IRUGO, nvme_cmb_show, NULL); +static u32 nvme_pseudo_cmbsz (struct pci_dev *pdev,int bar) +{ +return NVME_CMBSZ_WDS |NVME_CMBSZ_RDS | + (((ilog2 (SZ_16M ) -12 ) /4 ) <<NVME_CMBSZ_SZU_SHIFT ) | + ((pci_resource_len (pdev, bar) /SZ_16M ) <<NVME_CMBSZ_SZ_SHIFT ); +} +static u64 nvme_cmb_size_unit (struct nvme_dev *dev) {u8 szu = (dev->cmbsz >>NVME_CMBSZ_SZU_SHIFT ) &NVME_CMBSZ_SZU_MASK ;
@@ -1663,10 +1670,15 @@ static void nvme_map_cmb(struct nvme_dev *dev)struct pci_dev *pdev =to_pci_dev (dev->dev);int bar; - dev->cmbsz =readl (dev->bar +NVME_REG_CMBSZ ); -if (!dev->cmbsz) -return ; - dev->cmbloc =readl (dev->bar +NVME_REG_CMBLOC ); +if (dev->ctrl.quirks &NVME_QUIRK_PSEUDO_CMB_BAR4 ) { + dev->cmbsz =nvme_pseudo_cmbsz (pdev,4 ); + dev->cmbloc =4 ; + }else { + dev->cmbsz =readl (dev->bar +NVME_REG_CMBSZ ); +if (!dev->cmbsz) +return ; + dev->cmbloc =readl (dev->bar +NVME_REG_CMBLOC ); + } size =nvme_cmb_size_unit (dev) *nvme_cmb_size (dev); offset =nvme_cmb_size_unit (dev) *NVME_CMB_OFST (dev->cmbloc);
@@ -2715,6 +2727,10 @@ static const struct pci_device_id nvme_id_table[] = { .driver_data = NVME_QUIRK_LIGHTNVM, }, { PCI_DEVICE(0x1d1d, 0x2601), /* CNEX Granby */ .driver_data = NVME_QUIRK_LIGHTNVM, }, + { PCI_DEVICE(0x11f8, 0xf117), /* Microsemi NVRAM adaptor */ + .driver_data = NVME_QUIRK_PSEUDO_CMB_BAR4, }, + { PCI_DEVICE(0x1db1, 0x0002), /* Everspin nvNitro adaptor */ + .driver_data = NVME_QUIRK_PSEUDO_CMB_BAR4, }, { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, 0xffffff) }, { PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001) }, { PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
Signature · 1 line
2.19.0
Christoph Hellwig
maintainerRe: [PATCH v9 11/13] nvme-pci: Add a quirk for a pseudo CMB
Quoted 5 lines · Logan Gunthorpe
Introduce a quirk to use CMB-like memory on older devices that have an exposed BAR but do not advertise support for using CMBLOC and CMBSIZE.
We'd like to use some of these older cards to test P2P memory.
I can't sat that I really like adding support for non-standardized "features" (vs bugs) in the nvme driver.
Logan Gunthorpe
maintainerRe: [PATCH v9 11/13] nvme-pci: Add a quirk for a pseudo CMB
Quoted 10 lines · Christoph Hellwig
Quoted 5 lines · Logan Gunthorpe
Introduce a quirk to use CMB-like memory on older devices that have an exposed BAR but do not advertise support for using CMBLOC and CMBSIZE.
We'd like to use some of these older cards to test P2P memory.
I can't sat that I really like adding support for non-standardized "features" (vs bugs) in the nvme driver.
I can live with this patch being dropped.
Logan
Logan Gunthorpe
maintainerPATCH v9 12/13 nvmet: Introduce helper functions to allocate and free request SGLs
In reply to Logan GunthorpeAdd helpers to allocate and free the SGL in a struct nvmet_req:
int nvmet_req_alloc_sgl(struct nvmet_req *req)
void nvmet_req_free_sgl(struct nvmet_req *req)
This will be expanded in a future patch to implement peer-to-peer memory DMAs and should be common with all target drivers.
The new helpers are used in nvmet-rdma. Seeing we use req.transfer_len as the length of the SGL it is set earlier and cleared on any error. It also seems to be unnecessary to accumulate the length as the map_sgl functions should only ever be called once per request.
--- drivers/nvme/target/core.c | 18 ++++++++++++++++++ drivers/nvme/target/nvmet.h | 2 ++ drivers/nvme/target/rdma.c | 20 ++++++++++++-------- 3 files changed, 32 insertions(+), 8 deletions(-)
drivers/nvme/target/core.c +18 -0
@@ -725,6 +725,24 @@ void nvmet_req_execute(struct nvmet_req *req) }EXPORT_SYMBOL_GPL (nvmet_req_execute); +int nvmet_req_alloc_sgl (struct nvmet_req *req) +{ + req->sg =sgl_alloc (req->transfer_len,GFP_KERNEL , &req->sg_cnt); +if (!req->sg) +return -ENOMEM ; + +return 0 ; +} +EXPORT_SYMBOL_GPL (nvmet_req_alloc_sgl); + +void nvmet_req_free_sgl (struct nvmet_req *req) +{ +sgl_free (req->sg); + req->sg =NULL ; + req->sg_cnt =0 ; +} +EXPORT_SYMBOL_GPL (nvmet_req_free_sgl); +static inline bool nvmet_cc_en (u32 cc) {return (cc >>NVME_CC_EN_SHIFT ) &0x1 ;
drivers/nvme/target/nvmet.h +2 -0
@@ -336,6 +336,8 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq,void nvmet_req_uninit (struct nvmet_req *req);void nvmet_req_execute (struct nvmet_req *req);void nvmet_req_complete (struct nvmet_req *req,u16 status); +int nvmet_req_alloc_sgl (struct nvmet_req *req); +void nvmet_req_free_sgl (struct nvmet_req *req);void nvmet_cq_setup (struct nvmet_ctrl *ctrl,struct nvmet_cq *cq,u16 qid,u16 size);
drivers/nvme/target/rdma.c +12 -8
@@ -503,7 +503,7 @@ static void nvmet_rdma_release_rsp(struct nvmet_rdma_rsp *rsp) }if (rsp->req.sg != rsp->cmd->inline_sg) -sgl_free (rsp->req.sg); +nvmet_req_free_sgl (&rsp->req);if (unlikely (!list_empty_careful (&queue->rsp_wr_wait_list)))nvmet_rdma_process_wr_wait_list (queue);
@@ -652,24 +652,24 @@ static u16 nvmet_rdma_map_sgl_keyed(struct nvmet_rdma_rsp *rsp, {struct rdma_cm_id *cm_id = rsp->queue->cm_id;u64 addr =le64_to_cpu (sgl->addr); -u32 len =get_unaligned_le24 (sgl->length);u32 key =get_unaligned_le32 (sgl->key);int ret; + rsp->req.transfer_len =get_unaligned_le24 (sgl->length); +/* no data command? */ -if (!len) +if (!rsp->req.transfer_len)return 0 ; - rsp->req.sg =sgl_alloc (len,GFP_KERNEL , &rsp->req.sg_cnt); -if (!rsp->req.sg) -return NVME_SC_INTERNAL ; + ret =nvmet_req_alloc_sgl (&rsp->req); +if (ret <0 ) + goto error_out; ret =rdma_rw_ctx_init (&rsp->rw, cm_id->qp, cm_id->port_num, rsp->req.sg, rsp->req.sg_cnt,0 , addr, key,nvmet_data_dir (&rsp->req));if (ret <0 ) -return NVME_SC_INTERNAL ; - rsp->req.transfer_len += len; + goto error_out; rsp->n_rdma += ret;if (invalidate) {
@@ -678,6 +678,10 @@ static u16 nvmet_rdma_map_sgl_keyed(struct nvmet_rdma_rsp *rsp, }return 0 ; + +error_out: + rsp->req.transfer_len =0 ; +return NVME_SC_INTERNAL ; }static u16 nvmet_rdma_map_sgl (struct nvmet_rdma_rsp *rsp)
Signature · 1 line
2.19.0
Re: [PATCH v9 12/13] nvmet: Introduce helper functions to allocate and free request SGLs
Looks fine,
Logan Gunthorpe
maintainerPATCH v9 13/13 nvmet: Optionally use PCI P2P memory
In reply to Logan GunthorpeWe create a configfs attribute in each nvme-fabrics namespace to enable P2P memory use. The attribute may be enabled (with a boolean) or a specific P2P device may be given (with the device's PCI name).
When enabled, the namespace will ensure the underlying block device supports P2P and that it is compatible with any specified P2P device. If no device was specified it will ensure there is compatible P2P memory somewhere in the system. Enabling an namespace with P2P memory will fail with EINVAL (and an appropriate dmesg error) if any of these conditions are not met.
Once a controller is setup on a specific port, the P2P device to use for each namespace will be found and stored in a radix tree by namespace ID. When memory is allocated for a request, the tree is used to look up the P2P device to allocate memory against. If no device is in the tree (because no appropriate device was found), or if allocation of P2P memory fails, the system will fall back to using regular memory.
--- drivers/nvme/target/configfs.c | 45 ++++++++ drivers/nvme/target/core.c | 164 +++++++++++++++++++++++++++++- drivers/nvme/target/io-cmd-bdev.c | 3 + drivers/nvme/target/nvmet.h | 15 +++ drivers/nvme/target/rdma.c | 2 + 5 files changed, 228 insertions(+), 1 deletion(-)
drivers/nvme/target/configfs.c +45 -0
@@ -17,6 +17,8 @@#include <linux/slab.h> #include <linux/stat.h> #include <linux/ctype.h> +#include <linux/pci.h> +#include <linux/pci-p2pdma.h> #include "nvmet.h"
@@ -340,6 +342,46 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item,CONFIGFS_ATTR (nvmet_ns_, device_path); +#ifdef CONFIG_PCI_P2PDMA +static ssize_t nvmet_ns_p2pmem_show (struct config_item *item,char *page) +{ +struct nvmet_ns *ns =to_nvmet_ns (item); + +return pci_p2pdma_enable_show (page, ns->p2p_dev, ns->use_p2pmem); +} + +static ssize_t nvmet_ns_p2pmem_store (struct config_item *item, +const char *page,size_t count) +{ +struct nvmet_ns *ns =to_nvmet_ns (item); +struct pci_dev *p2p_dev =NULL ; +bool use_p2pmem; +int error; +int ret = count; + +mutex_lock (&ns->subsys->lock); +if (ns->enabled) { + ret = -EBUSY ; + goto out_unlock; + } + + error =pci_p2pdma_enable_store (page, &p2p_dev, &use_p2pmem); +if (error) +return error; + + ns->use_p2pmem = use_p2pmem; +pci_dev_put (ns->p2p_dev); + ns->p2p_dev = p2p_dev; + +out_unlock: +mutex_unlock (&ns->subsys->lock); + +return ret; +} + +CONFIGFS_ATTR (nvmet_ns_, p2pmem); +#endif /* CONFIG_PCI_P2PDMA */ +static ssize_t nvmet_ns_device_uuid_show (struct config_item *item,char *page) {return sprintf (page,"%pUb\n" , &to_nvmet_ns (item)->uuid);
@@ -509,6 +551,9 @@ static struct configfs_attribute *nvmet_ns_attrs[] = { &nvmet_ns_attr_ana_grpid, &nvmet_ns_attr_enable, &nvmet_ns_attr_buffered_io, +#ifdef CONFIG_PCI_P2PDMA + &nvmet_ns_attr_p2pmem, +#endif NULL, };
drivers/nvme/target/core.c +163 -1
245 lines not on this page · raw message
drivers/nvme/target/io-cmd-bdev.c +3 -0
@@ -78,6 +78,9 @@ static void nvmet_bdev_execute_rw(struct nvmet_req *req) op =REQ_OP_READ ; } +if (is_pci_p2pdma_page (sg_page (req->sg))) + op_flags |=REQ_NOMERGE ; + sector =le64_to_cpu (req->cmd->rw.slba); sector <<= (req->ns->blksize_shift -9 );
drivers/nvme/target/nvmet.h +15 -0
@@ -26,6 +26,7 @@#include <linux/configfs.h> #include <linux/rcupdate.h> #include <linux/blkdev.h> +#include <linux/radix-tree.h> #define NVMET_ASYNC_EVENTS 4#define NVMET_ERROR_LOG_SLOTS 128
@@ -77,6 +78,9 @@ struct nvmet_ns {struct completion disable_done;mempool_t *bvec_pool;struct kmem_cache *bvec_cache; + +int use_p2pmem; +struct pci_dev *p2p_dev; };static inline struct nvmet_ns *to_nvmet_ns (struct config_item *item)
@@ -84,6 +88,11 @@ static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item) return container_of(to_config_group(item), struct nvmet_ns, group); } +static inline struct device *nvmet_ns_dev (struct nvmet_ns *ns) +{ +return disk_to_dev (ns->bdev->bd_disk); +} +struct nvmet_cq {u16 qid;u16 size;
@@ -184,6 +193,9 @@ struct nvmet_ctrl {char subsysnqn[NVMF_NQN_FIELD_LEN ];char hostnqn[NVMF_NQN_FIELD_LEN ]; + +struct device *p2p_client; +struct radix_tree_root p2p_ns_map; };struct nvmet_subsys {
@@ -294,6 +306,9 @@ struct nvmet_req {void (*execute)(struct nvmet_req *req);const struct nvmet_fabrics_ops *ops; + +struct pci_dev *p2p_dev; +struct device *p2p_client; };extern struct workqueue_struct *buffered_io_wq;
drivers/nvme/target/rdma.c +2 -0
@@ -749,6 +749,8 @@ static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue, cmd->send_sge.addr, cmd->send_sge.length, DMA_TO_DEVICE); + cmd->req.p2p_client = &queue->dev->device->dev; +if (!nvmet_req_init (&cmd->req, &queue->nvme_cq, &queue->nvme_sq, &nvmet_rdma_ops))return ;
Signature · 1 line
2.19.0
Sagi Grimberg
maintainerRe: [PATCH v9 13/13] nvmet: Optionally use PCI P2P memory
Quoted 24 lines
drivers/nvme/target/nvmet.h +4
-0@@ -26,6 +26,7 @@#include <linux/configfs.h> #include <linux/rcupdate.h> #include <linux/blkdev.h> +#include <linux/radix-tree.h> #define NVMET_ASYNC_EVENTS 4#define NVMET_ERROR_LOG_SLOTS 128@@ -77,6 +78,9 @@ struct nvmet_ns {struct completion disable_done;mempool_t *bvec_pool;struct kmem_cache *bvec_cache; + +int use_p2pmem; +struct pci_dev *p2p_dev; };static inline struct nvmet_ns *to_nvmet_ns (struct config_item *item)
drivers/nvme/target/nvmet.h static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item) · line 94
Quoted 7 lines
drivers/nvme/target/nvmet.h +4
-0@@ -84,6 +88,11 @@ static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item) return container_of(to_config_group(item), struct nvmet_ns, group); } +static inline struct device *nvmet_ns_dev (struct nvmet_ns *ns) +{ +return disk_to_dev (ns->bdev->bd_disk); +}
This needs to handle non bdev namespaces.
Quoted 29 lines
drivers/nvme/target/nvmet.h +7
-0+struct nvmet_cq {u16 qid;u16 size;@@ -184,6 +193,9 @@ struct nvmet_ctrl {char subsysnqn[NVMF_NQN_FIELD_LEN ];char hostnqn[NVMF_NQN_FIELD_LEN ]; + +struct device *p2p_client; +struct radix_tree_root p2p_ns_map; };struct nvmet_subsys {@@ -294,6 +306,9 @@ struct nvmet_req {void (*execute)(struct nvmet_req *req);const struct nvmet_fabrics_ops *ops; + +struct pci_dev *p2p_dev; +struct device *p2p_client; };extern struct workqueue_struct *buffered_io_wq;drivers/nvme/target/rdma.c +0
-0
drivers/nvme/target/rdma.c static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue, · line 756
Quoted 8 lines
drivers/nvme/target/rdma.c +2
-0@@ -749,6 +749,8 @@ static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue, cmd->send_sge.addr, cmd->send_sge.length, DMA_TO_DEVICE); + cmd->req.p2p_client = &queue->dev->device->dev; +if (!nvmet_req_init (&cmd->req, &queue->nvme_cq, &queue->nvme_sq, &nvmet_rdma_ops))return ;
And this?
--
drivers/nvme/target/fc.c +1 -0
@@ -2280,6 +2280,7 @@ nvmet_fc_handle_fcp_rqst(struct nvmet_fc_tgtport *tgtport, fod->req.cmd = &fod->cmdiubuf.sqe; fod->req.rsp = &fod->rspiubuf.cqe; fod->req.port = tgtport->pe->port; + fod->req.p2p_client = tgtport->dev;/* clear any response payload */ memset (&fod->rspiubuf,0 ,sizeof (fod->rspiubuf));
--
Other than that this looks good!
Logan Gunthorpe
maintainerRe: [PATCH v9 13/13] nvmet: Optionally use PCI P2P memory
Quoted 6 lines · Sagi Grimberg
Review
Quoted 4 lines
(quoted diff) +4
-0+static inline struct device *nvmet_ns_dev(struct nvmet_ns *ns) +{ + return disk_to_dev(ns->bdev->bd_disk); +}This needs to handle non bdev namespaces.
As it's coded now the helper never gets called unless ns->bdev is not null. But in general, yes you are right, we should probably return NULL if ns->bdev is NULL.
Quoted 15 lines
And this?
--drivers/nvme/target/fc.c +1
-0@@ -2280,6 +2280,7 @@ nvmet_fc_handle_fcp_rqst(struct nvmet_fc_tgtport *tgtport, fod->req.cmd = &fod->cmdiubuf.sqe; fod->req.rsp = &fod->rspiubuf.cqe; fod->req.port = tgtport->pe->port; + fod->req.p2p_client = tgtport->dev;/* clear any response payload */ memset(&fod->rspiubuf, 0, sizeof(fod->rspiubuf));
Quoted 1 line
--
Sure, I guess that makes sense. I've never tried it with fc hardware but I assume there's no reason it wouldn't work.
I'll queue these changes up for a v10.
Logan
Christoph Hellwig
maintainerRe: [PATCH v9 13/13] nvmet: Optionally use PCI P2P memory
Quoted 10 lines · Logan Gunthorpe
Quoted 6 lines · Sagi Grimberg
Review
Quoted 4 lines
(quoted diff) +4
-0+static inline struct device *nvmet_ns_dev(struct nvmet_ns *ns) +{ + return disk_to_dev(ns->bdev->bd_disk); +}This needs to handle non bdev namespaces.
Quoted 3 lines
As it's coded now the helper never gets called unless
ns->bdevis not null. But in general, yes you are right, we should probably return NULL ifns->bdevis NULL.
I'd rather skip that for now.
Quoted 10 lines
Quoted 10 lines
index
ef286b72d958..3d12f5f4568d100644drivers/nvme/target/fc.c +1
-0@@ -2280,6 +2280,7 @@ nvmet_fc_handle_fcp_rqst(struct nvmet_fc_tgtport *tgtport, fod->req.cmd = &fod->cmdiubuf.sqe; fod->req.rsp = &fod->rspiubuf.cqe; fod->req.port = tgtport->pe->port; + fod->req.p2p_client = tgtport->dev;
Quoted 8 lines
Quoted 3 lines
drivers/nvme/target/fc.c +0
-0/* clear any response payload */ memset(&fod->rspiubuf, 0, sizeof(fod->rspiubuf));--
Sure, I guess that makes sense. I've never tried it with fc hardware but I assume there's no reason it wouldn't work.
I'll queue these changes up for a v10.
And I'd wait until someone has actually tested this case.
Sagi Grimberg
maintainerRe: [PATCH v9 13/13] nvmet: Optionally use PCI P2P memory
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Review
Quoted 5 lines
(quoted diff) +4
-0+static inline struct device *nvmet_ns_dev(struct nvmet_ns *ns) +{ + return disk_to_dev(ns->bdev->bd_disk); +}This needs to handle non bdev namespaces.
Quoted 5 lines
Quoted 3 lines
As it's coded now the helper never gets called unless
ns->bdevis not null. But in general, yes you are right, we should probably return NULL ifns->bdevis NULL.I'd rather skip that for now.
If there is a v10 I don't see why that cannot be addressed.
Quoted 17 lines
Quoted 17 lines
Quoted 12 lines
index
ef286b72d958..3d12f5f4568d100644drivers/nvme/target/fc.c +1
-0@@ -2280,6 +2280,7 @@ nvmet_fc_handle_fcp_rqst(struct nvmet_fc_tgtport *tgtport, fod->req.cmd = &fod->cmdiubuf.sqe; fod->req.rsp = &fod->rspiubuf.cqe; fod->req.port = tgtport->pe->port; + fod->req.p2p_client = tgtport->dev;/* clear any response payload */ memset(&fod->rspiubuf, 0, sizeof(fod->rspiubuf));Quoted 1 line
--
Sure, I guess that makes sense. I've never tried it with fc hardware but I assume there's no reason it wouldn't work.
Quoted 3 lines
Quoted 1 line
I'll queue these changes up for a v10.
And I'd wait until someone has actually tested this case.
Oh yes, agreed. Should work though...
Logan Gunthorpe
maintainerRe: [PATCH v9 13/13] nvmet: Optionally use PCI P2P memory
In reply to Christoph HellwigQuoted 8 lines · Christoph Hellwig
Quoted 5 lines · Logan Gunthorpe
Quoted 1 line
This needs to handle non bdev namespaces.
As it's coded now the helper never gets called unless
ns->bdevis not null. But in general, yes you are right, we should probably return NULL ifns->bdevis NULL.I'd rather skip that for now.
Ok, if I do a v10 I'll add it but I don't have anything else queued up at the moment.
Quoted 6 lines
Quoted 4 lines
Sure, I guess that makes sense. I've never tried it with fc hardware but I assume there's no reason it wouldn't work.
I'll queue these changes up for a v10.
And I'd wait until someone has actually tested this case.
Agreed.
Thanks for the review!
Logan
Quoted 270 lines · Logan Gunthorpe
This is v9 for the p2pdma patch set. It has some substantial changes from the previous version. Essentially, the last patch has changed based on information from Sagi that the P2P memory can be assigned per namespace instead of globally. To that end, I've added a radix tree that stores the p2p devices to use for each namespace and a lookup will be done on that before allocating the memory.
This has some knock on effects of simplifying the requirements from the p2pdma code and so we drop all the client list code seeing we don't need to worry about selecting a P2P device that works with every namespace at once; only each namespace independantly. Thus, Patch 1, 6 and 13 have significant changes. However, I think the removal of the client list code will be generally appreciated based on the feedback I've gotten from Christian.
As usual, a git repo of the branch is available here:
https://github.com/sbates130272/linux-p2pmem pci-p2p-v9
Thanks,
Logan
--
Changes in v9:
* Rebased on v4.19-rc6
* Dropedpci_p2pdma_add_client(),pci_p2pdma_remove_client()andpci_p2pdma_client_list_free(), and reworkedpci_p2pdma_distance()andpci_p2pmem_find()to take simple arrays instead oflist_heads.
* Updated the documentation to match
* Reworked the nvmet code to have a configfs attribute per namespace instead of per port. Then each namespace has it's own P2P device stored in a radix-tree in the controller (it can't be stored in thenvme_nsstructure seeing it needs to be unique per controller).
* Dropped the 'sq' argument innvmet_req_alloc_sgl()seeing I've noticed it is now available in thenvmet_reqstructure.
* Collected Keith's reviewed-by tags (however, I have not applied his tag to the last patch seeing I think it has changed too much since his review).Changes in v8:
* Added a couple of comments to address Bart's feedback and removed the bogus call to
percpu_ref_switch_to_atomic_sync()Changes in v7:
* Rebased on v4.19-rc5
* Fixed up commit message of patch 7 that was no longer accurate. (as pointed out by Jens)
* Change the configfs to not use "auto" or "none" and instead just use a 0/1/<pci_dev> (or boolean). This matches the existing nvme-target configfs booleans. (Per Bjorn)
* A handful of other minor changes and edits that were noticed by Bjorn
* Collected Acks from BjornChanges in v6:
* Rebased on v4.19-rc3
* Remove the changes to the common
submit_bio()path and instead setREQ_NOMERGEin the NVME target driver, when appropriate. Per discussions with Jens and Christoph.* Some minor grammar changes in the documentation as spotted by Randy.
Changes in v5:
* Rebased on v4.19-rc1
* Drop changing ACS settings in this patchset. Now, the code will only allow P2P transactions between devices whos downstream ports do not restrict P2P TLPs.
* Drop the
REQ_PCI_P2PDMAblock flag and instead useis_pci_p2pdma_page()to tell if a request is P2P or not. In that case we check for queue support and enforce usingREQ_NOMERGE. Per feedback from Christoph.* Drop the
pci_p2pdma_unmap_sg()function as it was empty and only there for symmetry and compatibility withdma_unmap_sg. Per feedback from Christoph.* Split off the logic to handle enabling P2P in NVMe fabrics' configfs into specific helpers in the p2pdma code. Per feedback from Christoph.
* A number of other minor cleanups and fixes as pointed out by Christoph and others.
Changes in v4:
* Change the original
upstream_bridges_match()function toupstream_bridge_distance()which calculates the distance between two devices as long as they are behind the same root port. This should address Bjorn's concerns that the code was to focused on being behind a single switch.* The disable ACS function now disables ACS for all bridge ports instead of switch ports (ie. those that had two
upstream_bridgeports).* Change the
pci_p2pmem_alloc_sgl()andpci_p2pmem_free_sgl()API to be more likesgl_alloc()in that the alloc function returns the allocated scatterlist and nents is not required bythe free function.* Moved the new documentation into the driver-api tree as requested by Jonathan
* Add SGL alloc and free helpers in the nvmet code so that the individual drivers can share the code that allocates P2P memory. As requested by Christoph.
* Cleanup the
nvmet_p2pmem_store()function as Christoph thought my first attempt was ugly.* Numerous commit message and comment fix-ups
Changes in v3:
* Many more fixes and minor cleanups that were spotted by Bjorn
* Additional explanation of the ACS change in both the commit message and
Kconfigdoc. Also, the code that disables the ACS bits is surrounded explicitly by an #ifdef* Removed the flag we added to
rdma_rw_ctx()in favour of usingis_pci_p2pdma_page(), as suggested by Sagi.* Adjust
pci_p2pmem_find()so that it prefers P2P providers that are closest to (or the same as) the clients using them. In cases of ties, the provider is randomly chosen.* Modify the NVMe Target code so that the PCI device name of the provider may be explicitly specified, bypassing the logic in
pci_p2pmem_find(). (Note: it's still enforced that the provider must be behind the same switch as the clients).* As requested by Bjorn, added documentation for driver writers.
Changes in v2:
* Renamed everything to 'p2pdma' per the suggestion from Bjorn as well as a bunch of cleanup and spelling fixes he pointed out in the last series.
* To address Alex's ACS concerns, we change to a simpler method of just disabling ACS behind switches for any kernel that has
CONFIG_PCI_P2PDMA.* We also reject using devices that employ '
dma_virt_ops' which should fairly simply handle Jason's concerns that this work might break with the HFI, QIB and rxe drivers that use the virtual ops to implement their own special DMA operations.--
This is a continuation of our work to enable using Peer-to-Peer PCI memory in the kernel with initial support for the NVMe fabrics target subsystem. Many thanks go to Christoph Hellwig who provided valuable feedback to get these patches to where they are today.
The concept here is to use memory that's exposed on a PCI BAR as data buffers in the NVMe target code such that data can be transferred from an RDMA NIC to the special memory and then directly to an NVMe device avoiding system memory entirely. The upside of this is better QoS for applications running on the CPU utilizing memory and lower PCI bandwidth required to the CPU (such that systems could be designed with fewer lanes connected to the CPU).
Due to these trade-offs we've designed the system to only enable using the PCI memory in cases where the NIC, NVMe devices and memory are all behind the same PCI switch hierarchy. This will mean many setups that could likely work well will not be supported so that we can be more confident it will work and not place any responsibility on the user to understand their topology. (We chose to go this route based on feedback we received at the last LSF). Future work may enable these transfers using a white list of known good root complexes. However, at this time, there is no reliable way to ensure that Peer-to-Peer transactions are permitted between PCI Root Ports.
For PCI P2P DMA transfers to work in this situation the ACS bits must be disabled on the downstream ports (DSPs) for all devices involved in the transfer. This can be done using the "
disable_acs_redir" PCI kernel command line option which was introduced in v4.19.In order to enable PCI P2P functionality, we introduce a few new PCI functions such that a driver can register P2P memory with the system. Struct pages are created for this memory using
devm_memremap_pages()and the PCI bus offset is stored in the corresponding pagemap structure.Another set of functions allow a client driver to create a list of client devices that will be used in a given P2P transactions and then use that list to find any P2P memory that is supported by all the client devices.
In the block layer, we also introduce a flag for a request queue to indicate a given queue supports targeting P2P memory. The driver submitting bios must ensure that the queue supports P2P before attempting to submit BIOs backed by P2P memory. Also, P2P requests are marked to not be merged seeing a non-homogenous request would complicate the DMA mapping requirements.
In the PCI NVMe driver, we modify the existing CMB support to utilize the new PCI P2P memory infrastructure and also add support for P2P memory in its request queue. When a P2P request is received it uses the
pci_p2pmem_map_sg()function which applies the necessary transformation to get the correntpci_bus_addr_tfor the DMA transactions.In the RDMA core, we also adjust
rdma_rw_ctx_init()andrdma_rw_ctx_destroy()to take a flags argument which indicates whether to use the PCI P2P mapping functions or not. To avoid odd RDMA devices that don't use the proper DMA infrastructure this code rejects using any device that employs thevirt_dma_opsimplementation.Finally, in the NVMe fabrics target port we introduce a new configuration attribute: 'p2pmem'. When set to a true boolean, the port will attempt to find P2P memory supported by the RDMA NIC and all namespaces. It may also be set to a PCI device name to select a specific P2P memory to use. If supported memory is found, it will be used in all IO transfers. And if a port is using P2P memory, adding new namespaces that are not supported by that memory will fail.
These patches have been tested on a number of Intel based systems and for a variety of RDMA NICs (Mellanox, Broadcomm, Chelsio) and NVMe SSDs (Intel, Seagate, Samsung) and p2pdma devices (Eideticom, Microsemi, Chelsio and Everspin) using switches from both Microsemi and Broadcomm.
--
Logan Gunthorpe (13):
PCI/P2PDMA: Support peer-to-peer memory
PCI/P2PDMA: Add sysfs group to display p2pmem stats PCI/P2PDMA: Add PCI p2pmem DMA mappings to adjust the bus offset PCI/P2PDMA: Introduce configfs/sysfs enable attribute helpers
docs-rst: Add a new directory for PCI documentation PCI/P2PDMA: Add P2P DMA driver writer's documentation
block: Add PCI P2P flag for request queue and check support for
requests
IB/core: Ensure we map P2P memory correctly inrdma_rw_ctx_[init|destroy]()nvme-pci: Use PCI p2pmem subsystem to manage the CMB
nvme-pci: Add support for P2P memory in requests
nvme-pci: Add a quirk for a pseudo CMB
nvmet: Introduce helper functions to allocate and free request SGLs
nvmet: Optionally use PCI P2P memoryDocumentation/ABI/testing/sysfs-bus-pci | 24 + Documentation/driver-api/index.rst | 2 +- Documentation/driver-api/pci/index.rst | 21 + Documentation/driver-api/pci/p2pdma.rst | 145 ++++ Documentation/driver-api/{ => pci}/pci.rst | 0 drivers/infiniband/core/rw.c | 11 +- drivers/nvme/host/core.c | 4 + drivers/nvme/host/nvme.h | 8 + drivers/nvme/host/pci.c | 121 +-- drivers/nvme/target/configfs.c | 45 ++ drivers/nvme/target/core.c | 180 +++++ drivers/nvme/target/io-cmd-bdev.c | 3 + drivers/nvme/target/nvmet.h | 17 + drivers/nvme/target/rdma.c | 22 +- drivers/pci/Kconfig | 17 + drivers/pci/Makefile | 1 + drivers/pci/p2pdma.c | 809 +++++++++++++++++++++ include/linux/blkdev.h | 3 + include/linux/memremap.h | 6 + include/linux/mm.h | 18 +
Quoted 8 lines
include/linux/pci-p2pdma.h | 114 +++
include/linux/pci.h | 4 +
22 files changed, 1521 insertions(+), 54 deletions(-)
create mode 100644 Documentation/driver-api/pci/index.rst
create mode 100644 Documentation/driver-api/pci/p2pdma.rst
rename Documentation/driver-api/{ => pci}/pci.rst (100%)
create mode 100644 drivers/pci/p2pdma.c
create mode 100644 include/linux/pci-p2pdma.hI added the reviewed-by tags from Christoph, Jens' ack on the blkdev.h change, and applied these to pci/peer-to-peer with the intent of merging these for v4.20.
I gave up on waiting for an ack for the memremap.h and mm.h changes.
I dropped the "nvme-pci: Add a quirk for a pseudo CMB" quirk because of Christoph's objection. After this is all merged, I won't need to be involved, and you and the NVMe folks can hash that out.
If there are updates to "nvmet: Optionally use PCI P2P memory" based on Sagi's comments, send an incremental patch and I'll fold them in.
Bjorn
Logan Gunthorpe
maintainerQuoted 4 lines · Bjorn Helgaas
I added the reviewed-by tags from Christoph, Jens' ack on the
blkdev.hchange, and applied these to pci/peer-to-peer with the intent of merging these for v4.20.
Quoted 8 lines
I gave up on waiting for an ack for the
memremap.handmm.hchanges.I dropped the "nvme-pci: Add a quirk for a pseudo CMB" quirk because of Christoph's objection. After this is all merged, I won't need to be involved, and you and the NVMe folks can hash that out.
If there are updates to "nvmet: Optionally use PCI P2P memory" based on Sagi's comments, send an incremental patch and I'll fold them in.
Thanks for picking this up. However, I hate to throw a wrench in the works, but I had a v10[1] queued up because kbuild found some problems with the series over the weekend. I can send v10 off right away if you want to just replace it in your branch or, if you'd like, I can generate some incremental patches. Let me know which you'd prefer.
Thanks,
Logan
[1] https://github.com/sbates130272/linux-p2pmem pci-p2p-v10
Bjorn Helgaas
AppliedQuoted 13 lines · Logan Gunthorpe
Quoted 10 lines · Bjorn Helgaas
I added the reviewed-by tags from Christoph, Jens' ack on the
blkdev.hchange, and applied these to pci/peer-to-peer with the intent of merging these for v4.20.I gave up on waiting for an ack for the
memremap.handmm.hchanges.I dropped the "nvme-pci: Add a quirk for a pseudo CMB" quirk because of Christoph's objection. After this is all merged, I won't need to be involved, and you and the NVMe folks can hash that out.
Quoted 8 lines
Quoted 2 lines
If there are updates to "nvmet: Optionally use PCI P2P memory" based on Sagi's comments, send an incremental patch and I'll fold them in.
Thanks for picking this up. However, I hate to throw a wrench in the works, but I had a v10[1] queued up because kbuild found some problems with the series over the weekend. I can send v10 off right away if you want to just replace it in your branch or, if you'd like, I can generate some incremental patches. Let me know which you'd prefer.
I applied the updates from your v10 to my pci/peer-to-peer branch.
Quoted 1 line
[1] https://github.com/sbates130272/linux-p2pmem pci-p2p-v10
Logan Gunthorpe
maintainerQuoted 1 line · Bjorn Helgaas
I applied the updates from your v10 to my pci/peer-to-peer branch.
Perfect, thanks!
Logan