[PATCH v2 0/7] Use guard() instead of rwsem locking

[PATCH v2 0/7] Use guard() instead of rwsem locking

[Li Ming]

Use scoped resource management to replace open-coded locking operation
is recommended. CXL subsystem still remains some down_read()/up_read()
and down_write()/up_write() which can be replaced by guard() simply.

This patchset includes simply using guard() instead of some
down_read()/up_read() and down_write()/up_write() cases. Besides, it
also includes some function code cleanup after using guard().

base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 (tag: v6.14-rc2)

v2:
- Drop some local variables. (Jonathan)
- Rename __construct_region() to construct_auto_region(). (Jonathan and Dave)

Li Ming (7):
  cxl/core: Use guard() to replace open-coded down_read/write()
  cxl/core: cxl_mem_sanitize() cleanup
  cxl/memdev: cxl_memdev_ioctl() cleanup
  cxl/core: Use guard() to drop the goto pattern of cxl_dpa_free()
  cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()
  cxl/region: Drop goto pattern in cxl_dax_region_alloc()
  cxl/region: Drop goto pattern of construct_region()

 drivers/cxl/core/hdm.c    | 68 +++++++++++-----------------
 drivers/cxl/core/mbox.c   | 10 ++---
 drivers/cxl/core/memdev.c | 17 +++----
 drivers/cxl/core/port.c   |  8 +---
 drivers/cxl/core/region.c | 95 +++++++++++++++++++--------------------
 5 files changed, 85 insertions(+), 113 deletions(-)

-- 
2.34.1

[PATCH v2 1/7] cxl/core: Use guard() to replace open-coded down_read/write()

[Li Ming]

Some down/up_read() and down/up_write() cases can be replaced by a
guard() simply to drop explicit unlock invoked. It helps to align coding
style with current CXL subsystem's.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
v2:
- Drop some local variables. (Jonathan)
---
 drivers/cxl/core/hdm.c    | 15 +++++----------
 drivers/cxl/core/memdev.c |  9 +++------
 drivers/cxl/core/port.c   |  8 ++------
 drivers/cxl/core/region.c |  8 +++-----
 4 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 50e6a45b30ba..4a578092377e 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -213,13 +213,12 @@ void cxl_dpa_debug(struct seq_file *file, struct cxl_dev_state *cxlds)
 {
 	struct resource *p1, *p2;
 
-	down_read(&cxl_dpa_rwsem);
+	guard(rwsem_read)(&cxl_dpa_rwsem);
 	for (p1 = cxlds->dpa_res.child; p1; p1 = p1->sibling) {
 		__cxl_dpa_debug(file, p1, 0);
 		for (p2 = p1->child; p2; p2 = p2->sibling)
 			__cxl_dpa_debug(file, p2, 1);
 	}
-	up_read(&cxl_dpa_rwsem);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_dpa_debug, "CXL");
 
@@ -250,9 +249,8 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled)
 
 static void cxl_dpa_release(void *cxled)
 {
-	down_write(&cxl_dpa_rwsem);
+	guard(rwsem_write)(&cxl_dpa_rwsem);
 	__cxl_dpa_release(cxled);
-	up_write(&cxl_dpa_rwsem);
 }
 
 /*
@@ -362,14 +360,11 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_dpa_reserve, "CXL");
 
 resource_size_t cxl_dpa_size(struct cxl_endpoint_decoder *cxled)
 {
-	resource_size_t size = 0;
-
-	down_read(&cxl_dpa_rwsem);
+	guard(rwsem_read)(&cxl_dpa_rwsem);
 	if (cxled->dpa_res)
-		size = resource_size(cxled->dpa_res);
-	up_read(&cxl_dpa_rwsem);
+		return resource_size(cxled->dpa_res);
 
-	return size;
+	return 0;
 }
 
 resource_size_t cxl_dpa_resource_start(struct cxl_endpoint_decoder *cxled)
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index ae3dfcbe8938..98c05426aa4a 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -564,10 +564,9 @@ EXPORT_SYMBOL_NS_GPL(is_cxl_memdev, "CXL");
 void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
 				unsigned long *cmds)
 {
-	down_write(&cxl_memdev_rwsem);
+	guard(rwsem_write)(&cxl_memdev_rwsem);
 	bitmap_or(mds->exclusive_cmds, mds->exclusive_cmds, cmds,
 		  CXL_MEM_COMMAND_ID_MAX);
-	up_write(&cxl_memdev_rwsem);
 }
 EXPORT_SYMBOL_NS_GPL(set_exclusive_cxl_commands, "CXL");
 
@@ -579,10 +578,9 @@ EXPORT_SYMBOL_NS_GPL(set_exclusive_cxl_commands, "CXL");
 void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
 				  unsigned long *cmds)
 {
-	down_write(&cxl_memdev_rwsem);
+	guard(rwsem_write)(&cxl_memdev_rwsem);
 	bitmap_andnot(mds->exclusive_cmds, mds->exclusive_cmds, cmds,
 		      CXL_MEM_COMMAND_ID_MAX);
-	up_write(&cxl_memdev_rwsem);
 }
 EXPORT_SYMBOL_NS_GPL(clear_exclusive_cxl_commands, "CXL");
 
@@ -590,9 +588,8 @@ static void cxl_memdev_shutdown(struct device *dev)
 {
 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
 
-	down_write(&cxl_memdev_rwsem);
+	guard(rwsem_write)(&cxl_memdev_rwsem);
 	cxlmd->cxlds = NULL;
-	up_write(&cxl_memdev_rwsem);
 }
 
 static void cxl_memdev_unregister(void *_cxlmd)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 78a5c2c25982..2c59d65bc18b 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -549,13 +549,9 @@ static ssize_t decoders_committed_show(struct device *dev,
 				       struct device_attribute *attr, char *buf)
 {
 	struct cxl_port *port = to_cxl_port(dev);
-	int rc;
-
-	down_read(&cxl_region_rwsem);
-	rc = sysfs_emit(buf, "%d\n", cxl_num_decoders_committed(port));
-	up_read(&cxl_region_rwsem);
 
-	return rc;
+	guard(rwsem_read)(&cxl_region_rwsem);
+	return sysfs_emit(buf, "%d\n", cxl_num_decoders_committed(port));
 }
 
 static DEVICE_ATTR_RO(decoders_committed);
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index e8d11a988fd9..d8a71f9f9fa5 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3208,7 +3208,6 @@ static int match_region_by_range(struct device *dev, const void *data)
 	struct cxl_region_params *p;
 	struct cxl_region *cxlr;
 	const struct range *r = data;
-	int rc = 0;
 
 	if (!is_cxl_region(dev))
 		return 0;
@@ -3216,12 +3215,11 @@ static int match_region_by_range(struct device *dev, const void *data)
 	cxlr = to_cxl_region(dev);
 	p = &cxlr->params;
 
-	down_read(&cxl_region_rwsem);
+	guard(rwsem_read)(&cxl_region_rwsem);
 	if (p->res && p->res->start == r->start && p->res->end == r->end)
-		rc = 1;
-	up_read(&cxl_region_rwsem);
+		return 1;
 
-	return rc;
+	return 0;
 }
 
 /* Establish an empty region covering the given HPA range */
-- 
2.34.1

[PATCH v2 2/7] cxl/core: cxl_mem_sanitize() cleanup

[Li Ming]

