Messages in this thread |  | | Subject | Re: [PATCH 12/16] nvme-pci: Check DMA ops when indicating support for PCI P2PDMA | From | John Hubbard <> | Date | Sun, 2 May 2021 18:29:29 -0700 |
| |
On 4/8/21 10:01 AM, Logan Gunthorpe wrote: > 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(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 0896e21642be..223419454516 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -3907,7 +3907,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))
This is a little excessive, as I suspected. How about providing a default .supports_pci_p2pdma routine that returns false, so that the op is always available (non-null)? By "default", maybe that means either requiring an init_the_ops_struct() routine to be used, and/or checking all the users of struct nvme_ctrl_ops.
Another idea: maybe you don't really need a bool .supports_pci_p2pdma() routine at all, because the existing .flags really is about right. You just need the flags to be filled in dynamically. So, do that during nvme_pci setup/init time: that's when this module would call dma_pci_p2pdma_supported().
Actually, I think that second idea simplifies things quite a bit, but only if it's possible. I haven't worked through the startup order of calls in nvme_pci.
thanks, -- John Hubbard NVIDIA
> blk_queue_flag_set(QUEUE_FLAG_PCI_P2PDMA, ns->queue); > > ns->queue->queuedata = ns; > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h > index 07b34175c6ce..9c04df982d2c 100644 > --- a/drivers/nvme/host/nvme.h > +++ b/drivers/nvme/host/nvme.h > @@ -473,7 +473,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); > @@ -481,6 +480,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); > }; > > #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index 7249ae74f71f..14f092973792 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -2759,17 +2759,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) >
|  |