lkml.org 
[lkml]   [2022]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] livepatch: Move error print out of lock protection in klp_enable_patch()
From
Date


On 2022/9/1 21:24, Joe Lawrence wrote:
> 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 thought it was because mutex_lock()/mutex_unlock() was close enough to
reduce a "&klp_mutex" operation. Now, I was wrong.

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

Sorry. This is what I should have done. I built it on x86_64 with gcc-8.4.0

>
> 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.

OK. Should I send v2 to update commit message?

>
>> 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:

Yes

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

Thanks

>

--
Regards,
Zhen Lei

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