In cxl_mem_sanitize(), the down_read() and up_read() for
cxl_region_rwsem can be simply replaced by a guard(rwsem_read), and the
local variable 'rc' can be removed.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/mbox.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 548564c770c0..0601297af0c9 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1222,23 +1222,19 @@ int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd)
 {
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
 	struct cxl_port  *endpoint;
-	int rc;
 
 	/* synchronize with cxl_mem_probe() and decoder write operations */
 	guard(device)(&cxlmd->dev);
 	endpoint = cxlmd->endpoint;
-	down_read(&cxl_region_rwsem);
+	guard(rwsem_read)(&cxl_region_rwsem);
 	/*
 	 * Require an endpoint to be safe otherwise the driver can not
 	 * be sure that the device is unmapped.
 	 */
 	if (endpoint && cxl_num_decoders_committed(endpoint) == 0)
-		rc = __cxl_mem_sanitize(mds, cmd);
-	else
-		rc = -EBUSY;
-	up_read(&cxl_region_rwsem);
+		return __cxl_mem_sanitize(mds, cmd);
 
-	return rc;
+	return -EBUSY;
 }
 
 static int add_dpa_res(struct device *dev, struct resource *parent,
-- 
2.34.1

[PATCH v2 3/7] cxl/memdev: cxl_memdev_ioctl() cleanup

[Li Ming]

In cxl_memdev_ioctl(), the down_read(&cxl_memdev_rwsem) and
up_read(&cxl_memdev_rwsem) can be replaced by a
guard(rwsem_read)(&cxl_memdev_rwsem), it helps to remove the open-coded
up_read(&cxl_memdev_rwsem). Besides, the local var 'rc' can be also
removed to make the code more cleaner.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/memdev.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 98c05426aa4a..6f90dcd626f9 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -668,15 +668,13 @@ static long cxl_memdev_ioctl(struct file *file, unsigned int cmd,
 {
 	struct cxl_memdev *cxlmd = file->private_data;
 	struct cxl_dev_state *cxlds;
-	int rc = -ENXIO;
 
-	down_read(&cxl_memdev_rwsem);
+	guard(rwsem_read)(&cxl_memdev_rwsem);
 	cxlds = cxlmd->cxlds;
 	if (cxlds && cxlds->type == CXL_DEVTYPE_CLASSMEM)
-		rc = __cxl_memdev_ioctl(cxlmd, cmd, arg);
-	up_read(&cxl_memdev_rwsem);
+		return __cxl_memdev_ioctl(cxlmd, cmd, arg);
 
-	return rc;
+	return -ENXIO;
 }
 
 static int cxl_memdev_open(struct inode *inode, struct file *file)
-- 
2.34.1

[PATCH v2 4/7] cxl/core: Use guard() to drop the goto pattern of cxl_dpa_free()

[Li Ming]

cxl_dpa_free() has a goto pattern to call up_write() for cxl_dpa_rwsem,
it can be removed by using a guard() to replace the down_write() and
up_write().

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/hdm.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 4a578092377e..874a791f8292 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -382,35 +382,27 @@ int cxl_dpa_free(struct cxl_endpoint_decoder *cxled)
 {
 	struct cxl_port *port = cxled_to_port(cxled);
 	struct device *dev = &cxled->cxld.dev;
-	int rc;
 
-	down_write(&cxl_dpa_rwsem);
-	if (!cxled->dpa_res) {
-		rc = 0;
-		goto out;
-	}
+	guard(rwsem_write)(&cxl_dpa_rwsem);
+	if (!cxled->dpa_res)
+		return 0;
 	if (cxled->cxld.region) {
 		dev_dbg(dev, "decoder assigned to: %s\n",
 			dev_name(&cxled->cxld.region->dev));
-		rc = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 	if (cxled->cxld.flags & CXL_DECODER_F_ENABLE) {
 		dev_dbg(dev, "decoder enabled\n");
-		rc = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 	if (cxled->cxld.id != port->hdm_end) {
 		dev_dbg(dev, "expected decoder%d.%d\n", port->id,
 			port->hdm_end);
-		rc = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
+
 	devm_cxl_dpa_release(cxled);
-	rc = 0;
-out:
-	up_write(&cxl_dpa_rwsem);
-	return rc;
+	return 0;
 }
 
 int cxl_dpa_set_mode(struct cxl_endpoint_decoder *cxled,
-- 
2.34.1

[PATCH v2 5/7] cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()

[Li Ming]

In cxl_dpa_alloc(), some checking and operations need to be protected by
a rwsem called cxl_dpa_rwsem, so there is a goto pattern in
cxl_dpa_alloc() to release the rwsem. The goto pattern can be optimized
by using guard() to hold the rwsem.

Creating a new function called __cxl_dpa_alloc() to include all checking
and operations needed to be procted by cxl_dpa_rwsem. Using
guard(rwsem_write()) to hold cxl_dpa_rwsem at the beginning of the new
function.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/hdm.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 874a791f8292..1edbf7873471 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -442,29 +442,25 @@ int cxl_dpa_set_mode(struct cxl_endpoint_decoder *cxled,
 	return 0;
 }
 
-int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
+static int __cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
 {
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
 	resource_size_t free_ram_start, free_pmem_start;
-	struct cxl_port *port = cxled_to_port(cxled);
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct device *dev = &cxled->cxld.dev;
 	resource_size_t start, avail, skip;
 	struct resource *p, *last;
-	int rc;
 
-	down_write(&cxl_dpa_rwsem);
+	guard(rwsem_write)(&cxl_dpa_rwsem);
 	if (cxled->cxld.region) {
 		dev_dbg(dev, "decoder attached to %s\n",
 			dev_name(&cxled->cxld.region->dev));
-		rc = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 
 	if (cxled->cxld.flags & CXL_DECODER_F_ENABLE) {
 		dev_dbg(dev, "decoder enabled\n");
-		rc = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 
 	for (p = cxlds->ram_res.child, last = NULL; p; p = p->sibling)
@@ -504,21 +500,24 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
 		skip = skip_end - skip_start + 1;
 	} else {
 		dev_dbg(dev, "mode not set\n");
-		rc = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	if (size > avail) {
 		dev_dbg(dev, "%pa exceeds available %s capacity: %pa\n", &size,
 			cxl_decoder_mode_name(cxled->mode), &avail);
-		rc = -ENOSPC;
-		goto out;
+		return -ENOSPC;
 	}
 
-	rc = __cxl_dpa_reserve(cxled, start, size, skip);
-out:
-	up_write(&cxl_dpa_rwsem);
+	return __cxl_dpa_reserve(cxled, start, size, skip);
+}
+
+int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
+{
+	struct cxl_port *port = cxled_to_port(cxled);
+	int rc;
 
+	rc = __cxl_dpa_alloc(cxled, size);
 	if (rc)
 		return rc;
 
-- 
2.34.1

[PATCH v2 6/7] cxl/region: Drop goto pattern in cxl_dax_region_alloc()

[Li Ming]

In cxl_dax_region_alloc(), there is a goto pattern to release the rwsem
cxl_region_rwsem when the function returns, the down_read() and up_read
can be replaced by a guard(rwsem_read) then the goto pattern can be
removed.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/region.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index d8a71f9f9fa5..320a3f218131 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3038,17 +3038,13 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
 	struct cxl_dax_region *cxlr_dax;
 	struct device *dev;
 
-	down_read(&cxl_region_rwsem);
-	if (p->state != CXL_CONFIG_COMMIT) {
-		cxlr_dax = ERR_PTR(-ENXIO);
-		goto out;
-	}
+	guard(rwsem_read)(&cxl_region_rwsem);
+	if (p->state != CXL_CONFIG_COMMIT)
+		return ERR_PTR(-ENXIO);
 
 	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
-	if (!cxlr_dax) {
-		cxlr_dax = ERR_PTR(-ENOMEM);
-		goto out;
-	}
+	if (!cxlr_dax)
+		return ERR_PTR(-ENOMEM);
 
 	cxlr_dax->hpa_range.start = p->res->start;
 	cxlr_dax->hpa_range.end = p->res->end;
@@ -3061,8 +3057,6 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
 	dev->parent = &cxlr->dev;
 	dev->bus = &cxl_bus_type;
 	dev->type = &cxl_dax_region_type;
-out:
-	up_read(&cxl_region_rwsem);
 
 	return cxlr_dax;
 }
-- 
2.34.1

[PATCH v2 7/7] cxl/region: Drop goto pattern of construct_region()

[Li Ming]

Some operations need to be protected by the cxl_region_rwsem in
construct_region(). Currently, construct_region() uses down_write() and
up_write() for the cxl_region_rwsem locking, so there is a goto pattern
after down_write() invoked to release cxl_region_rwsem.

construct region() can be optimized to remove the goto pattern. The
changes are creating a new function called construct_auto_region() which
will include all checking and operations protected by the
cxl_region_rwsem, and using guard(rwsem_write) to replace down_write()
and up_write() in construct_auto_region().

Signed-off-by: Li Ming <ming.li@zohomail.com>
---
v2:
- Rename __construct_region() to construct_auto_region(). (Jonathan and Dave)
---
 drivers/cxl/core/region.c | 71 +++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 320a3f218131..7a9e51aba9f4 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3216,49 +3216,31 @@ static int match_region_by_range(struct device *dev, const void *data)
 	return 0;
 }
 
-/* Establish an empty region covering the given HPA range */
-static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
-					   struct cxl_endpoint_decoder *cxled)
+static int construct_auto_region(struct cxl_region *cxlr,
+				 struct cxl_root_decoder *cxlrd,
+				 struct cxl_endpoint_decoder *cxled)
 {
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
-	struct cxl_port *port = cxlrd_to_port(cxlrd);
 	struct range *hpa = &cxled->cxld.hpa_range;
 	struct cxl_region_params *p;
-	struct cxl_region *cxlr;
 	struct resource *res;
 	int rc;
 
-	do {
-		cxlr = __create_region(cxlrd, cxled->mode,
-				       atomic_read(&cxlrd->region_id));
-	} while (IS_ERR(cxlr) && PTR_ERR(cxlr) == -EBUSY);
-
-	if (IS_ERR(cxlr)) {
-		dev_err(cxlmd->dev.parent,
-			"%s:%s: %s failed assign region: %ld\n",
-			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
-			__func__, PTR_ERR(cxlr));
-		return cxlr;
-	}
-
-	down_write(&cxl_region_rwsem);
+	guard(rwsem_write)(&cxl_region_rwsem);
 	p = &cxlr->params;
 	if (p->state >= CXL_CONFIG_INTERLEAVE_ACTIVE) {
 		dev_err(cxlmd->dev.parent,
 			"%s:%s: %s autodiscovery interrupted\n",
 			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
 			__func__);
-		rc = -EBUSY;
-		goto err;
+		return -EBUSY;
 	}
 
 	set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
 
 	res = kmalloc(sizeof(*res), GFP_KERNEL);
-	if (!res) {
-		rc = -ENOMEM;
-		goto err;
-	}
+	if (!res)
+		return -ENOMEM;
 
 	*res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa),
 				    dev_name(&cxlr->dev));
@@ -3281,7 +3263,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 
 	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_target_group());
 	if (rc)
-		goto err;
+		return rc;
 
 	dev_dbg(cxlmd->dev.parent, "%s:%s: %s %s res: %pr iw: %d ig: %d\n",
 		dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev), __func__,
@@ -3290,14 +3272,39 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 
 	/* ...to match put_device() in cxl_add_to_region() */
 	get_device(&cxlr->dev);
-	up_write(&cxl_region_rwsem);
 
-	return cxlr;
+	return 0;
+}
 
-err:
-	up_write(&cxl_region_rwsem);
-	devm_release_action(port->uport_dev, unregister_region, cxlr);
-	return ERR_PTR(rc);
+/* Establish an empty region covering the given HPA range */
+static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
+					   struct cxl_endpoint_decoder *cxled)
+{
+	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+	struct cxl_port *port = cxlrd_to_port(cxlrd);
+	struct cxl_region *cxlr;
+	int rc;
+
+	do {
+		cxlr = __create_region(cxlrd, cxled->mode,
+				       atomic_read(&cxlrd->region_id));
+	} while (IS_ERR(cxlr) && PTR_ERR(cxlr) == -EBUSY);
+
+	if (IS_ERR(cxlr)) {
+		dev_err(cxlmd->dev.parent,
+			"%s:%s: %s failed assign region: %ld\n",
+			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
+			__func__, PTR_ERR(cxlr));
+		return cxlr;
+	}
+
+	rc = construct_auto_region(cxlr, cxlrd, cxled);
+	if (rc) {
+		devm_release_action(port->uport_dev, unregister_region, cxlr);
+		return ERR_PTR(rc);
+	}
+
+	return cxlr;
 }
 
 int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
-- 
2.34.1

Re: [PATCH v2 7/7] cxl/region: Drop goto pattern of construct_region()

[Jonathan Cameron]

On Mon, 17 Feb 2025 22:48:28 +0800
Li Ming <ming.li@zohomail.com> wrote:

> Some operations need to be protected by the cxl_region_rwsem in
> construct_region(). Currently, construct_region() uses down_write() and
> up_write() for the cxl_region_rwsem locking, so there is a goto pattern
> after down_write() invoked to release cxl_region_rwsem.
> 
> construct region() can be optimized to remove the goto pattern. The
> changes are creating a new function called construct_auto_region() which
> will include all checking and operations protected by the
> cxl_region_rwsem, and using guard(rwsem_write) to replace down_write()
> and up_write() in construct_auto_region().
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Re: [PATCH v2 0/7] Use guard() instead of rwsem locking

[Davidlohr Bueso]

On Mon, 17 Feb 2025, Li Ming wrote:

>Use scoped resource management to replace open-coded locking operation
>is recommended. CXL subsystem still remains some down_read()/up_read()
>and down_write()/up_write() which can be replaced by guard() simply.
>
>This patchset includes simply using guard() instead of some
>down_read()/up_read() and down_write()/up_write() cases. Besides, it
>also includes some function code cleanup after using guard().

For the series, feel free to add my:

Acked-by: Davidlohr Bueso <dave@stgolabs.net>

Re: [PATCH v2 1/7] cxl/core: Use guard() to replace open-coded down_read/write()

[Dave Jiang]

On 2/17/25 7:48 AM, Li Ming wrote:
> Some down/up_read() and down/up_write() cases can be replaced by a
> guard() simply to drop explicit unlock invoked. It helps to align coding
> style with current CXL subsystem's.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v2:
> - Drop some local variables. (Jonathan)
> ---
>  drivers/cxl/core/hdm.c    | 15 +++++----------
>  drivers/cxl/core/memdev.c |  9 +++------
>  drivers/cxl/core/port.c   |  8 ++------
>  drivers/cxl/core/region.c |  8 +++-----
>  4 files changed, 13 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 50e6a45b30ba..4a578092377e 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -213,13 +213,12 @@ void cxl_dpa_debug(struct seq_file *file, struct cxl_dev_state *cxlds)
>  {
>  	struct resource *p1, *p2;
>  
> -	down_read(&cxl_dpa_rwsem);
> +	guard(rwsem_read)(&cxl_dpa_rwsem);
>  	for (p1 = cxlds->dpa_res.child; p1; p1 = p1->sibling) {
>  		__cxl_dpa_debug(file, p1, 0);
>  		for (p2 = p1->child; p2; p2 = p2->sibling)
>  			__cxl_dpa_debug(file, p2, 1);
>  	}
> -	up_read(&cxl_dpa_rwsem);
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_dpa_debug, "CXL");
>  
> @@ -250,9 +249,8 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled)
>  
>  static void cxl_dpa_release(void *cxled)
>  {
> -	down_write(&cxl_dpa_rwsem);
> +	guard(rwsem_write)(&cxl_dpa_rwsem);
>  	__cxl_dpa_release(cxled);
> -	up_write(&cxl_dpa_rwsem);
>  }
>  
>  /*
> @@ -362,14 +360,11 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_dpa_reserve, "CXL");
>  
>  resource_size_t cxl_dpa_size(struct cxl_endpoint_decoder *cxled)
>  {
> -	resource_size_t size = 0;
> -
> -	down_read(&cxl_dpa_rwsem);
> +	guard(rwsem_read)(&cxl_dpa_rwsem);
>  	if (cxled->dpa_res)
> -		size = resource_size(cxled->dpa_res);
> -	up_read(&cxl_dpa_rwsem);
> +		return resource_size(cxled->dpa_res);
>  
> -	return size;
> +	return 0;
>  }
>  
>  resource_size_t cxl_dpa_resource_start(struct cxl_endpoint_decoder *cxled)
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index ae3dfcbe8938..98c05426aa4a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -564,10 +564,9 @@ EXPORT_SYMBOL_NS_GPL(is_cxl_memdev, "CXL");
>  void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
>  				unsigned long *cmds)
>  {
> -	down_write(&cxl_memdev_rwsem);
> +	guard(rwsem_write)(&cxl_memdev_rwsem);
>  	bitmap_or(mds->exclusive_cmds, mds->exclusive_cmds, cmds,
>  		  CXL_MEM_COMMAND_ID_MAX);
> -	up_write(&cxl_memdev_rwsem);
>  }
>  EXPORT_SYMBOL_NS_GPL(set_exclusive_cxl_commands, "CXL");
>  
> @@ -579,10 +578,9 @@ EXPORT_SYMBOL_NS_GPL(set_exclusive_cxl_commands, "CXL");
>  void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
>  				  unsigned long *cmds)
>  {
> -	down_write(&cxl_memdev_rwsem);
> +	guard(rwsem_write)(&cxl_memdev_rwsem);
>  	bitmap_andnot(mds->exclusive_cmds, mds->exclusive_cmds, cmds,
>  		      CXL_MEM_COMMAND_ID_MAX);
> -	up_write(&cxl_memdev_rwsem);
>  }
>  EXPORT_SYMBOL_NS_GPL(clear_exclusive_cxl_commands, "CXL");
>  
> @@ -590,9 +588,8 @@ static void cxl_memdev_shutdown(struct device *dev)
>  {
>  	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>  
> -	down_write(&cxl_memdev_rwsem);
> +	guard(rwsem_write)(&cxl_memdev_rwsem);
>  	cxlmd->cxlds = NULL;
> -	up_write(&cxl_memdev_rwsem);
>  }
>  
>  static void cxl_memdev_unregister(void *_cxlmd)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 78a5c2c25982..2c59d65bc18b 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -549,13 +549,9 @@ static ssize_t decoders_committed_show(struct device *dev,
>  				       struct device_attribute *attr, char *buf)
>  {
>  	struct cxl_port *port = to_cxl_port(dev);
> -	int rc;
> -
> -	down_read(&cxl_region_rwsem);
> -	rc = sysfs_emit(buf, "%d\n", cxl_num_decoders_committed(port));
> -	up_read(&cxl_region_rwsem);
>  
> -	return rc;
> +	guard(rwsem_read)(&cxl_region_rwsem);
> +	return sysfs_emit(buf, "%d\n", cxl_num_decoders_committed(port));
>  }
>  
>  static DEVICE_ATTR_RO(decoders_committed);
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e8d11a988fd9..d8a71f9f9fa5 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3208,7 +3208,6 @@ static int match_region_by_range(struct device *dev, const void *data)
>  	struct cxl_region_params *p;
>  	struct cxl_region *cxlr;
>  	const struct range *r = data;
> -	int rc = 0;
>  
>  	if (!is_cxl_region(dev))
>  		return 0;
> @@ -3216,12 +3215,11 @@ static int match_region_by_range(struct device *dev, const void *data)
>  	cxlr = to_cxl_region(dev);
>  	p = &cxlr->params;
>  
> -	down_read(&cxl_region_rwsem);
> +	guard(rwsem_read)(&cxl_region_rwsem);
>  	if (p->res && p->res->start == r->start && p->res->end == r->end)
> -		rc = 1;
> -	up_read(&cxl_region_rwsem);
> +		return 1;
>  
> -	return rc;
> +	return 0;
>  }
>  
>  /* Establish an empty region covering the given HPA range */

Re: [PATCH v2 2/7] cxl/core: cxl_mem_sanitize() cleanup

[Dave Jiang]

On 2/17/25 7:48 AM, Li Ming wrote:
> In cxl_mem_sanitize(), the down_read() and up_read() for
> cxl_region_rwsem can be simply replaced by a guard(rwsem_read), and the
> local variable 'rc' can be removed.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/mbox.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 548564c770c0..0601297af0c9 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1222,23 +1222,19 @@ int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd)
>  {
>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
>  	struct cxl_port  *endpoint;
> -	int rc;
>  
>  	/* synchronize with cxl_mem_probe() and decoder write operations */
>  	guard(device)(&cxlmd->dev);
>  	endpoint = cxlmd->endpoint;
> -	down_read(&cxl_region_rwsem);
> +	guard(rwsem_read)(&cxl_region_rwsem);
>  	/*
>  	 * Require an endpoint to be safe otherwise the driver can not
>  	 * be sure that the device is unmapped.
>  	 */
>  	if (endpoint && cxl_num_decoders_committed(endpoint) == 0)
> -		rc = __cxl_mem_sanitize(mds, cmd);
> -	else
> -		rc = -EBUSY;
> -	up_read(&cxl_region_rwsem);
> +		return __cxl_mem_sanitize(mds, cmd);
>  
> -	return rc;
> +	return -EBUSY;
>  }
>  
>  static int add_dpa_res(struct device *dev, struct resource *parent,

Re: [PATCH v2 3/7] cxl/memdev: cxl_memdev_ioctl() cleanup

[Dave Jiang]

On 2/17/25 7:48 AM, Li Ming wrote:
> In cxl_memdev_ioctl(), the down_read(&cxl_memdev_rwsem) and
> up_read(&cxl_memdev_rwsem) can be replaced by a
> guard(rwsem_read)(&cxl_memdev_rwsem), it helps to remove the open-coded
> up_read(&cxl_memdev_rwsem). Besides, the local var 'rc' can be also
> removed to make the code more cleaner.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/memdev.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 98c05426aa4a..6f90dcd626f9 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -668,15 +668,13 @@ static long cxl_memdev_ioctl(struct file *file, unsigned int cmd,
>  {
>  	struct cxl_memdev *cxlmd = file->private_data;
>  	struct cxl_dev_state *cxlds;
> -	int rc = -ENXIO;
>  
> -	down_read(&cxl_memdev_rwsem);
> +	guard(rwsem_read)(&cxl_memdev_rwsem);
>  	cxlds = cxlmd->cxlds;
>  	if (cxlds && cxlds->type == CXL_DEVTYPE_CLASSMEM)
> -		rc = __cxl_memdev_ioctl(cxlmd, cmd, arg);
> -	up_read(&cxl_memdev_rwsem);
> +		return __cxl_memdev_ioctl(cxlmd, cmd, arg);
>  
> -	return rc;
> +	return -ENXIO;
>  }
>  
>  static int cxl_memdev_open(struct inode *inode, struct file *file)

Re: [PATCH v2 4/7] cxl/core: Use guard() to drop the goto pattern of cxl_dpa_free()

[Dave Jiang]

On 2/17/25 7:48 AM, Li Ming wrote:
> cxl_dpa_free() has a goto pattern to call up_write() for cxl_dpa_rwsem,
> it can be removed by using a guard() to replace the down_write() and
> up_write().
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/hdm.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 4a578092377e..874a791f8292 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -382,35 +382,27 @@ int cxl_dpa_free(struct cxl_endpoint_decoder *cxled)
>  {
>  	struct cxl_port *port = cxled_to_port(cxled);
>  	struct device *dev = &cxled->cxld.dev;
> -	int rc;
>  
> -	down_write(&cxl_dpa_rwsem);
> -	if (!cxled->dpa_res) {
> -		rc = 0;
> -		goto out;
> -	}
> +	guard(rwsem_write)(&cxl_dpa_rwsem);
> +	if (!cxled->dpa_res)
> +		return 0;
>  	if (cxled->cxld.region) {
>  		dev_dbg(dev, "decoder assigned to: %s\n",
>  			dev_name(&cxled->cxld.region->dev));
> -		rc = -EBUSY;
> -		goto out;
> +		return -EBUSY;
>  	}
>  	if (cxled->cxld.flags & CXL_DECODER_F_ENABLE) {
>  		dev_dbg(dev, "decoder enabled\n");
> -		rc = -EBUSY;
> -		goto out;
> +		return -EBUSY;
>  	}
>  	if (cxled->cxld.id != port->hdm_end) {
>  		dev_dbg(dev, "expected decoder%d.%d\n", port->id,
>  			port->hdm_end);
> -		rc = -EBUSY;
> -		goto out;
> +		return -EBUSY;
>  	}
> +
>  	devm_cxl_dpa_release(cxled);
> -	rc = 0;
> -out:
> -	up_write(&cxl_dpa_rwsem);
> -	return rc;
> +	return 0;
>  }
>  
>  int cxl_dpa_set_mode(struct cxl_endpoint_decoder *cxled,

Re: [PATCH v2 5/7] cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()

[Dave Jiang]

On 2/17/25 7:48 AM, Li Ming wrote:
> In cxl_dpa_alloc(), some checking and operations need to be protected by
> a rwsem called cxl_dpa_rwsem, so there is a goto pattern in
> cxl_dpa_alloc() to release the rwsem. The goto pattern can be optimized
> by using guard() to hold the rwsem.
> 
> Creating a new function called __cxl_dpa_alloc() to include all checking
> and operations needed to be procted by cxl_dpa_rwsem. Using
s/procted/protected/

> guard(rwsem_write()) to hold cxl_dpa_rwsem at the beginning of the new
> function.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/hdm.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 874a791f8292..1edbf7873471 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -442,29 +442,25 @@ int cxl_dpa_set_mode(struct cxl_endpoint_decoder *cxled,
>  	return 0;
>  }
>  
> -int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
> +static int __cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
>  {
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
>  	resource_size_t free_ram_start, free_pmem_start;
> -	struct cxl_port *port = cxled_to_port(cxled);
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	struct device *dev = &cxled->cxld.dev;
>  	resource_size_t start, avail, skip;
>  	struct resource *p, *last;
> -	int rc;
>  
> -	down_write(&cxl_dpa_rwsem);
> +	guard(rwsem_write)(&cxl_dpa_rwsem);
>  	if (cxled->cxld.region) {
>  		dev_dbg(dev, "decoder attached to %s\n",
>  			dev_name(&cxled->cxld.region->dev));
> -		rc = -EBUSY;
> -		goto out;
> +		return -EBUSY;
>  	}
>  
>  	if (cxled->cxld.flags & CXL_DECODER_F_ENABLE) {
>  		dev_dbg(dev, "decoder enabled\n");
> -		rc = -EBUSY;
> -		goto out;
> +		return -EBUSY;
>  	}
>  
>  	for (p = cxlds->ram_res.child, last = NULL; p; p = p->sibling)
> @@ -504,21 +500,24 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
>  		skip = skip_end - skip_start + 1;
>  	} else {
>  		dev_dbg(dev, "mode not set\n");
> -		rc = -EINVAL;
> -		goto out;
> +		return -EINVAL;
>  	}
>  
>  	if (size > avail) {
>  		dev_dbg(dev, "%pa exceeds available %s capacity: %pa\n", &size,
>  			cxl_decoder_mode_name(cxled->mode), &avail);
> -		rc = -ENOSPC;
> -		goto out;
> +		return -ENOSPC;
>  	}
>  
> -	rc = __cxl_dpa_reserve(cxled, start, size, skip);
> -out:
> -	up_write(&cxl_dpa_rwsem);
> +	return __cxl_dpa_reserve(cxled, start, size, skip);
> +}
> +
> +int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
> +{
> +	struct cxl_port *port = cxled_to_port(cxled);
> +	int rc;
>  
> +	rc = __cxl_dpa_alloc(cxled, size);
>  	if (rc)
>  		return rc;
>  

Re: [PATCH v2 6/7] cxl/region: Drop goto pattern in cxl_dax_region_alloc()

[Dave Jiang]

On 2/17/25 7:48 AM, Li Ming wrote:
> In cxl_dax_region_alloc(), there is a goto pattern to release the rwsem
> cxl_region_rwsem when the function returns, the down_read() and up_read
> can be replaced by a guard(rwsem_read) then the goto pattern can be
> removed.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/region.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index d8a71f9f9fa5..320a3f218131 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3038,17 +3038,13 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>  	struct cxl_dax_region *cxlr_dax;
>  	struct device *dev;
>  
> -	down_read(&cxl_region_rwsem);
> -	if (p->state != CXL_CONFIG_COMMIT) {
> -		cxlr_dax = ERR_PTR(-ENXIO);
> -		goto out;
> -	}
> +	guard(rwsem_read)(&cxl_region_rwsem);
> +	if (p->state != CXL_CONFIG_COMMIT)
> +		return ERR_PTR(-ENXIO);
>  
>  	cxlr_dax = kzalloc(sizeof(*cxlr_dax), GFP_KERNEL);
> -	if (!cxlr_dax) {
> -		cxlr_dax = ERR_PTR(-ENOMEM);
> -		goto out;
> -	}
> +	if (!cxlr_dax)
> +		return ERR_PTR(-ENOMEM);
>  
>  	cxlr_dax->hpa_range.start = p->res->start;
>  	cxlr_dax->hpa_range.end = p->res->end;
> @@ -3061,8 +3057,6 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>  	dev->parent = &cxlr->dev;
>  	dev->bus = &cxl_bus_type;
>  	dev->type = &cxl_dax_region_type;
> -out:
> -	up_read(&cxl_region_rwsem);
>  
>  	return cxlr_dax;
>  }

Re: [PATCH v2 7/7] cxl/region: Drop goto pattern of construct_region()

[Dave Jiang]

On 2/17/25 7:48 AM, Li Ming wrote:
> Some operations need to be protected by the cxl_region_rwsem in
> construct_region(). Currently, construct_region() uses down_write() and
> up_write() for the cxl_region_rwsem locking, so there is a goto pattern
> after down_write() invoked to release cxl_region_rwsem.
> 
> construct region() can be optimized to remove the goto pattern. The
> changes are creating a new function called construct_auto_region() which
> will include all checking and operations protected by the
> cxl_region_rwsem, and using guard(rwsem_write) to replace down_write()
> and up_write() in construct_auto_region().
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
> v2:
> - Rename __construct_region() to construct_auto_region(). (Jonathan and Dave)
> ---
>  drivers/cxl/core/region.c | 71 +++++++++++++++++++++------------------
>  1 file changed, 39 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 320a3f218131..7a9e51aba9f4 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3216,49 +3216,31 @@ static int match_region_by_range(struct device *dev, const void *data)
>  	return 0;
>  }
>  
> -/* Establish an empty region covering the given HPA range */
> -static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
> -					   struct cxl_endpoint_decoder *cxled)
> +static int construct_auto_region(struct cxl_region *cxlr,
> +				 struct cxl_root_decoder *cxlrd,
> +				 struct cxl_endpoint_decoder *cxled)
>  {
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> -	struct cxl_port *port = cxlrd_to_port(cxlrd);
>  	struct range *hpa = &cxled->cxld.hpa_range;
>  	struct cxl_region_params *p;
> -	struct cxl_region *cxlr;
>  	struct resource *res;
>  	int rc;
>  
> -	do {
> -		cxlr = __create_region(cxlrd, cxled->mode,
> -				       atomic_read(&cxlrd->region_id));
> -	} while (IS_ERR(cxlr) && PTR_ERR(cxlr) == -EBUSY);
> -
> -	if (IS_ERR(cxlr)) {
> -		dev_err(cxlmd->dev.parent,
> -			"%s:%s: %s failed assign region: %ld\n",
> -			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> -			__func__, PTR_ERR(cxlr));
> -		return cxlr;
> -	}
> -
> -	down_write(&cxl_region_rwsem);
> +	guard(rwsem_write)(&cxl_region_rwsem);
>  	p = &cxlr->params;
>  	if (p->state >= CXL_CONFIG_INTERLEAVE_ACTIVE) {
>  		dev_err(cxlmd->dev.parent,
>  			"%s:%s: %s autodiscovery interrupted\n",
>  			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
>  			__func__);
> -		rc = -EBUSY;
> -		goto err;
> +		return -EBUSY;
>  	}
>  
>  	set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
>  
>  	res = kmalloc(sizeof(*res), GFP_KERNEL);
> -	if (!res) {
> -		rc = -ENOMEM;
> -		goto err;
> -	}
> +	if (!res)
> +		return -ENOMEM;
>  
>  	*res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa),
>  				    dev_name(&cxlr->dev));
> @@ -3281,7 +3263,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  
>  	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_target_group());
>  	if (rc)
> -		goto err;
> +		return rc;
>  
>  	dev_dbg(cxlmd->dev.parent, "%s:%s: %s %s res: %pr iw: %d ig: %d\n",
>  		dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev), __func__,
> @@ -3290,14 +3272,39 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  
>  	/* ...to match put_device() in cxl_add_to_region() */
>  	get_device(&cxlr->dev);
> -	up_write(&cxl_region_rwsem);
>  
> -	return cxlr;
> +	return 0;
> +}
>  
> -err:
> -	up_write(&cxl_region_rwsem);
> -	devm_release_action(port->uport_dev, unregister_region, cxlr);
> -	return ERR_PTR(rc);
> +/* Establish an empty region covering the given HPA range */
> +static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
> +					   struct cxl_endpoint_decoder *cxled)
> +{
> +	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> +	struct cxl_port *port = cxlrd_to_port(cxlrd);
> +	struct cxl_region *cxlr;
> +	int rc;
> +
> +	do {
> +		cxlr = __create_region(cxlrd, cxled->mode,
> +				       atomic_read(&cxlrd->region_id));
> +	} while (IS_ERR(cxlr) && PTR_ERR(cxlr) == -EBUSY);
> +
> +	if (IS_ERR(cxlr)) {
> +		dev_err(cxlmd->dev.parent,
> +			"%s:%s: %s failed assign region: %ld\n",
> +			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> +			__func__, PTR_ERR(cxlr));
> +		return cxlr;
> +	}
> +
> +	rc = construct_auto_region(cxlr, cxlrd, cxled);
> +	if (rc) {
> +		devm_release_action(port->uport_dev, unregister_region, cxlr);
> +		return ERR_PTR(rc);
> +	}
> +
> +	return cxlr;
>  }
>  
>  int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)

