All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: logang@deltatee.com, helgaas@kernel.org
Cc: linux-pci@vger.kernel.org
Subject: [PATCH] PCI/P2PDMA: simplify distance calculation
Date: Mon, 14 Jun 2021 07:53:10 +0200	[thread overview]
Message-ID: <20210614055310.3960791-1-hch@lst.de> (raw)

Merge __calc_map_type_and_dist and calc_map_type_and_dist_warn into
calc_map_type_and_dist to simplify the code a bit.  This now means
we add the devfn strings to the acs_buf unconditionallity even if
the buffer is not printed, but that is not a lot of overhead and
keeps the code much simpler.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/p2pdma.c | 190 +++++++++++++++++--------------------------
 1 file changed, 73 insertions(+), 117 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index deb097ceaf41..ca2574debb2d 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -388,79 +388,6 @@ static bool host_bridge_whitelist(struct pci_dev *a, struct pci_dev *b,
 	return false;
 }
 
-static enum pci_p2pdma_map_type
-__calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
-		int *dist, bool *acs_redirects, struct seq_buf *acs_list)
-{
-	struct pci_dev *a = provider, *b = client, *bb;
-	int dist_a = 0;
-	int dist_b = 0;
-	int acs_cnt = 0;
-
-	if (acs_redirects)
-		*acs_redirects = false;
-
-	/*
-	 * Note, we don't need to take references to devices returned by
-	 * pci_upstream_bridge() seeing we hold a reference to a child
-	 * device which will already hold a reference to the upstream bridge.
-	 */
-
-	while (a) {
-		dist_b = 0;
-
-		if (pci_bridge_has_acs_redir(a)) {
-			seq_buf_print_bus_devfn(acs_list, a);
-			acs_cnt++;
-		}
-
-		bb = b;
-
-		while (bb) {
-			if (a == bb)
-				goto check_b_path_acs;
-
-			bb = pci_upstream_bridge(bb);
-			dist_b++;
-		}
-
-		a = pci_upstream_bridge(a);
-		dist_a++;
-	}
-
-	if (dist)
-		*dist = dist_a + dist_b;
-
-	return PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
-
-check_b_path_acs:
-	bb = b;
-
-	while (bb) {
-		if (a == bb)
-			break;
-
-		if (pci_bridge_has_acs_redir(bb)) {
-			seq_buf_print_bus_devfn(acs_list, bb);
-			acs_cnt++;
-		}
-
-		bb = pci_upstream_bridge(bb);
-	}
-
-	if (dist)
-		*dist = dist_a + dist_b;
-
-	if (acs_cnt) {
-		if (acs_redirects)
-			*acs_redirects = true;
-
-		return PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
-	}
-
-	return PCI_P2PDMA_MAP_BUS_ADDR;
-}
-
 static unsigned long map_types_idx(struct pci_dev *client)
 {
 	return (pci_domain_nr(client->bus) << 16) |
@@ -502,63 +429,96 @@ static unsigned long map_types_idx(struct pci_dev *client)
  * PCI_P2PDMA_MAP_THRU_HOST_BRIDGE with the distance set to the number of
  * ports per above. If the device is not in the whitelist, return
  * PCI_P2PDMA_MAP_NOT_SUPPORTED.
- *
- * If any ACS redirect bits are set, then acs_redirects boolean will be set
- * to true and their PCI device names will be appended to the acs_list
- * seq_buf. This seq_buf is used to print a warning informing the user how
- * to disable ACS using a command line parameter.  (See
- * calc_map_type_and_dist_warn() below)
  */
 static enum pci_p2pdma_map_type
 calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
-		int *dist, bool *acs_redirects, struct seq_buf *acs_list)
+		int *dist, bool verbose)
 {
-	enum pci_p2pdma_map_type map_type;
+	enum pci_p2pdma_map_type map_type = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
+	struct pci_dev *a = provider, *b = client, *bb;
+	bool acs_redirects = false;
+	struct seq_buf acs_list;
+	int acs_cnt = 0;
+	int dist_a = 0;
+	int dist_b = 0;
+	char buf[128];
+
+	seq_buf_init(&acs_list, buf, sizeof(buf));
+
+	/*
+	 * Note, we don't need to take references to devices returned by
+	 * pci_upstream_bridge() seeing we hold a reference to a child
+	 * device which will already hold a reference to the upstream bridge.
+	 */
+	while (a) {
+		dist_b = 0;
 
-	map_type = __calc_map_type_and_dist(provider, client, dist,
-					    acs_redirects, acs_list);
+		if (pci_bridge_has_acs_redir(a)) {
+			seq_buf_print_bus_devfn(&acs_list, a);
+			acs_cnt++;
+		}
 
-	if (map_type == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE) {
-		if (!cpu_supports_p2pdma() &&
-		    !host_bridge_whitelist(provider, client, acs_redirects))
-			map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
+		bb = b;
+
+		while (bb) {
+			if (a == bb)
+				goto check_b_path_acs;
+
+			bb = pci_upstream_bridge(bb);
+			dist_b++;
+		}
+
+		a = pci_upstream_bridge(a);
+		dist_a++;
 	}
 
-	if (provider->p2pdma)
-		xa_store(&provider->p2pdma->map_types, map_types_idx(client),
-			 xa_mk_value(map_type), GFP_KERNEL);
+	*dist = dist_a + dist_b;
+	goto map_through_host_bridge;
 
-	return map_type;
-}
+check_b_path_acs:
+	bb = b;
 
-static enum pci_p2pdma_map_type
-calc_map_type_and_dist_warn(struct pci_dev *provider, struct pci_dev *client,
-			    int *dist)
-{
-	struct seq_buf acs_list;
-	bool acs_redirects;
-	char buf[128];
-	int ret;
+	while (bb) {
+		if (a == bb)
+			break;
 
-	seq_buf_init(&acs_list, buf, sizeof(buf));
+		if (pci_bridge_has_acs_redir(bb)) {
+			seq_buf_print_bus_devfn(&acs_list, bb);
+			acs_cnt++;
+		}
 
-	ret = calc_map_type_and_dist(provider, client, dist, &acs_redirects,
-				     &acs_list);
-	if (acs_redirects) {
+		bb = pci_upstream_bridge(bb);
+	}
+
+	*dist = dist_a + dist_b;
+
+	if (!acs_cnt) {
+		map_type = PCI_P2PDMA_MAP_BUS_ADDR;
+		goto done;
+	}
+
+	if (verbose) {
+		acs_list.buffer[acs_list.len-1] = 0; /* drop final semicolon */
 		pci_warn(client, "ACS redirect is set between the client and provider (%s)\n",
 			 pci_name(provider));
-		/* Drop final semicolon */
-		acs_list.buffer[acs_list.len-1] = 0;
 		pci_warn(client, "to disable ACS redirect for this path, add the kernel parameter: pci=disable_acs_redir=%s\n",
 			 acs_list.buffer);
 	}
+	acs_redirects = true;
 
-	if (ret == PCI_P2PDMA_MAP_NOT_SUPPORTED) {
-		pci_warn(client, "cannot be used for peer-to-peer DMA as the client and provider (%s) do not share an upstream bridge or whitelisted host bridge\n",
-			 pci_name(provider));
+map_through_host_bridge:
+	if (!cpu_supports_p2pdma() &&
+	    !host_bridge_whitelist(provider, client, acs_redirects)) {
+		if (verbose)
+			pci_warn(client, "cannot be used for peer-to-peer DMA as the client and provider (%s) do not share an upstream bridge or whitelisted host bridge\n",
+				 pci_name(provider));
+		map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
 	}
-
-	return ret;
+done:
+	if (provider->p2pdma)
+		xa_store(&provider->p2pdma->map_types, map_types_idx(client),
+			 xa_mk_value(map_type), GFP_KERNEL);
+	return map_type;
 }
 
 /**
@@ -599,12 +559,8 @@ int pci_p2pdma_distance_many(struct pci_dev *provider, struct device **clients,
 			return -1;
 		}
 
-		if (verbose)
-			map = calc_map_type_and_dist_warn(provider, pci_client,
-							  &distance);
-		else
-			map = calc_map_type_and_dist(provider, pci_client,
-						     &distance, NULL, NULL);
+		map = calc_map_type_and_dist(provider, pci_client, &distance,
+					     verbose);
 
 		pci_dev_put(pci_client);
 
-- 
2.30.2


             reply	other threads:[~2021-06-14  5:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14  5:53 Christoph Hellwig [this message]
2021-06-14 16:59 ` [PATCH] PCI/P2PDMA: simplify distance calculation Logan Gunthorpe
2021-06-16 21:11 ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210614055310.3960791-1-hch@lst.de \
    --to=hch@lst.de \
    --cc=helgaas@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=logang@deltatee.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.