All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shuotao Xu <xushuotao@gmail.com>
To: amd-gfx@lists.freedesktop.org
Cc: Mukul Joshi <mukul.joshi@amd.com>,
	Andrey.Grodzovsky@amd.com, Felix.Kuehling@amd.com,
	pengc@microsoft.com, Lei.Qu@microsoft.com,
	Shuotao Xu <shuotaoxu@microsoft.com>,
	Ran.Shu@microsoft.com, Ziyue.Yang@microsoft.com
Subject: [PATCH 1/2] drm/amdkfd: Cleanup IO links during KFD device removal
Date: Fri,  8 Apr 2022 08:45:43 +0000	[thread overview]
Message-ID: <20220408084544.9313-1-shuotaoxu@microsoft.com> (raw)

Currently, the IO-links to the device being removed from topology,
are not cleared. As a result, there would be dangling links left in
the KFD topology. This patch aims to fix the following:
1. Cleanup all IO links to the device being removed.
2. Ensure that node numbering in sysfs and nodes proximity domain
   values are consistent after the device is removed:
   a. Adding a device and removing a GPU device are made mutually
      exclusive.
   b. The global proximity domain counter is no longer required to be
      an atomic counter. A normal 32-bit counter can be used instead.
3. Update generation_count to let user-mode know that topology has
   changed due to device removal.