Re: [PATCH v2 1/7] cxl/core: Use guard() to replace open-coded down_read/write()

[Alison Schofield]

On Mon, Feb 17, 2025 at 10:48:22PM +0800, Li Ming wrote:
> Some down/up_read() and down/up_write() cases can be replaced by a
> guard() simply to drop explicit unlock invoked. It helps to align coding
> style with current CXL subsystem's.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

snip

Re: [PATCH v2 2/7] cxl/core: cxl_mem_sanitize() cleanup

[Alison Schofield]

On Mon, Feb 17, 2025 at 10:48:23PM +0800, Li Ming wrote:
> In cxl_mem_sanitize(), the down_read() and up_read() for
> cxl_region_rwsem can be simply replaced by a guard(rwsem_read), and the
> local variable 'rc' can be removed.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
>  drivers/cxl/core/mbox.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

snip

Re: [PATCH v2 3/7] cxl/memdev: cxl_memdev_ioctl() cleanup

[Alison Schofield]

On Mon, Feb 17, 2025 at 10:48:24PM +0800, Li Ming wrote:
> In cxl_memdev_ioctl(), the down_read(&cxl_memdev_rwsem) and
> up_read(&cxl_memdev_rwsem) can be replaced by a
> guard(rwsem_read)(&cxl_memdev_rwsem), it helps to remove the open-coded
> up_read(&cxl_memdev_rwsem). Besides, the local var 'rc' can be also
> removed to make the code more cleaner.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

