From: "Radim Krčmář" <rkrcmar@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Subject: [PATCH 0/7] static_key: fix timer bugs & change api (a bit)
Date: Thu, 17 Oct 2013 12:10:23 +0200 [thread overview]
Message-ID: <1382004631-25895-1-git-send-email-rkrcmar@redhat.com> (raw)
While fixing a simple crash on kvm module unload
1: rate_limit timer can fire after we free its key's module memory.
I noticed other deficiencies in jump_label/static_key code
(The most important is 4.)
2: jump_label_rate_limit() uses an initializator and thus cannot be used
more than once, leaving no good way to change the timeout.
I have made the API easier on programmers: [1/7]
* timer is automatically flushed on rmmod
* jump_label_rate_limit() initializes only once
- pretty hacky, but we cannot automatically initialize on
kernel_init/insmod due to insufficient information and while I
would love getting it through another section, it is probably
better to do a useless check with this low-rate operation
* we could flush the timer on change
* 'init()' + 'set()' (+ 'exit()' ?) would be an alternative ...
3: schedule_delayed_work() does not queue the work if one is already
pending, but atomic_inc(&key->enabled) is called anyway in
static_key_slow_dec_deferred(), with the false belief it will be
decreased when the timer fires.
Fixed in [2,3,4/7], by addition of static_key_slow_inc_deferred().
I'm still not happy with the final design: we don't want delayed
decrease, we just don't want change more that once a interval.
A good solution should immediately enable/disable if interval since last
change has already passed.
I'll generalize ratelimit (probably) to suit our needs, but it won't be
quick, so we could use this in the meantime.
4: static_key_{true,false}() is a horrible name for a representation of
a boolean and its use is unnecessarily restricted
In patch [5/7], static keys are transformed so one key can be hinted
both ways. This is done by bloating the jump_entry.
Patch [6/7] changes the name to static_key_{likely,unlikely}() because
we no longer have incompatible behaviour.
5: jump_label_init() is called too late
I've seen some patches that debugged the case where we use static_keys
before patching.
Moving jump_label_init() near the top of start_kernel() should help us
avoid it. [7/7]
n: jump_label and static_key api should be split;
static_key_deferred isn't complete api, or subclass of static_key;
some functions should be renamed, some removed;
...
There are already some patches prepared, but the diffstat isn't pretty,
so I'm keeping them to ripen.
Applied on top of torvalds-3.12-rc5.
Radim Krčmář (7):
static_key: flush rate limit timer on rmmod
static_key: add static_key_slow_inc_deferred()
static_key: keep deferred enabled counter debt
static_key: use static_key_slow_inc_deferred()
jump_label: relax branch hinting restrictions
static_key: use {,un}likely instead of {tru,fals}e
init: execute jump_label_init() earlier
Documentation/static-keys.txt | 39 ++++++++------------
arch/arm/include/asm/jump_label.h | 19 +++++++---
arch/arm/kernel/jump_label.c | 2 +-
arch/mips/include/asm/jump_label.h | 19 +++++++---
arch/mips/kernel/jump_label.c | 2 +-
arch/powerpc/include/asm/jump_label.h | 19 +++++++---
arch/powerpc/kernel/jump_label.c | 2 +-
arch/s390/include/asm/jump_label.h | 19 +++++++---
arch/s390/kernel/jump_label.c | 2 +-
arch/sparc/include/asm/jump_label.h | 19 +++++++---
arch/sparc/kernel/jump_label.c | 2 +-
arch/x86/include/asm/jump_label.h | 19 +++++++---
arch/x86/include/asm/spinlock.h | 4 +--
arch/x86/kernel/jump_label.c | 32 +++--------------
arch/x86/kvm/lapic.c | 7 ++--
arch/x86/kvm/lapic.h | 6 ++--
arch/x86/kvm/mmu_audit.c | 2 +-
include/linux/context_tracking.h | 10 +++---
include/linux/jump_label.h | 66 +++++++++++++---------------------
include/linux/jump_label_ratelimit.h | 6 ++++
include/linux/memcontrol.h | 2 +-
include/linux/netfilter.h | 2 +-
include/linux/perf_event.h | 6 ++--
include/linux/tick.h | 2 +-
include/linux/tracepoint.h | 4 +--
include/linux/vtime.h | 2 +-
include/net/sock.h | 4 +--
init/main.c | 6 +++-
kernel/context_tracking.c | 4 +--
kernel/events/core.c | 6 ++--
kernel/jump_label.c | 68 ++++++++++++++++++++---------------
kernel/sched/core.c | 4 ++-
kernel/sched/cputime.c | 2 +-
kernel/sched/fair.c | 2 +-
kernel/sched/sched.h | 4 +--
lib/crc-t10dif.c | 2 +-
net/core/dev.c | 8 ++---
net/ipv4/udp.c | 4 +--
net/ipv6/udp.c | 4 +--
39 files changed, 231 insertions(+), 201 deletions(-)
--
1.8.3.1
next reply other threads:[~2013-10-17 10:17 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-17 10:10 Radim Krčmář [this message]
2013-10-17 10:10 ` [PATCH 1/7] static_key: flush rate limit timer on rmmod Radim Krčmář
2013-10-17 10:35 ` Paolo Bonzini
2013-10-18 7:24 ` Radim Krčmář
2013-10-17 10:10 ` [PATCH 2/7] static_key: add static_key_slow_inc_deferred() Radim Krčmář
2013-10-17 10:10 ` [PATCH 3/7] static_key: keep deferred enabled counter debt Radim Krčmář
2013-10-17 10:10 ` [PATCH 4/7] static_key: use static_key_slow_inc_deferred() Radim Krčmář
2013-10-17 10:39 ` Paolo Bonzini
2013-10-18 7:29 ` Radim Krčmář
2013-10-17 10:10 ` [PATCH 5/7] jump_label: relax branch hinting restrictions Radim Krčmář
2013-10-17 17:35 ` Steven Rostedt
2013-10-18 7:30 ` Radim Krčmář
2013-10-17 10:10 ` [PATCH 7/7] init: execute jump_label_init() earlier Radim Krčmář
2013-10-17 10:43 ` [PATCH 0/7] static_key: fix timer bugs & change api (a bit) Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1382004631-25895-1-git-send-email-rkrcmar@redhat.com \
--to=rkrcmar@redhat.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®