Mailing List Archive: [PATCH RFC 0/8] jump-label: allow early jump_label

[PATCH RFC 0/8] jump-label: allow early jump_label_enable()

Sep 29, 2011, 4:26 PM

Post #1 of 5 (281 views)

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Hi all,

While trying to use the jump-label stuff for my PV ticketlock changes,
I had some problems using jump labels early in the kernel's lifetime
(pre-SMP).

The basic problem is that even if I enable a jump_label_key, the
jump_label_init() initializer ends up nopping out all the code sites.

This series enables early use of jump labels by making
jump_label_init() respect already-enabled keys.

To do this, I've dropped arch_jump_label_poke_text_early() and
replaced it with arch_jump_label_transform_early(), which is the same
as the non-_early variant except that it expects to be operating in a
pre-SMP environment.

I've tested this on x86, but all the other architecture changes are
completely untested (not even breathed on by a compiler).

One big question which arises is whether the _early() function is
necessary at all. All the stop_machine/mutex/etc stuff that
arch_jump_label_transform() ends up doing is redundant pre-SMP, but it
shouldn't hurt. Maybe we can just drop the _early function? It works
on x86, at least, because jump_label_enable() works, which uses the full
form. And dropping it would reduce this to a very much smaller series.

Thanks,
J

Jeremy Fitzhardinge (8):
jump_label: use proper atomic_t initializer
jump_label: if a key has already been initialized, don't nop it out
x86/jump_label: add arch_jump_label_transform_early()
sparc/jump_label: add arch_jump_label_transform_early()
mips/jump_label: add arch_jump_label_transform_early()
powerpc/jump_label: add arch_jump_label_transform_early()
s390/jump-label: add arch_jump_label_transform_early()
jump_label: drop default arch_jump_label_transform_early

arch/mips/kernel/jump_label.c | 21 +++++++++++++---
arch/powerpc/kernel/jump_label.c | 6 ++++
arch/s390/kernel/jump_label.c | 49 +++++++++++++++++++++++--------------
arch/sparc/kernel/jump_label.c | 24 +++++++++++-------
arch/x86/kernel/jump_label.c | 20 +++++++++++----
include/linux/jump_label.h | 7 +++--
kernel/jump_label.c | 20 ++++++---------
7 files changed, 94 insertions(+), 53 deletions(-)

--
1.7.6.2

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH RFC 0/8] jump-label: allow early jump_label_enable() [ In reply to ]

rostedt at goodmis

Sep 29, 2011, 5:52 PM

Post #2 of 5 (281 views)

Permalink

On Thu, 2011-09-29 at 16:26 -0700, Jeremy Fitzhardinge wrote:
> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>

> One big question which arises is whether the _early() function is
> necessary at all. All the stop_machine/mutex/etc stuff that
> arch_jump_label_transform() ends up doing is redundant pre-SMP, but it
> shouldn't hurt. Maybe we can just drop the _early function? It works
> on x86, at least, because jump_label_enable() works, which uses the full
> form. And dropping it would reduce this to a very much smaller series.

It does slow down the boot process, which is not a good thing when
everyone is pushing for the fastest restarts.

What we should probably do is have a global read_mostly variable called,
smp_activated or something, then things that can be called before and
after can read this variable to determine if it can skip certain
protections.

While we're at it, perhaps we could add a memory_initialized for things
like tracers that want to trace early but need to wait till it can
allocate buffers. If we had this flag, it could instead do an early
memory init to create the buffers.

-- Steve

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH RFC 0/8] jump-label: allow early jump_label_enable() [ In reply to ]

jeremy at goop

Sep 29, 2011, 9:40 PM

Post #3 of 5 (280 views)

Permalink

On 09/29/2011 05:52 PM, Steven Rostedt wrote:
> On Thu, 2011-09-29 at 16:26 -0700, Jeremy Fitzhardinge wrote:
>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>
>> One big question which arises is whether the _early() function is
>> necessary at all. All the stop_machine/mutex/etc stuff that
>> arch_jump_label_transform() ends up doing is redundant pre-SMP, but it
>> shouldn't hurt. Maybe we can just drop the _early function? It works
>> on x86, at least, because jump_label_enable() works, which uses the full
>> form. And dropping it would reduce this to a very much smaller series.
> It does slow down the boot process, which is not a good thing when
> everyone is pushing for the fastest restarts.

Would it really though? stop_machine() doesn't do very much when there
are no other cpus.

Not that I measured or anything, but there was no obvious big lag at boot.