snip

Re: [PATCH v2 4/7] cxl/core: Use guard() to drop the goto pattern of cxl_dpa_free()

[Alison Schofield]

On Mon, Feb 17, 2025 at 10:48:25PM +0800, Li Ming wrote:
> cxl_dpa_free() has a goto pattern to call up_write() for cxl_dpa_rwsem,
> it can be removed by using a guard() to replace the down_write() and
> up_write().
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

snip

Re: [PATCH v2 5/7] cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()

[Alison Schofield]

On Mon, Feb 17, 2025 at 10:48:26PM +0800, Li Ming wrote:
> In cxl_dpa_alloc(), some checking and operations need to be protected by
> a rwsem called cxl_dpa_rwsem, so there is a goto pattern in
> cxl_dpa_alloc() to release the rwsem. The goto pattern can be optimized
> by using guard() to hold the rwsem.
> 
> Creating a new function called __cxl_dpa_alloc() to include all checking
> and operations needed to be procted by cxl_dpa_rwsem. Using
> guard(rwsem_write()) to hold cxl_dpa_rwsem at the beginning of the new
> function.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

snip

Re: [PATCH v2 6/7] cxl/region: Drop goto pattern in cxl_dax_region_alloc()

[Alison Schofield]

On Mon, Feb 17, 2025 at 10:48:27PM +0800, Li Ming wrote:
> In cxl_dax_region_alloc(), there is a goto pattern to release the rwsem
> cxl_region_rwsem when the function returns, the down_read() and up_read
> can be replaced by a guard(rwsem_read) then the goto pattern can be
> removed.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

