[PATCH v3 0/6] powerpc: use lock guards for mutex Set 1

[PATCH v3 0/6] powerpc: use lock guards for mutex Set 1

[Shrikanth Hegde]

This is an effort to make the code simpler by making use of lock
guards which were introduced in [1], which works by using __cleanup 
attributes. More details in v1 cover letter

v2->v3:
- Collects tags (Andrew Donnellan & Srikar Dronamraju)
- Address comments from Srikar Dronamraju 

v1->v2:
- Fix changelog of powernv (Andrew Donnellan)
- use scoped_guard in couple of places to avoid holding mutex
  un-necessarily (Peter Zijlstra)

[1]: https://lkml.kernel.org/r/20230612093537.614161713%40infradead.org
v1: https://lore.kernel.org/all/20250314054544.1998928-1-sshegde@linux.ibm.com/#t
v2: https://lore.kernel.org/all/20250314114502.2083434-1-sshegde@linux.ibm.com/

Shrikanth Hegde (6):
  powerpc: eeh: use lock guard for mutex
  powerpc: rtas: use lock guard for mutex
  powerpc: fadump: use lock guard for mutex
  powerpc: book3s: vas: use lock guard for mutex
  powerpc: powernv: ocxl: use lock guard for mutex
  powerpc: sysdev: use lock guard for mutex

 arch/powerpc/kernel/eeh.c                   | 20 +++----
 arch/powerpc/kernel/fadump.c                | 11 ++--
 arch/powerpc/kernel/rtas_flash.c            | 64 +++++++--------------
 arch/powerpc/platforms/book3s/vas-api.c     | 32 +++++------
 arch/powerpc/platforms/powernv/ocxl.c       | 12 +---
 arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c | 14 +----
 6 files changed, 50 insertions(+), 103 deletions(-)

-- 
2.39.3

[PATCH v3 1/6] powerpc: eeh: use lock guard for mutex

[Shrikanth Hegde]

use guard(mutex) for scope based resource management of mutex.
This would make the code simpler and easier to maintain.

More details on lock guards can be found at
https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u

Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/kernel/eeh.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 83fe99861eb1..929474c0ec77 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1208,16 +1208,16 @@ int eeh_dev_open(struct pci_dev *pdev)
 	struct eeh_dev *edev;
 	int ret = -ENODEV;
 
-	mutex_lock(&eeh_dev_mutex);
+	guard(mutex)(&eeh_dev_mutex);
 
 	/* No PCI device ? */
 	if (!pdev)
-		goto out;
+		return ret;
 
 	/* No EEH device or PE ? */
 	edev = pci_dev_to_eeh_dev(pdev);
 	if (!edev || !edev->pe)
-		goto out;
+		return ret;
 
 	/*
 	 * The PE might have been put into frozen state, but we
@@ -1227,16 +1227,12 @@ int eeh_dev_open(struct pci_dev *pdev)
 	 */
 	ret = eeh_pe_change_owner(edev->pe);
 	if (ret)
-		goto out;
+		return ret;
 
 	/* Increase PE's pass through count */
 	atomic_inc(&edev->pe->pass_dev_cnt);
-	mutex_unlock(&eeh_dev_mutex);
 
 	return 0;
-out:
-	mutex_unlock(&eeh_dev_mutex);
-	return ret;
 }
 EXPORT_SYMBOL_GPL(eeh_dev_open);
 
@@ -1252,22 +1248,20 @@ void eeh_dev_release(struct pci_dev *pdev)
 {
 	struct eeh_dev *edev;
 
-	mutex_lock(&eeh_dev_mutex);
+	guard(mutex)(&eeh_dev_mutex);
 
 	/* No PCI device ? */
 	if (!pdev)
-		goto out;
+		return;
 
 	/* No EEH device ? */
 	edev = pci_dev_to_eeh_dev(pdev);
 	if (!edev || !edev->pe || !eeh_pe_passed(edev->pe))
-		goto out;
+		return;
 
 	/* Decrease PE's pass through count */
 	WARN_ON(atomic_dec_if_positive(&edev->pe->pass_dev_cnt) < 0);
 	eeh_pe_change_owner(edev->pe);
-out:
-	mutex_unlock(&eeh_dev_mutex);
 }
 EXPORT_SYMBOL(eeh_dev_release);
 
