Messages in this thread | | | Date | Thu, 1 Sep 2022 16:18:23 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH] livepatch: Move error print out of lock protection in klp_enable_patch() |
| |
On Thu 2022-09-01 10:27:06, 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
Please, is this part of some longterm effort to reduce the size of the kernel? Or is this just some random observation?
> 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);
I do not see how this change could reliably reduce the code size.
As Joe wrote, it looks like a random effect that is specific to a particular compiler version, compilation options, and architecture.
I am against these kind of random microptimizations. It is just a call for problems. If you move printk() outside of a lock, you need to make sure that the information is not racy.
It might be safe in this particular case. But it is a bad practice. It adds an extra work. It is error-prone with questionable gain.
I am sorry but I NACK this patch. There must be better ways to reduce the kernel binary size.
Best Regards, Petr
| |