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

[PATCH v1 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)

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    | 62 +++++++++++----------------
 drivers/cxl/core/mbox.c   | 10 ++---
 drivers/cxl/core/memdev.c | 17 +++-----
 drivers/cxl/core/port.c   |  8 +---
 drivers/cxl/core/region.c | 90 +++++++++++++++++++--------------------
 5 files changed, 81 insertions(+), 106 deletions(-)

-- 
2.34.1

[PATCH v1 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.

Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/hdm.c    | 9 +++------
 drivers/cxl/core/memdev.c | 9 +++------
 drivers/cxl/core/port.c   | 8 ++------
 drivers/cxl/core/region.c | 3 +--
 4 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 50e6a45b30ba..2bb2782a72ad 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);
 }
 
 /*
@@ -364,10 +362,9 @@ 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 size;
 }
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..e3bb33109ced 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3216,10 +3216,9 @@ 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 rc;
 }
-- 
2.34.1

[PATCH v1 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.

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 v1 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.

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 v1 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().

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 2bb2782a72ad..4d753520a79a 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -384,35 +384,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 v1 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.

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 4d753520a79a..27813c430db9 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -444,29 +444,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)
@@ -506,21 +502,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 v1 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.

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 e3bb33109ced..36f3771818d3 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 v1 7/7] cxl/region: Drop goto pattern of construct_region()

[Li Ming]

Some operations need to be procted by the cxl_region_rwsem in construct
region(). Currently, construct_region() uses down_write() and up_write()
for the cxl_region_rwsem, 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_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_region().

Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 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 36f3771818d3..170278bdcedc 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3217,49 +3217,31 @@ static int match_region_by_range(struct device *dev, const void *data)
 	return 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)
+static int __construct_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));
@@ -3282,7 +3264,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__,
@@ -3291,14 +3273,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_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 v1 1/7] cxl/core: Use guard() to replace open-coded down_read/write()

[Jonathan Cameron]

On Tue, 11 Feb 2025 15:57:21 +0800
Li Ming <ming.li@zohomail.com> 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.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

I wondered if it was worth doing some early returns and dropping
local variables, but on balance think it isn't worth doing.


> @@ -364,10 +362,9 @@ 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);

Could return resource_size() here and return 0 below.
I'm not sure it gains us much though. 

> -	up_read(&cxl_dpa_rwsem);
>  
>  	return size;
>  }
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e8d11a988fd9..e3bb33109ced 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3216,10 +3216,9 @@ 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;

could do return 1 here and return 0 but it doesn't gain us much.

> -	up_read(&cxl_region_rwsem);
>  
>  	return rc;
>  }

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

[Jonathan Cameron]

On Tue, 11 Feb 2025 15:57:22 +0800
Li Ming <ming.li@zohomail.com> 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.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

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

[Jonathan Cameron]

On Tue, 11 Feb 2025 15:57:23 +0800
Li Ming <ming.li@zohomail.com> 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.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Seems reasonable to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

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

[Jonathan Cameron]

On Tue, 11 Feb 2025 15:57:24 +0800
Li Ming <ming.li@zohomail.com> 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().
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.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 2bb2782a72ad..4d753520a79a 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -384,35 +384,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 v1 5/7] cxl/core: Use guard() to drop goto pattern of cxl_dpa_alloc()

[Jonathan Cameron]

On Tue, 11 Feb 2025 15:57:25 +0800
Li Ming <ming.li@zohomail.com> 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.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

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

[Jonathan Cameron]

On Tue, 11 Feb 2025 15:57:26 +0800
Li Ming <ming.li@zohomail.com> 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.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

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

[Jonathan Cameron]

On Tue, 11 Feb 2025 15:57:27 +0800
Li Ming <ming.li@zohomail.com> wrote:

> Some operations need to be procted by the cxl_region_rwsem in construct
> region(). Currently, construct_region() uses down_write() and up_write()
> for the cxl_region_rwsem, 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_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_region().
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
>  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 36f3771818d3..170278bdcedc 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3217,49 +3217,31 @@ static int match_region_by_range(struct device *dev, const void *data)
>  	return 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)
> +static int __construct_region(struct cxl_region *cxlr,

This is only factoring out part, so I'm not sure the naming makes sense.
I don't have a better name however as this is doing various different things...
setup_region() doesn't feel right either...


> +			      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));
> @@ -3282,7 +3264,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__,
> @@ -3291,14 +3273,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_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 v1 7/7] cxl/region: Drop goto pattern of construct_region()

[Dave Jiang]

On 2/11/25 10:36 AM, Jonathan Cameron wrote:
> On Tue, 11 Feb 2025 15:57:27 +0800
> Li Ming <ming.li@zohomail.com> wrote:
> 
>> Some operations need to be procted by the cxl_region_rwsem in construct
>> region(). Currently, construct_region() uses down_write() and up_write()
>> for the cxl_region_rwsem, 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_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_region().
>>
>> Signed-off-by: Li Ming <ming.li@zohomail.com>
>> ---
>>  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 36f3771818d3..170278bdcedc 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -3217,49 +3217,31 @@ static int match_region_by_range(struct device *dev, const void *data)
>>  	return 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)
>> +static int __construct_region(struct cxl_region *cxlr,
> 
> This is only factoring out part, so I'm not sure the naming makes sense.
> I don't have a better name however as this is doing various different things...
> setup_region() doesn't feel right either...

setup_auto_region()? construct_auto_region()?

DJ

> 
> 
>> +			      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));
>> @@ -3282,7 +3264,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__,
>> @@ -3291,14 +3273,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_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 v1 1/7] cxl/core: Use guard() to replace open-coded down_read/write()

[Li Ming]

On 2/12/2025 1:26 AM, Jonathan Cameron wrote:
> On Tue, 11 Feb 2025 15:57:21 +0800
> Li Ming <ming.li@zohomail.com> 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.
>>
>> Signed-off-by: Li Ming <ming.li@zohomail.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> I wondered if it was worth doing some early returns and dropping
> local variables, but on balance think it isn't worth doing.
>
>
>> @@ -364,10 +362,9 @@ 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);
> Could return resource_size() here and return 0 below.
> I'm not sure it gains us much though. 

Right, I will do it in V2.


>> -	up_read(&cxl_dpa_rwsem);
>>  
>>  	return size;
>>  }
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index e8d11a988fd9..e3bb33109ced 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -3216,10 +3216,9 @@ 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;
> could do return 1 here and return 0 but it doesn't gain us much.

Will do it in V2. thanks for review.


Ming

>
>> -	up_read(&cxl_region_rwsem);
>>  
>>  	return rc;
>>  }
>

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

[Li Ming]

On 2/12/2025 5:24 AM, Dave Jiang wrote:
>
> On 2/11/25 10:36 AM, Jonathan Cameron wrote:
>> On Tue, 11 Feb 2025 15:57:27 +0800
>> Li Ming <ming.li@zohomail.com> wrote:
>>
>>> Some operations need to be procted by the cxl_region_rwsem in construct
>>> region(). Currently, construct_region() uses down_write() and up_write()
>>> for the cxl_region_rwsem, 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_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_region().
>>>
>>> Signed-off-by: Li Ming <ming.li@zohomail.com>
>>> ---
>>>  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 36f3771818d3..170278bdcedc 100644
>>> --- a/drivers/cxl/core/region.c
>>> +++ b/drivers/cxl/core/region.c
>>> @@ -3217,49 +3217,31 @@ static int match_region_by_range(struct device *dev, const void *data)
>>>  	return 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)
>>> +static int __construct_region(struct cxl_region *cxlr,
>> This is only factoring out part, so I'm not sure the naming makes sense.
>> I don't have a better name however as this is doing various different things...
>> setup_region() doesn't feel right either...
> setup_auto_region()? construct_auto_region()?
>
> DJ

Will use construct_auto_region() in V2 if no a better name, thanks for that.


Ming