snip

Re: [PATCH v2 7/7] cxl/region: Drop goto pattern of construct_region()

[Alison Schofield]

On Mon, Feb 17, 2025 at 10:48:28PM +0800, Li Ming wrote:
> Some operations need to be protected by the cxl_region_rwsem in
> construct_region(). Currently, construct_region() uses down_write() and
> up_write() for the cxl_region_rwsem locking, so there is a goto pattern
> after down_write() invoked to release cxl_region_rwsem.
> 
> construct region() can be optimized to remove the goto pattern. The
> changes are creating a new function called construct_auto_region() which
> will include all checking and operations protected by the
> cxl_region_rwsem, and using guard(rwsem_write) to replace down_write()
> and up_write() in construct_auto_region().
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

snip

Re: [PATCH v2 0/7] Use guard() instead of rwsem locking

[Ira Weiny]

Li Ming wrote:
> Use scoped resource management to replace open-coded locking operation
> is recommended. CXL subsystem still remains some down_read()/up_read()
> and down_write()/up_write() which can be replaced by guard() simply.
> 
> This patchset includes simply using guard() instead of some
> down_read()/up_read() and down_write()/up_write() cases. Besides, it
> also includes some function code cleanup after using guard().
> 
> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 (tag: v6.14-rc2)

For the series.

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> 
> v2:
> - Drop some local variables. (Jonathan)
> - Rename __construct_region() to construct_auto_region(). (Jonathan and Dave)
> 
> Li Ming (7):
>   cxl/core: Use guard() to replace open-coded down_read/write()
>   cxl/core: cxl_mem_sanitize() cleanup
>   cxl/memdev: cxl_memdev_ioctl() cleanup
>   cxl/core: Use guard() to drop the goto pattern of cxl_dpa_free()
>   cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()
>   cxl/region: Drop goto pattern in cxl_dax_region_alloc()
>   cxl/region: Drop goto pattern of construct_region()
> 
>  drivers/cxl/core/hdm.c    | 68 +++++++++++-----------------
>  drivers/cxl/core/mbox.c   | 10 ++---
>  drivers/cxl/core/memdev.c | 17 +++----
>  drivers/cxl/core/port.c   |  8 +---
>  drivers/cxl/core/region.c | 95 +++++++++++++++++++--------------------
>  5 files changed, 85 insertions(+), 113 deletions(-)
> 
> -- 
> 2.34.1
> 

