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
There is also an example of using scoped_guard.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
arch/powerpc/platforms/book3s/vas-api.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
index 0b6365d85d11..eb1a97271afb 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -425,7 +425,7 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
return VM_FAULT_SIGBUS;
}
- mutex_lock(&txwin->task_ref.mmap_mutex);
+ guard(mutex)(&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
@@ -437,11 +437,9 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
if (paste_addr) {
fault = vmf_insert_pfn(vma, vma->vm_start,
(paste_addr >> PAGE_SHIFT));
- mutex_unlock(&txwin->task_ref.mmap_mutex);
return fault;
}
}
- mutex_unlock(&txwin->task_ref.mmap_mutex);
/*
* Received this fault due to closing the actual window.
@@ -494,9 +492,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 +540,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 +569,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.3On Fri, Mar 14, 2025 at 11:15:42AM +0530, Shrikanth Hegde wrote:
> 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
>
> There is also an example of using scoped_guard.
>
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
> arch/powerpc/platforms/book3s/vas-api.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index 0b6365d85d11..eb1a97271afb 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -425,7 +425,7 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
> return VM_FAULT_SIGBUS;
> }
>
> - mutex_lock(&txwin->task_ref.mmap_mutex);
> + guard(mutex)(&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
> @@ -437,11 +437,9 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
> if (paste_addr) {
> fault = vmf_insert_pfn(vma, vma->vm_start,
> (paste_addr >> PAGE_SHIFT));
> - mutex_unlock(&txwin->task_ref.mmap_mutex);
> return fault;
> }
> }
> - mutex_unlock(&txwin->task_ref.mmap_mutex);
I had to open up this file to check, but this seems incorrect since you
now also run do_fail_paste() with the lock held, where previously you
did not.
> /*
> * Received this fault due to closing the actual window.
On 3/14/25 13:55, Peter Zijlstra wrote:
> On Fri, Mar 14, 2025 at 11:15:42AM +0530, Shrikanth Hegde wrote:
>> 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
>>
>> There is also an example of using scoped_guard.
>>
>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/book3s/vas-api.c | 19 ++++++-------------
>> 1 file changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
>> index 0b6365d85d11..eb1a97271afb 100644
>> --- a/arch/powerpc/platforms/book3s/vas-api.c
>> +++ b/arch/powerpc/platforms/book3s/vas-api.c
>> @@ -425,7 +425,7 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
>> return VM_FAULT_SIGBUS;
>> }
>>
>> - mutex_lock(&txwin->task_ref.mmap_mutex);
>> + guard(mutex)(&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
>> @@ -437,11 +437,9 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
>> if (paste_addr) {
>> fault = vmf_insert_pfn(vma, vma->vm_start,
>> (paste_addr >> PAGE_SHIFT));
>> - mutex_unlock(&txwin->task_ref.mmap_mutex);
>> return fault;
>> }
>> }
>> - mutex_unlock(&txwin->task_ref.mmap_mutex);
>
> I had to open up this file to check, but this seems incorrect since you
> now also run do_fail_paste() with the lock held, where previously you
> did not.
>
Yes. Got it. let me use scoped_guard for it as well. There is get_user
and other things in the fail parse and having it with mutex will not be
good.
I went through the rest of the patches too. It is mostly return after
mutex.
Only in Patch 5/6 there is additional debug statement. Let me put a
comment there.
>
>> /*
>> * Received this fault due to closing the actual window.
© 2016 - 2025 Red Hat, Inc.