-- 
2.39.3

[PATCH v3 2/6] powerpc: rtas: use lock guard for mutex

[Shrikanth Hegde]

use guard(mutex) for scope based resource management of mutex.
This would make the code simpler and easier to maintain.

More details on lock guards can be found at
https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u

Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/kernel/rtas_flash.c | 64 ++++++++++----------------------
 1 file changed, 20 insertions(+), 44 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index 5407024881e5..583dc16e9d3c 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -312,13 +312,13 @@ static ssize_t rtas_flash_write(struct file *file, const char __user *buffer,
 {
 	struct rtas_update_flash_t *const uf = &rtas_update_flash_data;
 	char *p;
-	int next_free, rc;
+	int next_free;
 	struct flash_block_list *fl;
 
-	mutex_lock(&rtas_update_flash_mutex);
+	guard(mutex)(&rtas_update_flash_mutex);
 
 	if (uf->status == FLASH_AUTH || count == 0)
-		goto out;	/* discard data */
+		return count;	/* discard data */
 
 	/* In the case that the image is not ready for flashing, the memory
 	 * allocated for the block list will be freed upon the release of the 
@@ -327,7 +327,7 @@ static ssize_t rtas_flash_write(struct file *file, const char __user *buffer,
 	if (uf->flist == NULL) {
 		uf->flist = kmem_cache_zalloc(flash_block_cache, GFP_KERNEL);
 		if (!uf->flist)
-			goto nomem;
+			return -ENOMEM;
 	}
 
 	fl = uf->flist;
@@ -338,7 +338,7 @@ static ssize_t rtas_flash_write(struct file *file, const char __user *buffer,
 		/* Need to allocate another block_list */
 		fl->next = kmem_cache_zalloc(flash_block_cache, GFP_KERNEL);
 		if (!fl->next)
-			goto nomem;
+			return -ENOMEM;
 		fl = fl->next;
 		next_free = 0;
 	}
@@ -347,25 +347,17 @@ static ssize_t rtas_flash_write(struct file *file, const char __user *buffer,
 		count = RTAS_BLK_SIZE;
 	p = kmem_cache_zalloc(flash_block_cache, GFP_KERNEL);
 	if (!p)
-		goto nomem;
+		return -ENOMEM;
 	
 	if(copy_from_user(p, buffer, count)) {
 		kmem_cache_free(flash_block_cache, p);
-		rc = -EFAULT;
-		goto error;
+		return -EFAULT;
 	}
 	fl->blocks[next_free].data = p;
 	fl->blocks[next_free].length = count;
 	fl->num_blocks++;
-out:
-	mutex_unlock(&rtas_update_flash_mutex);
-	return count;
 
-nomem:
-	rc = -ENOMEM;
-error:
-	mutex_unlock(&rtas_update_flash_mutex);
-	return rc;
+	return count;
 }
 
 /*
@@ -405,19 +397,18 @@ static ssize_t manage_flash_write(struct file *file, const char __user *buf,
 	static const char reject_str[] = "0";
 	static const char commit_str[] = "1";
 	char stkbuf[10];
-	int op, rc;
+	int op;
 
-	mutex_lock(&rtas_manage_flash_mutex);
+	guard(mutex)(&rtas_manage_flash_mutex);
 
 	if ((args_buf->status == MANAGE_AUTH) || (count == 0))
-		goto out;
+		return count;
 		
 	op = -1;
 	if (buf) {
 		if (count > 9) count = 9;
-		rc = -EFAULT;
 		if (copy_from_user (stkbuf, buf, count))
-			goto error;
+			return -EFAULT;
 		if (strncmp(stkbuf, reject_str, strlen(reject_str)) == 0) 
 			op = RTAS_REJECT_TMP_IMG;
 		else if (strncmp(stkbuf, commit_str, strlen(commit_str)) == 0) 
@@ -425,18 +416,11 @@ static ssize_t manage_flash_write(struct file *file, const char __user *buf,
 	}
 	
 	if (op == -1) {   /* buf is empty, or contains invalid string */
-		rc = -EINVAL;
-		goto error;
+		return -EINVAL;
 	}
 
 	manage_flash(args_buf, op);
