[PATCH 0/7] static_key: fix timer bugs & change api (a bit)

From: Radim KrÄmÃÅ
Date: Thu Oct 17 2013 - 06:17:32 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/