> What we should probably do is have a global read_mostly variable called,
> smp_activated or something, then things that can be called before and
> after can read this variable to determine if it can skip certain
> protections.

Could do that if it turns out to be a problem.

> While we're at it, perhaps we could add a memory_initialized for things
> like tracers that want to trace early but need to wait till it can
> allocate buffers. If we had this flag, it could instead do an early
> memory init to create the buffers.

That seems orthogonal to the jump_label changes.

J

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH RFC 0/8] jump-label: allow early jump_label_enable() [ In reply to ]

rostedt at goodmis

Sep 30, 2011, 8:28 AM

Post #4 of 5 (282 views)

Permalink

On Thu, 2011-09-29 at 21:40 -0700, Jeremy Fitzhardinge wrote:
> On 09/29/2011 05:52 PM, Steven Rostedt wrote:
> > On Thu, 2011-09-29 at 16:26 -0700, Jeremy Fitzhardinge wrote:
> >> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> >>
> >> One big question which arises is whether the _early() function is
> >> necessary at all. All the stop_machine/mutex/etc stuff that
> >> arch_jump_label_transform() ends up doing is redundant pre-SMP, but it
> >> shouldn't hurt. Maybe we can just drop the _early function? It works
> >> on x86, at least, because jump_label_enable() works, which uses the full
> >> form. And dropping it would reduce this to a very much smaller series.
> > It does slow down the boot process, which is not a good thing when
> > everyone is pushing for the fastest restarts.
>
> Would it really though? stop_machine() doesn't do very much when there
> are no other cpus.
>
> Not that I measured or anything, but there was no obvious big lag at boot.

Just bringing up the point, but without measurements, its all hand
waving. It may not be a big deal, and simpler code is always better if
it doesn't harm anything else.

>
> > What we should probably do is have a global read_mostly variable called,
> > smp_activated or something, then things that can be called before and
> > after can read this variable to determine if it can skip certain
> > protections.
>
> Could do that if it turns out to be a problem.
>
> > While we're at it, perhaps we could add a memory_initialized for things
> > like tracers that want to trace early but need to wait till it can
> > allocate buffers. If we had this flag, it could instead do an early
> > memory init to create the buffers.
>
> That seems orthogonal to the jump_label changes.

It is. But it's something that bugs me and this just reminded me of
it ;)

-- Steve

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH RFC 0/8] jump-label: allow early jump_label_enable() [ In reply to ]

jeremy at goop

Sep 30, 2011, 9:09 AM

Post #5 of 5 (280 views)

Permalink

On 09/30/2011 08:28 AM, Steven Rostedt wrote:
> On Thu, 2011-09-29 at 21:40 -0700, Jeremy Fitzhardinge wrote:
>> On 09/29/2011 05:52 PM, Steven Rostedt wrote:
>>> On Thu, 2011-09-29 at 16:26 -0700, Jeremy Fitzhardinge wrote:
>>>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>>>
>>>> One big question which arises is whether the _early() function is
>>>> necessary at all. All the stop_machine/mutex/etc stuff that
>>>> arch_jump_label_transform() ends up doing is redundant pre-SMP, but it
>>>> shouldn't hurt. Maybe we can just drop the _early function? It works
>>>> on x86, at least, because jump_label_enable() works, which uses the full
>>>> form. And dropping it would reduce this to a very much smaller series.
>>> It does slow down the boot process, which is not a good thing when
>>> everyone is pushing for the fastest restarts.
>> Would it really though? stop_machine() doesn't do very much when there
>> are no other cpus.
>>
>> Not that I measured or anything, but there was no obvious big lag at boot.
> Just bringing up the point, but without measurements, its all hand
> waving. It may not be a big deal, and simpler code is always better if
> it doesn't harm anything else.

I think the simplest thing is to make stop_machine() well-defined in a
pre-smp environment, where it just directly calls the callback:

diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index ba5070c..b6ad9b3 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -485,6 +485,11 @@ int __stop_machine(int (*fn)(void *), void *data, const struct cpumask *cpus)
.num_threads = num_online_cpus(),
.active_cpus = cpus };

+ if (smdata.num_threads == 1) {
+ (*fn)(data);
+ return 0;
+ }
+
/* Set the initial state and stop all online cpus. */
set_state(&smdata, STOPMACHINE_PREPARE);
return stop_cpus(cpu_online_mask, stop_machine_cpu_stop, &smdata);

so that its guaranteed safe to use at any point.

J

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel