1. Patchew
  2. linux
  3. [PATCH v1 0/7] Use guard() instead of rwsem locking
  4. View patch

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

Li Ming posted 7 patches 10 months, 1 week ago
There is a newer version of this series
[PATCH v1 1/7] cxl/core: Use guard() to replace open-coded down_read/write()
Posted by Li Ming 10 months, 1 week ago 
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
Re: [PATCH v1 1/7] cxl/core: Use guard() to replace open-coded down_read/write()
Posted by Jonathan Cameron 10 months, 1 week ago 
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 1/7] cxl/core: Use guard() to replace open-coded down_read/write()
Posted by Li Ming 10 months, 1 week ago 
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;
>>  }
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.