Re: [PATCH v2 0/7] Use guard() instead of rwsem locking

[Dave Jiang]

On 2/17/25 7:48 AM, Li Ming wrote:
> Use scoped resource management to replace open-coded locking operation
> is recommended. CXL subsystem still remains some down_read()/up_read()
> and down_write()/up_write() which can be replaced by guard() simply.
> 
> This patchset includes simply using guard() instead of some
> down_read()/up_read() and down_write()/up_write() cases. Besides, it
> also includes some function code cleanup after using guard().
> 
> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 (tag: v6.14-rc2)
> 
> v2:
> - Drop some local variables. (Jonathan)
> - Rename __construct_region() to construct_auto_region(). (Jonathan and Dave)

Hi Ming,
Can you please do me a favor and rebase this against cxl/next for a v3? I think there are some conflicts against the DPA cleanup that's not easily resolved. Thanks!

DJ

> 
> Li Ming (7):
>   cxl/core: Use guard() to replace open-coded down_read/write()
>   cxl/core: cxl_mem_sanitize() cleanup
>   cxl/memdev: cxl_memdev_ioctl() cleanup
>   cxl/core: Use guard() to drop the goto pattern of cxl_dpa_free()
>   cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()
>   cxl/region: Drop goto pattern in cxl_dax_region_alloc()
>   cxl/region: Drop goto pattern of construct_region()
> 
>  drivers/cxl/core/hdm.c    | 68 +++++++++++-----------------
>  drivers/cxl/core/mbox.c   | 10 ++---
>  drivers/cxl/core/memdev.c | 17 +++----
>  drivers/cxl/core/port.c   |  8 +---
>  drivers/cxl/core/region.c | 95 +++++++++++++++++++--------------------
>  5 files changed, 85 insertions(+), 113 deletions(-)
> 

