linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] livepatch: Avoid CPU hogging with cond_resched
@ 2021-12-29 21:56 David Vernet
  2022-01-05 10:17 ` Miroslav Benes
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: David Vernet @ 2021-12-29 21:56 UTC (permalink / raw)
  To: live-patching, linux-kernel, jpoimboe, pmladek, jikos, mbenes,
	joe.lawrence
  Cc: void

When initializing a 'struct klp_object' in klp_init_object_loaded(), and
performing relocations in klp_resolve_symbols(), klp_find_object_symbol()
is invoked to look up the address of a symbol in an already-loaded module
(or vmlinux). This, in turn, calls kallsyms_on_each_symbol() or
module_kallsyms_on_each_symbol() to find the address of the symbol that is
being patched.

It turns out that symbol lookups often take up the most CPU time when
enabling and disabling a patch, and may hog the CPU and cause other tasks
on that CPU's runqueue to starve -- even in paths where interrupts are
enabled.  For example, under certain workloads, enabling a KLP patch with
many objects or functions may cause ksoftirqd to be starved, and thus for
interrupts to be backlogged and delayed. This may end up causing TCP
retransmits on the host where the KLP patch is being applied, and in
general, may cause any interrupts serviced by softirqd to be delayed while
the patch is being applied.

So as to ensure that kallsyms_on_each_symbol() does not end up hogging the
CPU, this patch adds a call to cond_resched() in kallsyms_on_each_symbol()
and module_kallsyms_on_each_symbol(), which are invoked when doing a symbol
lookup in vmlinux and a module respectively.  Without this patch, if a
live-patch is applied on a 36-core Intel host with heavy TCP traffic, a
~10x spike is observed in TCP retransmits while the patch is being applied.
Additionally, collecting sched events with perf indicates that ksoftirqd is
awakened ~1.3 seconds before it's eventually scheduled.  With the patch, no
increase in TCP retransmit events is observed, and ksoftirqd is scheduled
shortly after it's awakened.

Signed-off-by: David Vernet <void@manifault.com>
---
 kernel/kallsyms.c | 1 +
 kernel/module.c   | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 0ba87982d017..2a9afe484aec 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -223,6 +223,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 		ret = fn(data, namebuf, NULL, kallsyms_sym_address(i));
 		if (ret != 0)
 			return ret;
+		cond_resched();
 	}
 	return 0;
 }
diff --git a/kernel/module.c b/kernel/module.c
index 40ec9a030eec..c96160f7f3f5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4462,6 +4462,8 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
 				 mod, kallsyms_symbol_value(sym));
 			if (ret != 0)
 				goto out;
+
+			cond_resched();
 		}
 	}
 out:
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH] livepatch: Avoid CPU hogging with cond_resched
@ 2021-12-30  4:16 David Vernet
  2021-12-31 23:05 ` Kumar Kartikeya Dwivedi
  2022-01-03 16:04 ` Petr Mladek
  0 siblings, 2 replies; 14+ messages in thread
From: David Vernet @ 2021-12-30  4:16 UTC (permalink / raw)
  To: live-patching, linux-kernel, jpoimboe, pmladek, jikos, mbenes,
	joe.lawrence
  Cc: linux-modules, mcgrof, jeyu, bpf, ast, daniel, andrii, kafai,
	songliubraving, yhs, john.fastabend, kpsingh, netdev, memxor

Adding modules + BPF list and maintainers to this thread.

David Vernet <void@manifault.com> wrote on Wed [2021-Dec-29 13:56:47 -0800]:
> When initializing a 'struct klp_object' in klp_init_object_loaded(), and
> performing relocations in klp_resolve_symbols(), klp_find_object_symbol()
> is invoked to look up the address of a symbol in an already-loaded module
> (or vmlinux). This, in turn, calls kallsyms_on_each_symbol() or
> module_kallsyms_on_each_symbol() to find the address of the symbol that is
> being patched.
> 
> It turns out that symbol lookups often take up the most CPU time when
> enabling and disabling a patch, and may hog the CPU and cause other tasks
> on that CPU's runqueue to starve -- even in paths where interrupts are
> enabled.  For example, under certain workloads, enabling a KLP patch with
> many objects or functions may cause ksoftirqd to be starved, and thus for
> interrupts to be backlogged and delayed. This may end up causing TCP
> retransmits on the host where the KLP patch is being applied, and in
> general, may cause any interrupts serviced by softirqd to be delayed while
> the patch is being applied.
> 
> So as to ensure that kallsyms_on_each_symbol() does not end up hogging the
> CPU, this patch adds a call to cond_resched() in kallsyms_on_each_symbol()
> and module_kallsyms_on_each_symbol(), which are invoked when doing a symbol
> lookup in vmlinux and a module respectively.  Without this patch, if a
> live-patch is applied on a 36-core Intel host with heavy TCP traffic, a
> ~10x spike is observed in TCP retransmits while the patch is being applied.
> Additionally, collecting sched events with perf indicates that ksoftirqd is
> awakened ~1.3 seconds before it's eventually scheduled.  With the patch, no
> increase in TCP retransmit events is observed, and ksoftirqd is scheduled
> shortly after it's awakened.
> 
> Signed-off-by: David Vernet <void@manifault.com>
> ---
>  kernel/kallsyms.c | 1 +
>  kernel/module.c   | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 0ba87982d017..2a9afe484aec 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -223,6 +223,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
>  		ret = fn(data, namebuf, NULL, kallsyms_sym_address(i));
>  		if (ret != 0)
>  			return ret;
> +		cond_resched();
>  	}
>  	return 0;
>  }
> diff --git a/kernel/module.c b/kernel/module.c
> index 40ec9a030eec..c96160f7f3f5 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -4462,6 +4462,8 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
>  				 mod, kallsyms_symbol_value(sym));
>  			if (ret != 0)
>  				goto out;
> +
> +			cond_resched();
>  		}
>  	}
>  out:
> -- 
> 2.30.2
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-01-11  1:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-29 21:56 [PATCH] livepatch: Avoid CPU hogging with cond_resched David Vernet
2022-01-05 10:17 ` Miroslav Benes
2022-01-07  0:21 ` Song Liu
2022-01-07  8:17   ` Petr Mladek
2022-01-10 14:55     ` David Vernet
2022-01-07 13:03 ` Petr Mladek
2022-01-07 14:13 ` Joe Lawrence
2022-01-07 16:46   ` Song Liu
2022-01-10 16:16     ` Joe Lawrence
2022-01-11  1:49       ` Song Liu
2021-12-30  4:16 David Vernet
2021-12-31 23:05 ` Kumar Kartikeya Dwivedi
2022-01-03 16:04 ` Petr Mladek
2022-01-10 14:38   ` David Vernet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).