Reviewed-by: Shuotao Xu <shuotaoxu@microsoft.com>
CC: Shuotao Xu <shuotaoxu@microsoft.com>
Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c     |  4 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h     |  2 +
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 79 ++++++++++++++++++++---
 3 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index 1eaabd2cb41b..afc8a7fcdad8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -1056,7 +1056,7 @@ static int kfd_parse_subtype_iolink(struct crat_subtype_iolink *iolink,
 	 * table, add corresponded reversed direction link now.
 	 */
 	if (props && (iolink->flags & CRAT_IOLINK_FLAGS_BI_DIRECTIONAL)) {
-		to_dev = kfd_topology_device_by_proximity_domain(id_to);
+		to_dev = kfd_topology_device_by_proximity_domain_no_lock(id_to);
 		if (!to_dev)
 			return -ENODEV;
 		/* same everything but the other direction */
@@ -2225,7 +2225,7 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
 	 */
 	if (kdev->hive_id) {
 		for (nid = 0; nid < proximity_domain; ++nid) {
-			peer_dev = kfd_topology_device_by_proximity_domain(nid);
+			peer_dev = kfd_topology_device_by_proximity_domain_no_lock(nid);
 			if (!peer_dev->gpu)
 				continue;
 			if (peer_dev->gpu->hive_id != kdev->hive_id)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index e1b7e6afa920..8a43def1f638 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1016,6 +1016,8 @@ int kfd_topology_add_device(struct kfd_dev *gpu);
 int kfd_topology_remove_device(struct kfd_dev *gpu);
 struct kfd_topology_device *kfd_topology_device_by_proximity_domain(
 						uint32_t proximity_domain);
+struct kfd_topology_device *kfd_topology_device_by_proximity_domain_no_lock(
+						uint32_t proximity_domain);
 struct kfd_topology_device *kfd_topology_device_by_id(uint32_t gpu_id);
 struct kfd_dev *kfd_device_by_id(uint32_t gpu_id);
 struct kfd_dev *kfd_device_by_pci_dev(const struct pci_dev *pdev);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 3bdcae239bc0..874a273b81f7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -46,27 +46,38 @@ static struct list_head topology_device_list;
 static struct kfd_system_properties sys_props;
 
 static DECLARE_RWSEM(topology_lock);
-static atomic_t topology_crat_proximity_domain;
+static uint32_t topology_crat_proximity_domain;
 
-struct kfd_topology_device *kfd_topology_device_by_proximity_domain(
+struct kfd_topology_device *kfd_topology_device_by_proximity_domain_no_lock(
 						uint32_t proximity_domain)
 {
 	struct kfd_topology_device *top_dev;
 	struct kfd_topology_device *device = NULL;
 
-	down_read(&topology_lock);
-
 	list_for_each_entry(top_dev, &topology_device_list, list)
 		if (top_dev->proximity_domain == proximity_domain) {
 			device = top_dev;
 			break;
 		}
 
+	return device;
+}
+
+struct kfd_topology_device *kfd_topology_device_by_proximity_domain(
+						uint32_t proximity_domain)
+{
+	struct kfd_topology_device *device = NULL;
+
+	down_read(&topology_lock);
+
+	device = kfd_topology_device_by_proximity_domain_no_lock(
+							proximity_domain);
 	up_read(&topology_lock);
 
 	return device;
 }
 
+
 struct kfd_topology_device *kfd_topology_device_by_id(uint32_t gpu_id)
 {
 	struct kfd_topology_device *top_dev = NULL;
@@ -1060,7 +1071,7 @@ int kfd_topology_init(void)
 	down_write(&topology_lock);
 	kfd_topology_update_device_list(&temp_topology_device_list,
 					&topology_device_list);
-	atomic_set(&topology_crat_proximity_domain, sys_props.num_devices-1);
+	topology_crat_proximity_domain = sys_props.num_devices-1;
 	ret = kfd_topology_update_sysfs();
 	up_write(&topology_lock);
 
@@ -1295,8 +1306,6 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
 
 	pr_debug("Adding new GPU (ID: 0x%x) to topology\n", gpu_id);
 
-	proximity_domain = atomic_inc_return(&topology_crat_proximity_domain);
-
 	/* Include the CPU in xGMI hive if xGMI connected by assigning it the hive ID. */
 	if (gpu->hive_id && gpu->adev->gmc.xgmi.connected_to_cpu) {
 		struct kfd_topology_device *top_dev;
@@ -1321,12 +1330,16 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
 	 */
 	dev = kfd_assign_gpu(gpu);
 	if (!dev) {
+		down_write(&topology_lock);
+		proximity_domain = ++topology_crat_proximity_domain;
+
 		res = kfd_create_crat_image_virtual(&crat_image, &image_size,
 						    COMPUTE_UNIT_GPU, gpu,
 						    proximity_domain);
 		if (res) {
 			pr_err("Error creating VCRAT for GPU (ID: 0x%x)\n",
 			       gpu_id);
+			topology_crat_proximity_domain--;
 			return res;
 		}
 		res = kfd_parse_crat_table(crat_image,
@@ -1335,10 +1348,10 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
 		if (res) {
 			pr_err("Error parsing VCRAT for GPU (ID: 0x%x)\n",
 			       gpu_id);
+			topology_crat_proximity_domain--;
 			goto err;
 		}
 
-		down_write(&topology_lock);
 		kfd_topology_update_device_list(&temp_topology_device_list,
 			&topology_device_list);
 
@@ -1485,25 +1498,73 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
 	return res;
 }
 
+static void kfd_topology_update_io_links(int proximity_domain)
+{
+	struct kfd_topology_device *dev;
+	struct kfd_iolink_properties *iolink, *p2plink, *tmp;
+	/*
+	 * The topology list currently is arranged in increasing
+	 * order of proximity domain.
+	 *
+	 * Two things need to be done when a device is removed:
+	 * 1. All the IO links to this device need to be
+	 *    removed.
+	 * 2. All nodes after the current device node need to move
+	 *    up once this device node is removed from the topology
+	 *    list. As a result, the proximity domain values for
+	 *    all nodes after the node being deleted reduce by 1.
+	 *    This would also cause the proximity domain values for
+	 *    io links to be updated based on new proximity
+	 *    domain values.
+	 */
+	list_for_each_entry(dev, &topology_device_list, list) {
+		if (dev->proximity_domain > proximity_domain)
+			dev->proximity_domain--;
+
+		list_for_each_entry_safe(iolink, tmp, &dev->io_link_props, list) {
+			/*
+			 * If there is an io link to the dev being deleted
+			 * then remove that IO link also.
+			 */
+			if (iolink->node_to == proximity_domain) {
+				list_del(&iolink->list);
+				dev->io_link_count--;
+				dev->node_props.io_links_count--;
+			} else if (iolink->node_from > proximity_domain) {
+				iolink->node_from--;
+			} else if (iolink->node_to > proximity_domain) {
+				iolink->node_to--;
+			}
+		}
+
+	}
+}
+
 int kfd_topology_remove_device(struct kfd_dev *gpu)
 {
 	struct kfd_topology_device *dev, *tmp;
 	uint32_t gpu_id;
 	int res = -ENODEV;
+	int i = 0;
 
 	down_write(&topology_lock);
 
-	list_for_each_entry_safe(dev, tmp, &topology_device_list, list)
+	list_for_each_entry_safe(dev, tmp, &topology_device_list, list) {
 		if (dev->gpu == gpu) {
 			gpu_id = dev->gpu_id;
 			kfd_remove_sysfs_node_entry(dev);
 			kfd_release_topology_device(dev);
 			sys_props.num_devices--;
+			kfd_topology_update_io_links(i);
+			topology_crat_proximity_domain = sys_props.num_devices-1;
+			sys_props.generation_count++;
 			res = 0;
 			if (kfd_topology_update_sysfs() < 0)
 				kfd_topology_release_sysfs();
 			break;
 		}
+		i++;
+	}
 
 	up_write(&topology_lock);
 
-- 
2.25.1


             reply	other threads:[~2022-04-08 13:50 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-08  8:45 Shuotao Xu [this message]
2022-04-08  8:45 ` [PATCH 2/2] drm/amdkfd: Add PCIe Hotplug Support for AMDKFD Shuotao Xu
2022-04-08 15:28   ` Andrey Grodzovsky
2022-04-09  1:28     ` [EXTERNAL] " Shuotao Xu
2022-04-11 15:52       ` Andrey Grodzovsky
2022-04-13 16:03         ` Shuotao Xu
2022-04-13 17:31           ` Andrey Grodzovsky
2022-04-14 14:00             ` Shuotao Xu
2022-04-14 14:24               ` Shuotao Xu
2022-04-14 15:13               ` Andrey Grodzovsky
2022-04-15 10:12                 ` Shuotao Xu
2022-04-15 16:43                   ` Andrey Grodzovsky
2022-04-18 13:22                     ` Shuotao Xu
2022-04-18 15:23                       ` Andrey Grodzovsky
2022-04-19  7:41                         ` Shuotao Xu
2022-04-19 16:01                           ` Andrey Grodzovsky
2022-04-19 16:18                             ` Felix Kuehling
2022-04-20  9:24                             ` Shuotao Xu
2022-04-20 15:44                               ` Andrey Grodzovsky
2022-04-20 18:41                                 ` Andrey Grodzovsky
2022-04-27  9:20                                   ` Shuotao Xu
2022-04-27 16:04                                     ` Andrey Grodzovsky
2022-05-10 11:03                                       ` Shuotao Xu
2022-05-10 16:34                                         ` Andrey Grodzovsky
2022-05-10 20:31                                         ` Felix Kuehling
2022-05-11  3:35                                           ` Shuotao Xu
2022-05-11 13:49                                             ` Andrey Grodzovsky
2022-05-11 16:49                                               ` Felix Kuehling
2022-05-11 17:02                                                 ` Andrey Grodzovsky
2022-04-12  0:07 ` [PATCH 1/2] drm/amdkfd: Cleanup IO links during KFD device removal Felix Kuehling
2022-04-12  1:38   ` [EXTERNAL] " Shuotao Xu

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=20220408084544.9313-1-shuotaoxu@microsoft.com \
    --to=xushuotao@gmail.com \
    --cc=Andrey.Grodzovsky@amd.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=Lei.Qu@microsoft.com \
    --cc=Ran.Shu@microsoft.com \
    --cc=Ziyue.Yang@microsoft.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=mukul.joshi@amd.com \
    --cc=pengc@microsoft.com \
    --cc=shuotaoxu@microsoft.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.