Re: [PATCH v2 0/7] Use guard() instead of rwsem locking

[Li Ming]

On 2/20/2025 5:09 AM, Dave Jiang wrote:
>
> On 2/17/25 7:48 AM, Li Ming wrote:
>> Use scoped resource management to replace open-coded locking operation
>> is recommended. CXL subsystem still remains some down_read()/up_read()
>> and down_write()/up_write() which can be replaced by guard() simply.
>>
>> This patchset includes simply using guard() instead of some
>> down_read()/up_read() and down_write()/up_write() cases. Besides, it
>> also includes some function code cleanup after using guard().
>>
>> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 (tag: v6.14-rc2)
>>
>> v2:
>> - Drop some local variables. (Jonathan)
>> - Rename __construct_region() to construct_auto_region(). (Jonathan and Dave)
> Hi Ming,
> Can you please do me a favor and rebase this against cxl/next for a v3? I think there are some conflicts against the DPA cleanup that's not easily resolved. Thanks!
>
> DJ

Sure, will do that.


Ming

Re: [PATCH v2 5/7] cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()

[Dan Williams]

Li Ming wrote:
> In cxl_dpa_alloc(), some checking and operations need to be protected by
> a rwsem called cxl_dpa_rwsem, so there is a goto pattern in
> cxl_dpa_alloc() to release the rwsem. The goto pattern can be optimized
> by using guard() to hold the rwsem.
> 
> Creating a new function called __cxl_dpa_alloc() to include all checking
> and operations needed to be procted by cxl_dpa_rwsem. Using
> guard(rwsem_write()) to hold cxl_dpa_rwsem at the beginning of the new
> function.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
>  drivers/cxl/core/hdm.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 874a791f8292..1edbf7873471 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
[..]
> @@ -504,21 +500,24 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
>  		skip = skip_end - skip_start + 1;
>  	} else {
>  		dev_dbg(dev, "mode not set\n");
> -		rc = -EINVAL;
> -		goto out;
> +		return -EINVAL;
>  	}
>  
>  	if (size > avail) {
>  		dev_dbg(dev, "%pa exceeds available %s capacity: %pa\n", &size,
>  			cxl_decoder_mode_name(cxled->mode), &avail);
> -		rc = -ENOSPC;
> -		goto out;
> +		return -ENOSPC;
>  	}
>  
> -	rc = __cxl_dpa_reserve(cxled, start, size, skip);
> -out:
> -	up_write(&cxl_dpa_rwsem);
> +	return __cxl_dpa_reserve(cxled, start, size, skip);
> +}
> +
> +int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
> +{
> +	struct cxl_port *port = cxled_to_port(cxled);

Am I missing something? This @port variable is unused?

> +	int rc;
>  
> +	rc = __cxl_dpa_alloc(cxled, size);
>  	if (rc)
>  		return rc;
>  

So I think you can drop this new cxl_dpa_alloc + __cxl_dpa_alloc scheme?

After that you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH v2 7/7] cxl/region: Drop goto pattern of construct_region()

