[PATCH 3/7] cxl/region: Hold memdev lock during region poison injection/clear

[Li Ming]

cxl_dpa_to_region() will require callers holding the given CXL memdev
lock for endpoint validity checking in the following patch. To prepare
it, region poison injection/clearing debugfs interfaces need to ensure
the correct CXL memdev lock is held at the beginning.

To keep lock sequence(cxlmd.dev -> cxl_rwsem.region -> cxl_rwsem.dpa)
for avoiding deadlock. the interfaces have to find out the correct CXL
memdev at first, holding lock in the sequence then checking if the DPA
data has been changed before holding locks.

Suggested-by: Dan Williams <dan.j.williams@xxxxxxxxx>
Signed-off-by: Li Ming <ming.li@xxxxxxxxxxxx>
---
 drivers/cxl/core/region.c | 101 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 77 insertions(+), 24 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 42874948b589..806512dfab07 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -4074,12 +4074,60 @@ static int validate_region_offset(struct cxl_region *cxlr, u64 offset)
 	return 0;
 }
 
+static int cxl_region_poison_lookup_locked(struct cxl_region *cxlr, u64 offset,
+					   struct dpa_result *res)
+{
+	int rc;
+
+	*res = (struct dpa_result){ .dpa = ULLONG_MAX, .cxlmd = NULL };
+
+	if (validate_region_offset(cxlr, offset))
+		return -EINVAL;
+
+	offset -= cxlr->params.cache_size;
+	rc = region_offset_to_dpa_result(cxlr, offset, res);
+	if (rc || !res->cxlmd || res->dpa == ULLONG_MAX) {
+		dev_dbg(&cxlr->dev,
+			"Failed to resolve DPA for region offset %#llx rc %d\n",
+			offset, rc);
+
+		return rc ? rc : -EINVAL;
+	}
+
+	return 0;
+}
+
+static int cxl_region_poison_lookup(struct cxl_region *cxlr, u64 offset,
+				    struct dpa_result *res)
+{
+	int rc;
+
+	ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
+	if ((rc = ACQUIRE_ERR(rwsem_read_intr, &region_rwsem)))
+		return rc;
+
+	ACQUIRE(rwsem_read_intr, dpa_rwsem)(&cxl_rwsem.dpa);
+	if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem)))
+		return rc;
+
+	return cxl_region_poison_lookup_locked(cxlr, offset, res);
+}
+
 static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
 {
-	struct dpa_result result = { .dpa = ULLONG_MAX, .cxlmd = NULL };
 	struct cxl_region *cxlr = data;
+	struct dpa_result res1, res2;
 	int rc;
 
+	/* To retrieve the correct memdev */
+	rc = cxl_region_poison_lookup(cxlr, offset, &res1);
+	if (rc)
+		return rc;
+
+	ACQUIRE(device_intr, devlock)(&res1.cxlmd->dev);
+	if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
+		return rc;
+
 	ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
 	if ((rc = ACQUIRE_ERR(rwsem_read_intr, &region_rwsem)))
 		return rc;
@@ -4088,20 +4136,18 @@ static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
 	if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem)))
 		return rc;
 
-	if (validate_region_offset(cxlr, offset))
-		return -EINVAL;
-
-	offset -= cxlr->params.cache_size;
-	rc = region_offset_to_dpa_result(cxlr, offset, &result);
-	if (rc || !result.cxlmd || result.dpa == ULLONG_MAX) {
+	/*
+	 * Retrieve memdev and DPA data again in case that the data
+	 * has been changed before holding locks.
+	 */
+	rc = cxl_region_poison_lookup_locked(cxlr, offset, &res2);
+	if (rc || res2.cxlmd != res1.cxlmd || res2.dpa != res1.dpa) {
 		dev_dbg(&cxlr->dev,
-			"Failed to resolve DPA for region offset %#llx rc %d\n",
-			offset, rc);
-
-		return rc ? rc : -EINVAL;
+			"Error injection raced region reconfiguration: %d", rc);
+		return -ENXIO;
 	}
 
-	return cxl_inject_poison_locked(result.cxlmd, result.dpa);
+	return cxl_inject_poison_locked(res2.cxlmd, res2.dpa);
 }
 
 DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL,
@@ -4109,10 +4155,19 @@ DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL,
 
 static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
 {
-	struct dpa_result result = { .dpa = ULLONG_MAX, .cxlmd = NULL };
 	struct cxl_region *cxlr = data;
+	struct dpa_result res1, res2;
 	int rc;
 
+	/* To retrieve the correct memdev */
+	rc = cxl_region_poison_lookup(cxlr, offset, &res1);
+	if (rc)
+		return rc;
+
+	ACQUIRE(device_intr, devlock)(&res1.cxlmd->dev);
+	if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
+		return rc;
+
 	ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
 	if ((rc = ACQUIRE_ERR(rwsem_read_intr, &region_rwsem)))
 		return rc;
@@ -4121,20 +4176,18 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
 	if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem)))
 		return rc;
 
-	if (validate_region_offset(cxlr, offset))
-		return -EINVAL;
-
-	offset -= cxlr->params.cache_size;
-	rc = region_offset_to_dpa_result(cxlr, offset, &result);
-	if (rc || !result.cxlmd || result.dpa == ULLONG_MAX) {
+	/*
+	 * Retrieve memdev and DPA data again in case that the data
+	 * has been changed before holding locks.
+	 */
+	rc = cxl_region_poison_lookup_locked(cxlr, offset, &res2);
+	if (rc || res2.cxlmd != res1.cxlmd || res2.dpa != res1.dpa) {
 		dev_dbg(&cxlr->dev,
-			"Failed to resolve DPA for region offset %#llx rc %d\n",
-			offset, rc);
-
-		return rc ? rc : -EINVAL;
+			"Error clearing raced region reconfiguration: %d", rc);
+		return -ENXIO;
 	}
 
-	return cxl_clear_poison_locked(result.cxlmd, result.dpa);
+	return cxl_clear_poison_locked(res2.cxlmd, res2.dpa);
 }
 
 DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,

-- 
2.43.0