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

Shrikanth Hegde posted 6 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH 3/6] powerpc: fadump: use lock guard for mutex
Posted by Shrikanth Hegde 9 months, 1 week ago
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

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

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 4b371c738213..5fd2c546fd8c 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -1374,15 +1374,13 @@ static void fadump_free_elfcorehdr_buf(void)
 
 static void fadump_invalidate_release_mem(void)
 {
-	mutex_lock(&fadump_mutex);
+	guard(mutex)(&fadump_mutex);
+
 	if (!fw_dump.dump_active) {
-		mutex_unlock(&fadump_mutex);
 		return;
 	}
 
 	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 3/6] powerpc: fadump: use lock guard for mutex
Posted by Peter Zijlstra 9 months, 1 week ago
On Fri, Mar 14, 2025 at 11:15:41AM +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
> 
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
>  arch/powerpc/kernel/fadump.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 4b371c738213..5fd2c546fd8c 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -1374,15 +1374,13 @@ static void fadump_free_elfcorehdr_buf(void)
>  
>  static void fadump_invalidate_release_mem(void)
>  {
> -	mutex_lock(&fadump_mutex);
> +	guard(mutex)(&fadump_mutex);
> +
>  	if (!fw_dump.dump_active) {
> -		mutex_unlock(&fadump_mutex);
>  		return;
>  	}
>  
>  	fadump_cleanup();
> -	mutex_unlock(&fadump_mutex);
> -

This will result in running the below functions with the mutex held.

>  	fadump_free_elfcorehdr_buf();
>  	fadump_release_memory(fw_dump.boot_mem_top, memblock_end_of_DRAM());
>  	fadump_free_cpu_notes_buf();


The equivalent transformation for the above code would look like:

static void fadump_invalidate_release_mem(void)
{
	scoped_guard (mutex, &fadump_mutex) {
		if (!fw_dump.dump_active)
			return;

		fadump_cleanup();
	}

	fadump_free_elfcorehdr_buf();
	...
Re: [PATCH 3/6] powerpc: fadump: use lock guard for mutex
Posted by Shrikanth Hegde 9 months, 1 week ago

On 3/14/25 13:52, Peter Zijlstra wrote:

Thanks Peter for taking a look.

> On Fri, Mar 14, 2025 at 11:15:41AM +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
>>
>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> ---
>>   arch/powerpc/kernel/fadump.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 4b371c738213..5fd2c546fd8c 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -1374,15 +1374,13 @@ static void fadump_free_elfcorehdr_buf(void)
>>   
>>   static void fadump_invalidate_release_mem(void)
>>   {
>> -	mutex_lock(&fadump_mutex);
>> +	guard(mutex)(&fadump_mutex);
>> +
>>   	if (!fw_dump.dump_active) {
>> -		mutex_unlock(&fadump_mutex);
>>   		return;
>>   	}
>>   
>>   	fadump_cleanup();
>> -	mutex_unlock(&fadump_mutex);
>> -
> 
> This will result in running the below functions with the mutex held.
> 
>>   	fadump_free_elfcorehdr_buf();
>>   	fadump_release_memory(fw_dump.boot_mem_top, memblock_end_of_DRAM());
>>   	fadump_free_cpu_notes_buf();
> 

Ok. Got it, since the variable is still in scope unlock wont be called.
So, will use scoped_guard as you suggested below in v2.

> 
> The equivalent transformation for the above code would look like:
> 
> static void fadump_invalidate_release_mem(void)
> {
> 	scoped_guard (mutex, &fadump_mutex) {
> 		if (!fw_dump.dump_active)
> 			return;
> 
> 		fadump_cleanup();
> 	}
> 
> 	fadump_free_elfcorehdr_buf();
> 	...

ok.