[Dan Williams]

Li Ming wrote:
> Some operations need to be protected by the cxl_region_rwsem in
> construct_region(). Currently, construct_region() uses down_write() and
> up_write() for the cxl_region_rwsem locking, so there is a goto pattern
> after down_write() invoked to release cxl_region_rwsem.
> 
> construct region() can be optimized to remove the goto pattern. The
> changes are creating a new function called construct_auto_region() which
> will include all checking and operations protected by the
> cxl_region_rwsem, and using guard(rwsem_write) to replace down_write()
> and up_write() in construct_auto_region().
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
> v2:
> - Rename __construct_region() to construct_auto_region(). (Jonathan and Dave)
> ---
>  drivers/cxl/core/region.c | 71 +++++++++++++++++++++------------------
>  1 file changed, 39 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 320a3f218131..7a9e51aba9f4 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3216,49 +3216,31 @@ static int match_region_by_range(struct device *dev, const void *data)
>  	return 0;
>  }
>  
> -/* Establish an empty region covering the given HPA range */
> -static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
> -					   struct cxl_endpoint_decoder *cxled)
> +static int construct_auto_region(struct cxl_region *cxlr,

...probably would have called this __construct_region() since there is
little distinction that merits adding the "auto" qualifier.

Other than that, for this and the others you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH v2 5/7] cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()

[Li Ming]

On 2/20/2025 9:01 AM, Dan Williams wrote:
> Li Ming wrote:
>> In cxl_dpa_alloc(), some checking and operations need to be protected by
>> a rwsem called cxl_dpa_rwsem, so there is a goto pattern in
>> cxl_dpa_alloc() to release the rwsem. The goto pattern can be optimized
>> by using guard() to hold the rwsem.
>>
>> Creating a new function called __cxl_dpa_alloc() to include all checking
>> and operations needed to be procted by cxl_dpa_rwsem. Using
>> guard(rwsem_write()) to hold cxl_dpa_rwsem at the beginning of the new
>> function.
>>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Li Ming <ming.li@zohomail.com>
>> ---
>>  drivers/cxl/core/hdm.c | 29 ++++++++++++++---------------
>>  1 file changed, 14 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index 874a791f8292..1edbf7873471 100644
>> --- a/drivers/cxl/core/hdm.c
>> +++ b/drivers/cxl/core/hdm.c
> [..]
>> @@ -504,21 +500,24 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
>>  		skip = skip_end - skip_start + 1;
>>  	} else {
>>  		dev_dbg(dev, "mode not set\n");
>> -		rc = -EINVAL;
>> -		goto out;
>> +		return -EINVAL;
>>  	}
>>  
>>  	if (size > avail) {
>>  		dev_dbg(dev, "%pa exceeds available %s capacity: %pa\n", &size,
>>  			cxl_decoder_mode_name(cxled->mode), &avail);
>> -		rc = -ENOSPC;
>> -		goto out;
>> +		return -ENOSPC;
>>  	}
>>  
>> -	rc = __cxl_dpa_reserve(cxled, start, size, skip);
>> -out:
>> -	up_write(&cxl_dpa_rwsem);
>> +	return __cxl_dpa_reserve(cxled, start, size, skip);
>> +}
>> +
>> +int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size)
>> +{
>> +	struct cxl_port *port = cxled_to_port(cxled);
> Am I missing something? This @port variable is unused?
@port will be used after below __cxl_dpa_alloc().
>
>> +	int rc;
>>  
>> +	rc = __cxl_dpa_alloc(cxled, size);
>>  	if (rc)
>>  		return rc;
>>  
> So I think you can drop this new cxl_dpa_alloc + __cxl_dpa_alloc scheme?

After __cxl_dpa_alloc(), a 'devm_add_action_or_reset(&port->dev, cxl_dpa_release, cxled)' will be invoked, cxl_dpa_rwsem is possible to be held in cxl_dpa_release() in devm_add_action_or_reset() failure case.

So I create __cxl_dpa_alloc() to hold cxl_dpa_rwsem for the operations needed cxl_dpa_resem protection, make sure that the cxl_dpa_rwsem is released before devm_add_action_or_reset() invoking.

>
> After that you can add:
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH v2 5/7] cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()

[Dan Williams]

Li Ming wrote:
[..]
> After __cxl_dpa_alloc(), a 'devm_add_action_or_reset(&port->dev,
> cxl_dpa_release, cxled)' will be invoked, cxl_dpa_rwsem is possible to
> be held in cxl_dpa_release() in devm_add_action_or_reset() failure
> case.
> 
> So I create __cxl_dpa_alloc() to hold cxl_dpa_rwsem for the operations
> needed cxl_dpa_resem protection, make sure that the cxl_dpa_rwsem is
> released before devm_add_action_or_reset() invoking.

Ah, got it, missing context, makes sense.

Re: [PATCH v2 7/7] cxl/region: Drop goto pattern of construct_region()

[Li Ming]

On 2/20/2025 9:04 AM, Dan Williams wrote:
> Li Ming wrote:
>> Some operations need to be protected by the cxl_region_rwsem in
>> construct_region(). Currently, construct_region() uses down_write() and
>> up_write() for the cxl_region_rwsem locking, so there is a goto pattern
>> after down_write() invoked to release cxl_region_rwsem.
>>
>> construct region() can be optimized to remove the goto pattern. The
>> changes are creating a new function called construct_auto_region() which
>> will include all checking and operations protected by the
>> cxl_region_rwsem, and using guard(rwsem_write) to replace down_write()
>> and up_write() in construct_auto_region().
>>
>> Signed-off-by: Li Ming <ming.li@zohomail.com>
>> ---
>> v2:
>> - Rename __construct_region() to construct_auto_region(). (Jonathan and Dave)
>> ---
>>  drivers/cxl/core/region.c | 71 +++++++++++++++++++++------------------
>>  1 file changed, 39 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 320a3f218131..7a9e51aba9f4 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -3216,49 +3216,31 @@ static int match_region_by_range(struct device *dev, const void *data)
>>  	return 0;
>>  }
>>  
>> -/* Establish an empty region covering the given HPA range */
>> -static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>> -					   struct cxl_endpoint_decoder *cxled)
>> +static int construct_auto_region(struct cxl_region *cxlr,
> ...probably would have called this __construct_region() since there is
> little distinction that merits adding the "auto" qualifier.

Sure, will revert this renaming, continue to use __construct_region(), thanks for review.


Ming

>
> Other than that, for this and the others you can add:
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>