-out:
-	mutex_unlock(&rtas_manage_flash_mutex);
 	return count;
-
-error:
-	mutex_unlock(&rtas_manage_flash_mutex);
-	return rc;
 }
 
 /*
@@ -499,16 +483,14 @@ static ssize_t validate_flash_write(struct file *file, const char __user *buf,
 {
 	struct rtas_validate_flash_t *const args_buf =
 		&rtas_validate_flash_data;
-	int rc;
 
-	mutex_lock(&rtas_validate_flash_mutex);
+	guard(mutex)(&rtas_validate_flash_mutex);
 
 	/* We are only interested in the first 4K of the
 	 * candidate image */
 	if ((*off >= VALIDATE_BUF_SIZE) || 
 		(args_buf->status == VALIDATE_AUTH)) {
 		*off += count;
-		mutex_unlock(&rtas_validate_flash_mutex);
 		return count;
 	}
 
@@ -519,20 +501,14 @@ static ssize_t validate_flash_write(struct file *file, const char __user *buf,
 		args_buf->status = VALIDATE_INCOMPLETE;
 	}
 
-	if (!access_ok(buf, count)) {
-		rc = -EFAULT;
-		goto done;
-	}
-	if (copy_from_user(args_buf->buf + *off, buf, count)) {
-		rc = -EFAULT;
-		goto done;
-	}
+	if (!access_ok(buf, count))
+		return -EFAULT;
+
+	if (copy_from_user(args_buf->buf + *off, buf, count))
+		return -EFAULT;
 
 	*off += count;
-	rc = count;
-done:
-	mutex_unlock(&rtas_validate_flash_mutex);
-	return rc;
+	return count;
 }
 
 static int validate_flash_release(struct inode *inode, struct file *file)
-- 
2.39.3

[PATCH v3 3/6] powerpc: fadump: use lock guard for mutex

[Shrikanth Hegde]

use scoped_guard for scope based resource management of mutex.
This would make the code simpler and easier to maintain.

