[PATCH] liveupdate: use scoped_guard for mutex in session operations

Chenghao Duan posted 1 patch 1 month, 2 weeks ago
kernel/liveupdate/luo_session.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
[PATCH] liveupdate: use scoped_guard for mutex in session operations
Posted by Chenghao Duan 1 month, 2 weeks ago
Replace manually paired mutex_lock/unlock with scoped_guard to align
with the coding style of the rest of the codebase and simplify locking
paths.

Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
---
 kernel/liveupdate/luo_session.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/kernel/liveupdate/luo_session.c b/kernel/liveupdate/luo_session.c
index f38b5b18f3f8..06307868a44e 100644
--- a/kernel/liveupdate/luo_session.c
+++ b/kernel/liveupdate/luo_session.c
@@ -290,9 +290,8 @@ static int luo_session_retrieve_fd(struct luo_session *session,
 	if (argp->fd < 0)
 		return argp->fd;
 
-	mutex_lock(&session->mutex);
-	err = luo_retrieve_file(&session->file_set, argp->token, &file);
-	mutex_unlock(&session->mutex);
+	scoped_guard(mutex, &session->mutex)
+		err = luo_retrieve_file(&session->file_set, argp->token, &file);
 	if (err < 0)
 		goto err_put_fd;
 
@@ -478,9 +477,8 @@ int luo_session_create(const char *name, struct file **filep)
 	if (err)
 		goto err_free;
 
-	mutex_lock(&session->mutex);
-	err = luo_session_getfile(session, filep);
-	mutex_unlock(&session->mutex);
+	scoped_guard(mutex, &session->mutex)
+		err = luo_session_getfile(session, filep);
 	if (err)
 		goto err_remove;
 	up_read(&luo_session_serialize_rwsem);
-- 
2.25.1
Re: [PATCH] liveupdate: use scoped_guard for mutex in session operations
Posted by Pratyush Yadav 1 month, 2 weeks ago
On Fri, Aug 14 2026, Chenghao Duan wrote:

> Replace manually paired mutex_lock/unlock with scoped_guard to align
> with the coding style of the rest of the codebase and simplify locking
> paths.

No. This is done explicitly because we don't want to mix gotos with the
automatic cleanup-style locking. I don't think we should change this.

[...]

-- 
Regards,
Pratyush Yadav
Re: [PATCH] liveupdate: use scoped_guard for mutex in session operations
Posted by Chenghao Duan 1 month, 1 week ago
On Fri, Aug 14, 2026 at 04:10:34PM +0200, Pratyush Yadav wrote:
> On Fri, Aug 14 2026, Chenghao Duan wrote:
> 
> > Replace manually paired mutex_lock/unlock with scoped_guard to align
> > with the coding style of the rest of the codebase and simplify locking
> > paths.
> 
> No. This is done explicitly because we don't want to mix gotos with the
> automatic cleanup-style locking. I don't think we should change this.
> 

Thank you for the clarification. I understand your concern about mixing
`goto`-based error handling with automatic cleanup-style locking.

I was wondering if these particular cases might still be suitable for
using `scoped_guard()`. The patch only changes two manually paired mutex
lock/unlock instances, and in both cases, the guard scope is limited to
the operation that requires the mutex. The lock is released before the
subsequent error-handling logic is executed, so the `goto` paths do not
cross the scope of the guard.

We could also use an explicit `scoped_guard { ... }` scope to make the
lifetime of the guard more obvious and easier to review.

Of course, I may be missing some broader considerations. If there are
other reasons why these cases should retain the explicit lock/unlock
pattern, I would be happy to follow your guidance.

Regards,
Chenghao

> [...]
> 
> -- 
> Regards,
> Pratyush Yadav
Re: [PATCH] liveupdate: use scoped_guard for mutex in session operations
Posted by Pratyush Yadav 1 month, 1 week ago
On Mon, Aug 17 2026, Chenghao Duan wrote:

> On Fri, Aug 14, 2026 at 04:10:34PM +0200, Pratyush Yadav wrote:
>> On Fri, Aug 14 2026, Chenghao Duan wrote:
>> 
>> > Replace manually paired mutex_lock/unlock with scoped_guard to align
>> > with the coding style of the rest of the codebase and simplify locking
>> > paths.
>> 
>> No. This is done explicitly because we don't want to mix gotos with the
>> automatic cleanup-style locking. I don't think we should change this.
>> 
>
> Thank you for the clarification. I understand your concern about mixing
> `goto`-based error handling with automatic cleanup-style locking.
>
> I was wondering if these particular cases might still be suitable for
> using `scoped_guard()`. The patch only changes two manually paired mutex
> lock/unlock instances, and in both cases, the guard scope is limited to
> the operation that requires the mutex. The lock is released before the
> subsequent error-handling logic is executed, so the `goto` paths do not
> cross the scope of the guard.
>
> We could also use an explicit `scoped_guard { ... }` scope to make the
> lifetime of the guard more obvious and easier to review.
>
> Of course, I may be missing some broader considerations. If there are
> other reasons why these cases should retain the explicit lock/unlock
> pattern, I would be happy to follow your guidance.

That's one of the recommendations of the cleanup API. From
include/linux/cleanup.h:

    Lastly, given that the benefit of cleanup helpers is removal of
    "goto", and that the "goto" statement can jump between scopes, the
    expectation is that usage of "goto" and cleanup helpers is never
    mixed in the same function. I.e. for a given routine, convert all
    resources that need a "goto" cleanup to scope-based cleanup, or
    convert none of them.

Which sort of makes sense I think. Also, cleanup-style guards can cause
compilation failures too. For example, if you do:

    err = sanity_check();
    if (err)
    	goto out;
    
    guard(mutex)(&my_lock);
    
    ...

This will fail to compile.

Now this problem doesn't exist with scoped guards as far as I can tell,
but I do see why it is recommended to not mix them with gotos.

-- 
Regards,
Pratyush Yadav