| Message ID | 20210916234100.122368-10-logang@deltatee.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Userspace P2PDMA with O_DIRECT NVMe devices | expand |
Logan, > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 7efb31b87f37..916750a54f60 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -3771,7 +3771,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, > blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, ns->queue); > > blk_queue_flag_set(QUEUE_FLAG_NONROT, ns->queue); > - if (ctrl->ops->flags & NVME_F_PCI_P2PDMA) > + if (ctrl->ops->supports_pci_p2pdma && > + ctrl->ops->supports_pci_p2pdma(ctrl)) > blk_queue_flag_set(QUEUE_FLAG_PCI_P2PDMA, ns->queue); > > ns->ctrl = ctrl; > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h > index 9871c0c9374c..fb9bfc52a6d7 100644 > --- a/drivers/nvme/host/nvme.h > +++ b/drivers/nvme/host/nvme.h > @@ -477,7 +477,6 @@ 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); > @@ -485,6 +484,7 @@ struct nvme_ctrl_ops { > void (*submit_async_event)(struct nvme_ctrl *ctrl); > void (*delete_ctrl)(struct nvme_ctrl *ctrl); > int (*get_address)(struct nvme_ctrl *ctrl, char *buf, int size); > + bool (*supports_pci_p2pdma)(struct nvme_ctrl *ctrl); > }; > Is this new ops only needed for the PCIe transport ? or do you have following patches to use this op for the other transports ? If it is only needed for the PCIe then we need to find a way to not add this somehow...
On 2021-09-29 11:06 p.m., Chaitanya Kulkarni wrote: > Logan, > >> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c >> index 7efb31b87f37..916750a54f60 100644 >> --- a/drivers/nvme/host/core.c >> +++ b/drivers/nvme/host/core.c >> @@ -3771,7 +3771,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, >> blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, ns->queue); >> >> blk_queue_flag_set(QUEUE_FLAG_NONROT, ns->queue); >> - if (ctrl->ops->flags & NVME_F_PCI_P2PDMA) >> + if (ctrl->ops->supports_pci_p2pdma && >> + ctrl->ops->supports_pci_p2pdma(ctrl)) >> blk_queue_flag_set(QUEUE_FLAG_PCI_P2PDMA, ns->queue); >> >> ns->ctrl = ctrl; >> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h >> index 9871c0c9374c..fb9bfc52a6d7 100644 >> --- a/drivers/nvme/host/nvme.h >> +++ b/drivers/nvme/host/nvme.h >> @@ -477,7 +477,6 @@ 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); >> @@ -485,6 +484,7 @@ struct nvme_ctrl_ops { >> void (*submit_async_event)(struct nvme_ctrl *ctrl); >> void (*delete_ctrl)(struct nvme_ctrl *ctrl); >> int (*get_address)(struct nvme_ctrl *ctrl, char *buf, int size); >> + bool (*supports_pci_p2pdma)(struct nvme_ctrl *ctrl); >> }; >> > > Is this new ops only needed for the PCIe transport ? or do you have > following patches to use this op for the other transports ? No, I don't think this will make sense for transports that are not based on PCI devices. > If it is only needed for the PCIe then we need to find a way to > not add this somehow... I don't see how we can do that. The core code needs to know whether the transport supports this and must have an operation to query it. Logan
>> >> Is this new ops only needed for the PCIe transport ? or do you have >> following patches to use this op for the other transports ? > > No, I don't think this will make sense for transports that are not based > on PCI devices. > >> If it is only needed for the PCIe then we need to find a way to >> not add this somehow... > > I don't see how we can do that. The core code needs to know whether the > transport supports this and must have an operation to query it. > Okay. > Logan >
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 7efb31b87f37..916750a54f60 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -3771,7 +3771,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, ns->queue); blk_queue_flag_set(QUEUE_FLAG_NONROT, ns->queue); - if (ctrl->ops->flags & NVME_F_PCI_P2PDMA) + if (ctrl->ops->supports_pci_p2pdma && + ctrl->ops->supports_pci_p2pdma(ctrl)) blk_queue_flag_set(QUEUE_FLAG_PCI_P2PDMA, ns->queue); ns->ctrl = ctrl; diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 9871c0c9374c..fb9bfc52a6d7 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -477,7 +477,6 @@ 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); @@ -485,6 +484,7 @@ struct nvme_ctrl_ops { void (*submit_async_event)(struct nvme_ctrl *ctrl); void (*delete_ctrl)(struct nvme_ctrl *ctrl); int (*get_address)(struct nvme_ctrl *ctrl, char *buf, int size); + bool (*supports_pci_p2pdma)(struct nvme_ctrl *ctrl); }; /* diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index b82492cd7503..7d1ef66eac2e 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2874,17 +2874,24 @@ static int nvme_pci_get_address(struct nvme_ctrl *ctrl, char *buf, int size) return snprintf(buf, size, "%s\n", dev_name(&pdev->dev)); } +static bool nvme_pci_supports_pci_p2pdma(struct nvme_ctrl *ctrl) +{ + struct nvme_dev *dev = to_nvme_dev(ctrl); + + return dma_pci_p2pdma_supported(dev->dev); +} + static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = { .name = "pcie", .module = THIS_MODULE, - .flags = NVME_F_METADATA_SUPPORTED | - NVME_F_PCI_P2PDMA, + .flags = NVME_F_METADATA_SUPPORTED, .reg_read32 = nvme_pci_reg_read32, .reg_write32 = nvme_pci_reg_write32, .reg_read64 = nvme_pci_reg_read64, .free_ctrl = nvme_pci_free_ctrl, .submit_async_event = nvme_pci_submit_async_event, .get_address = nvme_pci_get_address, + .supports_pci_p2pdma = nvme_pci_supports_pci_p2pdma, }; static int nvme_dev_map(struct nvme_dev *dev)
Introduce a supports_pci_p2pdma() operation in nvme_ctrl_ops to replace the fixed NVME_F_PCI_P2PDMA flag such that the dma_map_ops flags can be checked for PCI P2PDMA support. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> --- drivers/nvme/host/core.c | 3 ++- drivers/nvme/host/nvme.h | 2 +- drivers/nvme/host/pci.c | 11 +++++++++-- 3 files changed, 12 insertions(+), 4 deletions(-)