| Message ID | 1726733624-2142-3-git-send-email-shivasharan.srikanteshwara@broadcom.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/portdrv: Report inter switch P2P links through sysfs | expand |
Hi Shivasharan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on next-20240919]
[cannot apply to pci/for-linus linus/master v6.11]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shivasharan-S/PCI-portdrv-Enable-reporting-inter-switch-P2P-links/20240919-162626
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/1726733624-2142-3-git-send-email-shivasharan.srikanteshwara%40broadcom.com
patch subject: [PATCH 2/2 v2] PCI/P2PDMA: Modify p2p_dma_distance to detect P2P links
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20240920/202409201333.LCx5k41f-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240920/202409201333.LCx5k41f-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409201333.LCx5k41f-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/pci/pcie/portdrv.c:86: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Determine if device supports Inter switch P2P links.
>> drivers/pci/pcie/portdrv.c:116: warning: Function parameter or struct member 'a' not described in 'pcie_port_is_p2p_link_available'
>> drivers/pci/pcie/portdrv.c:116: warning: Function parameter or struct member 'b' not described in 'pcie_port_is_p2p_link_available'
vim +116 drivers/pci/pcie/portdrv.c
106
107 /**
108 * pcie_port_is_p2p_link_available: Determine if a P2P link is available
109 * between the two upstream bridges. The serial number of the two devices
110 * will be compared and if they are same then it is considered that the P2P
111 * link is available.
112 *
113 * Return value: true if inter switch P2P is available, return false otherwise.
114 */
115 bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b)
> 116 {
117 u64 dsn_a, dsn_b;
118
119 /*
120 * Check if the devices support Inter switch P2P.
121 */
122 if (!pcie_port_is_p2p_supported(a) ||
123 !pcie_port_is_p2p_supported(b))
124 return false;
125
126 dsn_a = pci_get_dsn(a);
127 if (!dsn_a)
128 return false;
129
130 dsn_b = pci_get_dsn(b);
131 if (!dsn_b)
132 return false;
133
134 if (dsn_a == dsn_b)
135 return true;
136
137 return false;
138 }
139 EXPORT_SYMBOL_GPL(pcie_port_is_p2p_link_available);
140
On Thu, 19 Sep 2024 01:13:44 -0700 Shivasharan S <shivasharan.srikanteshwara@broadcom.com> wrote: > Update the p2p_dma_distance() to determine inter-switch P2P links existing > between two switches and use this to calculate the DMA distance between > two devices. > > Signed-off-by: Shivasharan S <shivasharan.srikanteshwara@broadcom.com> > --- > drivers/pci/p2pdma.c | 17 ++++++++++++++++- > drivers/pci/pcie/portdrv.c | 34 ++++++++++++++++++++++++++++++++++ > drivers/pci/pcie/portdrv.h | 2 ++ > 3 files changed, 52 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > index 4f47a13cb500..eed3b69e7293 100644 > --- a/drivers/pci/p2pdma.c > +++ b/drivers/pci/p2pdma.c > @@ -21,6 +21,8 @@ > #include <linux/seq_buf.h> > #include <linux/xarray.h> > > +extern bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b); That's nasty. Include the header so you get a clean stub if this support is not built in etc. > + > diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c > index c940b4b242fd..2fe1598fc684 100644 > --- a/drivers/pci/pcie/portdrv.c > +++ b/drivers/pci/pcie/portdrv.c > @@ -104,6 +104,40 @@ static bool pcie_port_is_p2p_supported(struct pci_dev *dev) > return false; > } > > +/** > + * pcie_port_is_p2p_link_available: Determine if a P2P link is available > + * between the two upstream bridges. The serial number of the two devices > + * will be compared and if they are same then it is considered that the P2P > + * link is available. > + * > + * Return value: true if inter switch P2P is available, return false otherwise. > + */ > +bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b) > +{ > + u64 dsn_a, dsn_b; > + > + /* > + * Check if the devices support Inter switch P2P. > + */ Single line comment syntax fine here. However it's kind of obvious, so I'd just drop the comment. > + if (!pcie_port_is_p2p_supported(a) || > + !pcie_port_is_p2p_supported(b)) Don't wrap this. I think it's under 80 chars anyway. > + return false; > + > + dsn_a = pci_get_dsn(a); > + if (!dsn_a) > + return false; If we assume that dsn is the only right way to detect this (I'm fine with that for now) then we know the supported tests above would only pass if this is true. Hence return pci_get_dsn(a) == pci_get_dsn(b); should be fine. > + > + dsn_b = pci_get_dsn(b); > + if (!dsn_b) > + return false; > + > + if (dsn_a == dsn_b) > + return true; return dsn_a == dsn_b; > + > + return false; > +} > +EXPORT_SYMBOL_GPL(pcie_port_is_p2p_link_available); > + > /* > * Traverse list of all PCI bridges and find devices that support Inter switch P2P > * and have the same serial number to create report the BDF over sysfs.
On Tue, Sep 24, 2024 at 8:38 PM Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > > On Thu, 19 Sep 2024 01:13:44 -0700 > Shivasharan S <shivasharan.srikanteshwara@broadcom.com> wrote: > > > Update the p2p_dma_distance() to determine inter-switch P2P links existing > > between two switches and use this to calculate the DMA distance between > > two devices. > > > > Signed-off-by: Shivasharan S <shivasharan.srikanteshwara@broadcom.com> > > --- > > drivers/pci/p2pdma.c | 17 ++++++++++++++++- > > drivers/pci/pcie/portdrv.c | 34 ++++++++++++++++++++++++++++++++++ > > drivers/pci/pcie/portdrv.h | 2 ++ > > 3 files changed, 52 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > > index 4f47a13cb500..eed3b69e7293 100644 > > --- a/drivers/pci/p2pdma.c > > +++ b/drivers/pci/p2pdma.c > > @@ -21,6 +21,8 @@ > > #include <linux/seq_buf.h> > > #include <linux/xarray.h> > > > > +extern bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b); > > That's nasty. Include the header so you get a clean stub if > this support is not built in etc. > Will move this to the new header file that will be added. > > + > > > diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c > > index c940b4b242fd..2fe1598fc684 100644 > > --- a/drivers/pci/pcie/portdrv.c > > +++ b/drivers/pci/pcie/portdrv.c > > @@ -104,6 +104,40 @@ static bool pcie_port_is_p2p_supported(struct pci_dev *dev) > > return false; > > } > > > > +/** > > + * pcie_port_is_p2p_link_available: Determine if a P2P link is available > > + * between the two upstream bridges. The serial number of the two devices > > + * will be compared and if they are same then it is considered that the P2P > > + * link is available. > > + * > > + * Return value: true if inter switch P2P is available, return false otherwise. > > + */ > > +bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b) > > +{ > > + u64 dsn_a, dsn_b; > > + > > + /* > > + * Check if the devices support Inter switch P2P. > > + */ > > Single line comment syntax fine here. However it's kind > of obvious, so I'd just drop the comment. > > Will do. > > + if (!pcie_port_is_p2p_supported(a) || > > + !pcie_port_is_p2p_supported(b)) > > Don't wrap this. I think it's under 80 chars anyway. > Will do. > > + return false; > > + > > + dsn_a = pci_get_dsn(a); > > + if (!dsn_a) > > + return false; > If we assume that dsn is the only right way to detect this > (I'm fine with that for now) then we know the supported tests > above would only pass if this is true. Hence > > return pci_get_dsn(a) == pci_get_dsn(b); > > should be fine. > Agreed. Will rework this as suggested and update in the next patch. > > + > > + dsn_b = pci_get_dsn(b); > > + if (!dsn_b) > > + return false; > > + > > + if (dsn_a == dsn_b) > > + return true; > > return dsn_a == dsn_b; > Above changes will take care of this as well. > > + > > + return false; > > +} > > +EXPORT_SYMBOL_GPL(pcie_port_is_p2p_link_available); > > + > > /* > > * Traverse list of all PCI bridges and find devices that support Inter switch P2P > > * and have the same serial number to create report the BDF over sysfs. > >
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index 4f47a13cb500..eed3b69e7293 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -21,6 +21,8 @@ #include <linux/seq_buf.h> #include <linux/xarray.h> +extern bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b); + struct pci_p2pdma { struct gen_pool *pool; bool p2pmem_published; @@ -576,7 +578,7 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client, int *dist, bool verbose) { enum pci_p2pdma_map_type map_type = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE; - struct pci_dev *a = provider, *b = client, *bb; + struct pci_dev *a = provider, *b = client, *bb, *b_p2p_link = NULL; bool acs_redirects = false; struct pci_p2pdma *p2pdma; struct seq_buf acs_list; @@ -606,6 +608,16 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client, if (a == bb) goto check_b_path_acs; + /* + * If both upstream bridges have Inter switch P2P link + * available, P2P DMA distance can account for optimized + * path. + */ + if (pcie_port_is_p2p_link_available(a, bb)) { + b_p2p_link = bb; + goto check_b_path_acs; + } + bb = pci_upstream_bridge(bb); dist_b++; } @@ -629,6 +641,9 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client, acs_cnt++; } + if (bb == b_p2p_link) + break; + bb = pci_upstream_bridge(bb); } diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c index c940b4b242fd..2fe1598fc684 100644 --- a/drivers/pci/pcie/portdrv.c +++ b/drivers/pci/pcie/portdrv.c @@ -104,6 +104,40 @@ static bool pcie_port_is_p2p_supported(struct pci_dev *dev) return false; } +/** + * pcie_port_is_p2p_link_available: Determine if a P2P link is available + * between the two upstream bridges. The serial number of the two devices + * will be compared and if they are same then it is considered that the P2P + * link is available. + * + * Return value: true if inter switch P2P is available, return false otherwise. + */ +bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b) +{ + u64 dsn_a, dsn_b; + + /* + * Check if the devices support Inter switch P2P. + */ + if (!pcie_port_is_p2p_supported(a) || + !pcie_port_is_p2p_supported(b)) + return false; + + dsn_a = pci_get_dsn(a); + if (!dsn_a) + return false; + + dsn_b = pci_get_dsn(b); + if (!dsn_b) + return false; + + if (dsn_a == dsn_b) + return true; + + return false; +} +EXPORT_SYMBOL_GPL(pcie_port_is_p2p_link_available); + /* * Traverse list of all PCI bridges and find devices that support Inter switch P2P * and have the same serial number to create report the BDF over sysfs. diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index 1be06cb45665..b341aad6eb49 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -130,5 +130,7 @@ static inline bool pcie_pme_no_msi(void) { return false; } static inline void pcie_pme_interrupt_enable(struct pci_dev *dev, bool en) {} #endif /* !CONFIG_PCIE_PME */ +bool pcie_port_is_p2p_link_available(struct pci_dev *a, struct pci_dev *b); + struct device *pcie_port_find_device(struct pci_dev *dev, u32 service); #endif /* _PORTDRV_H_ */
Update the p2p_dma_distance() to determine inter-switch P2P links existing between two switches and use this to calculate the DMA distance between two devices. Signed-off-by: Shivasharan S <shivasharan.srikanteshwara@broadcom.com> --- drivers/pci/p2pdma.c | 17 ++++++++++++++++- drivers/pci/pcie/portdrv.c | 34 ++++++++++++++++++++++++++++++++++ drivers/pci/pcie/portdrv.h | 2 ++ 3 files changed, 52 insertions(+), 1 deletion(-)