More details on lock guards can be found at
https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u

Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/kernel/fadump.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index df16c7f547ab..b8c7993c5bb1 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -1375,15 +1375,12 @@ static void fadump_free_elfcorehdr_buf(void)
 
 static void fadump_invalidate_release_mem(void)
 {
-	mutex_lock(&fadump_mutex);
-	if (!fw_dump.dump_active) {
-		mutex_unlock(&fadump_mutex);
-		return;
+	scoped_guard(mutex, &fadump_mutex) {
+		if (!fw_dump.dump_active)
+			return;
+		fadump_cleanup();
 	}
 
-	fadump_cleanup();
-	mutex_unlock(&fadump_mutex);
-
 	fadump_free_elfcorehdr_buf();
 	fadump_release_memory(fw_dump.boot_mem_top, memblock_end_of_DRAM());
 	fadump_free_cpu_notes_buf();
-- 
2.39.3

Re: [PATCH v3 3/6] powerpc: fadump: use lock guard for mutex

[Sourabh Jain]

On 05/05/25 13:23, Shrikanth Hegde wrote:
> use scoped_guard for scope based resource management of mutex.
> This would make the code simpler and easier to maintain.
>
> More details on lock guards can be found at
> https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u
>
> Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
>   arch/powerpc/kernel/fadump.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index df16c7f547ab..b8c7993c5bb1 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -1375,15 +1375,12 @@ static void fadump_free_elfcorehdr_buf(void)
>   
>   static void fadump_invalidate_release_mem(void)
>   {
> -	mutex_lock(&fadump_mutex);
> -	if (!fw_dump.dump_active) {
> -		mutex_unlock(&fadump_mutex);
> -		return;
> +	scoped_guard(mutex, &fadump_mutex) {
> +		if (!fw_dump.dump_active)
> +			return;
> +		fadump_cleanup();
>   	}
>   
> -	fadump_cleanup();
> -	mutex_unlock(&fadump_mutex);
> -
>   	fadump_free_elfcorehdr_buf();
>   	fadump_release_memory(fw_dump.boot_mem_top, memblock_end_of_DRAM());
>   	fadump_free_cpu_notes_buf();

I tried to understand how scoped_guard gets unwrapped and what changes
it brings to the assembly of the update function. However, with GCC version
11.5.0 20240719 (Red Hat 11.5.0-5), identical assembly was generated for the
fadump_invalidate_release_mem function with or without this patch.

Which was a surprise to me because there are lot macros and compiler
magic involved here to call destructor ( for example 
https://clang.llvm.org/docs/AttributeReference.html#cleanup)
when a variable goes out of scope.

c000000000053978 <fadump_invalidate_release_mem.part.0>:
c000000000053978:       ae 01 4c 3c     addis   r2,r12,430
c00000000005397c:       88 47 42 38     addi    r2,r2,18312
c000000000053980:       a6 02 08 7c     mflr    r0
c000000000053984:       11 57 02 48     bl      c000000000079094 <_mcount>
c000000000053988:       a6 02 08 7c     mflr    r0
c00000000005398c:       f8 ff e1 fb     std     r31,-8(r1)
c000000000053990:       f0 ff c1 fb     std     r30,-16(r1)
c000000000053994:       1f 01 e2 3f     addis   r31,r2,287
c000000000053998:       30 ea ff 3b     addi    r31,r31,-5584
c00000000005399c:       10 00 01 f8     std     r0,16(r1)
c0000000000539a0:       81 ff 21 f8     stdu    r1,-128(r1)
c0000000000539a4:       18 00 41 f8     std     r2,24(r1)
c0000000000539a8:       ad fe ff 4b     bl      c000000000053854 
<fadump_cleanup+0x8>
c0000000000539ac:       c2 00 62 3c     addis   r3,r2,194
c0000000000539b0:       98 c3 63 38     addi    r3,r3,-15464
c0000000000539b4:       c9 1d 06 49     bl      c0000000010b577c 
<mutex_unlock+0x8>
c0000000000539b8:       00 00 00 60     nop
c0000000000539bc:       1f 01 22 3d     addis   r9,r2,287
snip...


Also, fadump_invalidate_release_mem() is only called in the fadump 
kernel in two scenarios
to release the reserved memory:

1. After dump collection
2. When fadump fails to process the dump

So even if the compiler messes up something here, there is no impact on 
dump collection as such.

So changes looks good to me:
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>

Re: [PATCH v3 3/6] powerpc: fadump: use lock guard for mutex

[Shrikanth Hegde]

On 5/8/25 11:23, Sourabh Jain wrote:
> 

Hi Sourabh.

> On 05/05/25 13:23, Shrikanth Hegde wrote:
>> use scoped_guard for scope based resource management of mutex.
>> This would make the code simpler and easier to maintain.
>>
>> More details on lock guards can be found at
>> https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u
>>
>> Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> ---
>>   arch/powerpc/kernel/fadump.c | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index df16c7f547ab..b8c7993c5bb1 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -1375,15 +1375,12 @@ static void fadump_free_elfcorehdr_buf(void)
>>   static void fadump_invalidate_release_mem(void)
>>   {
>> -    mutex_lock(&fadump_mutex);
>> -    if (!fw_dump.dump_active) {
>> -        mutex_unlock(&fadump_mutex);
>> -        return;
>> +    scoped_guard(mutex, &fadump_mutex) {
>> +        if (!fw_dump.dump_active)
>> +            return;
>> +        fadump_cleanup();
>>       }
>> -    fadump_cleanup();
>> -    mutex_unlock(&fadump_mutex);
>> -
>>       fadump_free_elfcorehdr_buf();
>>       fadump_release_memory(fw_dump.boot_mem_top, 
>> memblock_end_of_DRAM());
>>       fadump_free_cpu_notes_buf();
> 
> I tried to understand how scoped_guard gets unwrapped and what changes
> it brings to the assembly of the update function. However, with GCC version
> 11.5.0 20240719 (Red Hat 11.5.0-5), identical assembly was generated for 
> the
> fadump_invalidate_release_mem function with or without this patch.
> 
> Which was a surprise to me because there are lot macros and compiler
> magic involved here to call destructor ( for example https:// 
> clang.llvm.org/docs/AttributeReference.html#cleanup)
> when a variable goes out of scope.

that is nice to see.

> 
> c000000000053978 <fadump_invalidate_release_mem.part.0>:
> c000000000053978:       ae 01 4c 3c     addis   r2,r12,430
> c00000000005397c:       88 47 42 38     addi    r2,r2,18312
> c000000000053980:       a6 02 08 7c     mflr    r0
> c000000000053984:       11 57 02 48     bl      c000000000079094 <_mcount>
> c000000000053988:       a6 02 08 7c     mflr    r0
> c00000000005398c:       f8 ff e1 fb     std     r31,-8(r1)
> c000000000053990:       f0 ff c1 fb     std     r30,-16(r1)
> c000000000053994:       1f 01 e2 3f     addis   r31,r2,287
> c000000000053998:       30 ea ff 3b     addi    r31,r31,-5584
> c00000000005399c:       10 00 01 f8     std     r0,16(r1)
> c0000000000539a0:       81 ff 21 f8     stdu    r1,-128(r1)
> c0000000000539a4:       18 00 41 f8     std     r2,24(r1)
> c0000000000539a8:       ad fe ff 4b     bl      c000000000053854 
> <fadump_cleanup+0x8>
> c0000000000539ac:       c2 00 62 3c     addis   r3,r2,194
> c0000000000539b0:       98 c3 63 38     addi    r3,r3,-15464
> c0000000000539b4:       c9 1d 06 49     bl      c0000000010b577c 
> <mutex_unlock+0x8>
> c0000000000539b8:       00 00 00 60     nop
> c0000000000539bc:       1f 01 22 3d     addis   r9,r2,287
> snip...
> 
> 
> Also, fadump_invalidate_release_mem() is only called in the fadump 
> kernel in two scenarios
> to release the reserved memory:
> 
> 1. After dump collection
> 2. When fadump fails to process the dump
> 
> So even if the compiler messes up something here, there is no impact on 
> dump collection as such.

If there is a compiler mess up we will have a much bigger issue, since 
these are quite widely used in core areas such as scheduler, timers etc.

> 
> So changes looks good to me:
> Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>

Thanks for taking a look and reviewing it.

[PATCH v3 4/6] powerpc: book3s: vas: use lock guard for mutex

[Shrikanth Hegde]

use lock guards for scope based resource management of mutex.
This would make the code simpler and easier to maintain.

More details on lock guards can be found at
https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u

This shows the use of both guard and scoped_guard

Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/platforms/book3s/vas-api.c | 32 ++++++++++---------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
index 0b6365d85d11..d7462c16d828 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -425,23 +425,22 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
 		return VM_FAULT_SIGBUS;
 	}
 
-	mutex_lock(&txwin->task_ref.mmap_mutex);
 	/*
 	 * The window may be inactive due to lost credit (Ex: core
 	 * removal with DLPAR). If the window is active again when
 	 * the credit is available, map the new paste address at the
 	 * window virtual address.
 	 */
-	if (txwin->status == VAS_WIN_ACTIVE) {
-		paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
-		if (paste_addr) {
-			fault = vmf_insert_pfn(vma, vma->vm_start,
-					(paste_addr >> PAGE_SHIFT));
-			mutex_unlock(&txwin->task_ref.mmap_mutex);
-			return fault;
+	scoped_guard(mutex, &txwin->task_ref.mmap_mutex) {
+		if (txwin->status == VAS_WIN_ACTIVE) {
+			paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
+			if (paste_addr) {
+				fault = vmf_insert_pfn(vma, vma->vm_start,
+						(paste_addr >> PAGE_SHIFT));
+				return fault;
+			}
 		}
 	}
-	mutex_unlock(&txwin->task_ref.mmap_mutex);
 
 	/*
 	 * Received this fault due to closing the actual window.
@@ -494,9 +493,8 @@ static void vas_mmap_close(struct vm_area_struct *vma)
 		return;
 	}
 
-	mutex_lock(&txwin->task_ref.mmap_mutex);
-	txwin->task_ref.vma = NULL;
-	mutex_unlock(&txwin->task_ref.mmap_mutex);
+	scoped_guard(mutex, &txwin->task_ref.mmap_mutex)
+		txwin->task_ref.vma = NULL;
 }
 
 static const struct vm_operations_struct vas_vm_ops = {
@@ -543,18 +541,16 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 	 * close/open event and allows mmap() only when the window is
 	 * active.
 	 */
-	mutex_lock(&txwin->task_ref.mmap_mutex);
+	guard(mutex)(&txwin->task_ref.mmap_mutex);
 	if (txwin->status != VAS_WIN_ACTIVE) {
 		pr_err("Window is not active\n");
-		rc = -EACCES;
-		goto out;
+		return -EACCES;
 	}
 
 	paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
 	if (!paste_addr) {
 		pr_err("Window paste address failed\n");
-		rc = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	pfn = paste_addr >> PAGE_SHIFT;
@@ -574,8 +570,6 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
 	txwin->task_ref.vma = vma;
 	vma->vm_ops = &vas_vm_ops;
 
-out:
-	mutex_unlock(&txwin->task_ref.mmap_mutex);
 	return rc;
 }
 
-- 
2.39.3

[PATCH v3 5/6] powerpc: powernv: ocxl: use lock guard for mutex

[Shrikanth Hegde]

use guard(mutex) for scope based resource management of mutex.
This would make the code simpler and easier to maintain.

More details on lock guards can be found at
https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u

Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/ocxl.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index 64a9c7125c29..f8139948348e 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -172,12 +172,11 @@ static void pnv_ocxl_fixup_actag(struct pci_dev *dev)
 	if (phb->type != PNV_PHB_NPU_OCAPI)
 		return;
 
-	mutex_lock(&links_list_lock);
+	guard(mutex)(&links_list_lock);
 
 	link = find_link(dev);
 	if (!link) {
 		dev_warn(&dev->dev, "couldn't update actag information\n");
-		mutex_unlock(&links_list_lock);
 		return;
 	}
 
@@ -206,7 +205,6 @@ static void pnv_ocxl_fixup_actag(struct pci_dev *dev)
 	dev_dbg(&dev->dev, "total actags for function: %d\n",
 		link->fn_desired_actags[PCI_FUNC(dev->devfn)]);
 
-	mutex_unlock(&links_list_lock);
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pnv_ocxl_fixup_actag);
 
@@ -253,12 +251,11 @@ int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled,
 {
 	struct npu_link *link;
 
-	mutex_lock(&links_list_lock);
+	guard(mutex)(&links_list_lock);
 
 	link = find_link(dev);
 	if (!link) {
 		dev_err(&dev->dev, "actag information not found\n");
-		mutex_unlock(&links_list_lock);
 		return -ENODEV;
 	}
 	/*
@@ -274,7 +271,6 @@ int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled,
 	*enabled   = link->fn_actags[PCI_FUNC(dev->devfn)].count;
 	*supported = link->fn_desired_actags[PCI_FUNC(dev->devfn)];
 
-	mutex_unlock(&links_list_lock);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pnv_ocxl_get_actag);
@@ -293,12 +289,11 @@ int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count)
 	 *
 	 * We only support one AFU-carrying function for now.
 	 */
-	mutex_lock(&links_list_lock);
+	guard(mutex)(&links_list_lock);
 
 	link = find_link(dev);
 	if (!link) {
 		dev_err(&dev->dev, "actag information not found\n");
-		mutex_unlock(&links_list_lock);
 		return -ENODEV;
 	}
 
@@ -309,7 +304,6 @@ int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count)
 			break;
 		}
 
-	mutex_unlock(&links_list_lock);
 	dev_dbg(&dev->dev, "%d PASIDs available for function\n",
 		rc ? 0 : *count);
 	return rc;
-- 
2.39.3

[PATCH v3 6/6] powerpc: sysdev: use lock guard for mutex

[Shrikanth Hegde]

use guard(mutex) for scope based resource management of mutex
This would make the code simpler and easier to maintain.

More details on lock guards can be found at
https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u

Reviewed-by: Srikar Dronamraju <srikar@linux.ibm.com>
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c b/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
index ce6c739c51e5..06d9101a5d49 100644
--- a/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
+++ b/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
@@ -75,7 +75,7 @@ static ssize_t fsl_timer_wakeup_store(struct device *dev,
 	if (kstrtoll(buf, 0, &interval))
 		return -EINVAL;
 
-	mutex_lock(&sysfs_lock);
+	guard(mutex)(&sysfs_lock);
 
 	if (fsl_wakeup->timer) {
 		disable_irq_wake(fsl_wakeup->timer->irq);
@@ -83,31 +83,23 @@ static ssize_t fsl_timer_wakeup_store(struct device *dev,
 		fsl_wakeup->timer = NULL;
 	}
 
-	if (!interval) {
-		mutex_unlock(&sysfs_lock);
+	if (!interval)
 		return count;
-	}
 
 	fsl_wakeup->timer = mpic_request_timer(fsl_mpic_timer_irq,
 						fsl_wakeup, interval);
-	if (!fsl_wakeup->timer) {
-		mutex_unlock(&sysfs_lock);
+	if (!fsl_wakeup->timer)
 		return -EINVAL;
-	}
 
 	ret = enable_irq_wake(fsl_wakeup->timer->irq);
 	if (ret) {
 		mpic_free_timer(fsl_wakeup->timer);
 		fsl_wakeup->timer = NULL;
-		mutex_unlock(&sysfs_lock);
-
 		return ret;
 	}
 
 	mpic_start_timer(fsl_wakeup->timer);
 
-	mutex_unlock(&sysfs_lock);
-
 	return count;
 }
 
-- 
2.39.3

Re: [PATCH v3 0/6] powerpc: use lock guards for mutex Set 1

[Shrikanth Hegde]

On 5/5/25 13:23, Shrikanth Hegde wrote:
> This is an effort to make the code simpler by making use of lock
> guards which were introduced in [1], which works by using __cleanup
> attributes. More details in v1 cover letter
> 

compile/boot tested on PowerNV(P9). Also ran eeh selftests.
No regressions were seen compared to baseline.

Re: [PATCH v3 0/6] powerpc: use lock guards for mutex Set 1

[Venkat Rao Bagalkote]

On 05/05/25 1:23 pm, Shrikanth Hegde wrote:
> This is an effort to make the code simpler by making use of lock
> guards which were introduced in [1], which works by using __cleanup
> attributes. More details in v1 cover letter
>
> v2->v3:
> - Collects tags (Andrew Donnellan & Srikar Dronamraju)
> - Address comments from Srikar Dronamraju
>
> v1->v2:
> - Fix changelog of powernv (Andrew Donnellan)
> - use scoped_guard in couple of places to avoid holding mutex
>    un-necessarily (Peter Zijlstra)
>
> [1]: https://lkml.kernel.org/r/20230612093537.614161713%40infradead.org
> v1: https://lore.kernel.org/all/20250314054544.1998928-1-sshegde@linux.ibm.com/#t
> v2: https://lore.kernel.org/all/20250314114502.2083434-1-sshegde@linux.ibm.com/
>
> Shrikanth Hegde (6):
>    powerpc: eeh: use lock guard for mutex
>    powerpc: rtas: use lock guard for mutex
>    powerpc: fadump: use lock guard for mutex
>    powerpc: book3s: vas: use lock guard for mutex
>    powerpc: powernv: ocxl: use lock guard for mutex
>    powerpc: sysdev: use lock guard for mutex
>
>   arch/powerpc/kernel/eeh.c                   | 20 +++----
>   arch/powerpc/kernel/fadump.c                | 11 ++--
>   arch/powerpc/kernel/rtas_flash.c            | 64 +++++++--------------
>   arch/powerpc/platforms/book3s/vas-api.c     | 32 +++++------
>   arch/powerpc/platforms/powernv/ocxl.c       | 12 +---
>   arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c | 14 +----
>   6 files changed, 50 insertions(+), 103 deletions(-)
>
Tested this patch-set by applying on top of powerpc repo, with HEAD: 
b4432656b36e5cc1d50a1f2dc15357543add530e, and Build and boot test is 
successful.

Also FADUMP and DLPAR operations are successful. Hence, for the series,


Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>


Regards,

Venkat.