lkml.org 
[lkml]   [2022]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH] livepatch: Move error print out of lock protection in klp_enable_patch()
Date
On 8/31/22 10:27 PM, Zhen Lei wrote:
> The patch->mod is not a protected object of mutex_lock(&klp_mutex). Since
> it's in the error handling branch, it might not be helpful to reduce lock
> conflicts, but it can reduce some code size.
>
> Before:
> text data bss dec hex filename
> 10330 464 8 10802 2a32 kernel/livepatch/core.o
>
> After:
> text data bss dec hex filename
> 10307 464 8 10779 2a1b kernel/livepatch/core.o
>

Is a size change expected, or is it just compiler fall out from
shuffling the code around a little bit?

I see some arches do a little better, some a little worse with gcc-9.3.0
cross compilers:

Before
------
text data bss dec hex filename
8490 600 8 9098 238a arm64/kernel/livepatch/core.o
9424 680 8 10112 2780 s390/kernel/livepatch/core.o
9802 228 4 10034 2732 ppc32/kernel/livepatch/core.o
13746 456 8 14210 3782 ppc64le/kernel/livepatch/core.o
10443 464 8 10915 2aa3 x86_64/kernel/livepatch/core.o


After
-----
text data bss dec hex filename
8514 600 8 9122 23a2 arm64/kernel/livepatch/core.o
9424 680 8 10112 2780 s390/kernel/livepatch/core.o
9818 228 4 10050 2742 ppc32/kernel/livepatch/core.o
13762 456 8 14226 3792 ppc64le/kernel/livepatch/core.o
10446 464 8 10918 2aa6 x86_64/kernel/livepatch/core.o

In which case, I'd just omit the size savings from the commit msg.

> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> kernel/livepatch/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 42f7e716d56bf72..cb7abc821a50584 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1041,9 +1041,9 @@ int klp_enable_patch(struct klp_patch *patch)
> mutex_lock(&klp_mutex);
>
> if (!klp_is_patch_compatible(patch)) {
> + mutex_unlock(&klp_mutex);
> pr_err("Livepatch patch (%s) is not compatible with the already installed livepatches.\n",
> patch->mod->name);
> - mutex_unlock(&klp_mutex);
> return -EINVAL;
> }
>
>

That said, I don't see anything obviously wrong about the change (we
don't need to sync our error msgs, right?) so:

Acked-by: Joe Lawrence <joe.lawrence@redhat.com>

--
Joe

\
 
 \ /
  Last update: 2022-09-01 15:26    [W:0.088 / U:1.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site