| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20110523164541.GE2052@jolsa.brq.redhat.com>
Date: Mon, 23 May 2011 18:45:41 +0200
From: Jiri Olsa <jolsa@...hat.com>
To: rostedt@...dmis.org
Cc: Jason Baron <jbaron@...hat.com>, mingo@...e.hu,
linux-kernel@...r.kernel.org
Subject: Re: [PATCHv3] jump_label,x86: make batch update of jump_label entries
hi,
any feedback?
thanks,
jirka
On Mon, May 09, 2011 at 03:27:26PM -0400, Jason Baron wrote:
> On Mon, May 09, 2011 at 08:38:16PM +0200, Jiri Olsa wrote:
> > On Wed, May 04, 2011 at 11:41:41AM +0200, Jiri Olsa wrote:
> > > hi,
> > >
> > > I'm changing the jump label update code to use batch processing
> > > for x86 architectures.
> > >
> > > Currently each jump label update calls text_poke_smp for each
> > > jump label key entry. Thus one key update ends up calling stop
> > > machine multiple times.
> > >
> > > This patch is using text_poke_smp_batch, which is called for
> > > all the key's entries. Thus ensuring the stop machine is called
> > > only once per jump_label key.
> > >
> > > attached patches:
> > > 1/2 - jump_label,x86: use text_poke_smp_batch for entries update
> > > - added jump_label_update_end function which is paired with
> > > the key's entries update
> > > - jump_label_update_end calls arch_jump_label_update_end which
> > > is overloaded by x86 arch and makes the batch update of all the
> > > entries queued by arch_jump_label_transform function.
> > >
> > > 2/2 - jump_label,x86: using static arrays before dynamic allocation is needed
> > > - in the first patch, the queue array, which stores jump_label
> > > entries is allocated/resized dynamically.
> > > - due to the fact that many jump_label entries have low number
> > > of callers, it seems appropriate to use static sized array
> > > when the update starts and if needed (in case of high number
> > > of jump_label entries) allocate/use the dynamic array
> > >
> > >
> > > Patch 2/2 and could be ommited if the benefit/complexity ratio
> > > would seem too low.. ;)
> > >
> > > I tested this on x86 and s390 archs.
> > >
> > > v2 changes:
> > > - queueing all entries for single key and process them
> > > all at one time
> > >
> > > wrb,
> > > jirka
> > > ---
> > > arch/x86/kernel/jump_label.c | 177 +++++++++++++++++++++++++++++++++++++++--
> > > include/linux/jump_label.h | 1 +
> > > kernel/jump_label.c | 16 ++++-
> > > 3 files changed, 183 insertions(+), 11 deletions(-)
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to majordomo@...r.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at http://www.tux.org/lkml/
> >
> > hi,
> >
> > I did jump_label entries statistics on allyesconfig kernel
> > and got following numbers:
> >
> > callers - keys
> > 1 - 964
> > 2 - 28
> > 3 - 5
> > 4 - 1
> > 6 - 1
> > 11 - 2
> > 12 - 1
> > 14 - 2
> > 17 - 2
> > 21 - 1
> > 170 - 1
> >
> >
> > So the maximum is 170 callers and just for one key,
> > which is the key used in trace_module_get function.
> >
> > Jason suggested we might stay with the static way because for most
> > of the entries the maximum number of callers is up to 21.
> >
> > I'm attaching the static version for consideration.
> >
> > wbr,
> > jirka
> >
> > ---
> > Changing the jump label update code to use batch processing
> > for x86 architectures.
> >
> > Currently each jump label update calls text_poke_smp for each
> > jump label key entry. Thus one key update ends up calling stop
> > machine multiple times.
> >
> > This patch is using text_poke_smp_batch, which is called for
> > mmultiple entries at a time.
> >
> > Added jump_label_update_end function which is paired with
> > the key's entries update.
> >
> > The jump_label_update_end calls arch_jump_label_update_end
> > (with generic weak definition) which is overloaded by x86
> > arch and makes the batch update of all the entries queued
> > by arch_jump_label_transform function.
> >
> > The number of entries that can be updated at a single time
> > is set to 30. This number is based on statistics from allyesconfig
> > kernel showing most of the keys having upto 30 callers.
> >
> > callers - keys
> > 1 - 964
> > 2 - 28
> > 3 - 5
> > 4 - 1
> > 6 - 1
> > 11 - 2
> > 12 - 1
> > 14 - 2
> > 17 - 2
> > 21 - 1
> > 170 - 1
> >
> >
> > Signed-off-by: Jiri Olsa <jolsa@...hat.com>
> > ---
> > arch/x86/kernel/jump_label.c | 68 ++++++++++++++++++++++++++++++++++++-----
> > include/linux/jump_label.h | 1 +
> > kernel/jump_label.c | 16 ++++++++-
> > 3 files changed, 74 insertions(+), 11 deletions(-)
> >
>
> For me, this version is much simpler (less than half the code size of
> the original patch), while being optimal for 1007/1008 of the keys.
>
> Acked-by: Jason Baron <jbaron@...hat.com>
>
>
> Thanks.
>
> > diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
> > index 3fee346..bbde5db 100644
> > --- a/arch/x86/kernel/jump_label.c
> > +++ b/arch/x86/kernel/jump_label.c
> > @@ -24,24 +24,74 @@ union jump_code_union {
> > } __attribute__((packed));
> > };
> >
> > -void arch_jump_label_transform(struct jump_entry *entry,
> > - enum jump_label_type type)
> > +struct text_poke_buffer {
> > + u8 code[JUMP_LABEL_NOP_SIZE];
> > +};
> > +
> > +#define POKE_CNT_MAX 30
> > +
> > +static struct text_poke_param poke_pars[POKE_CNT_MAX];
> > +static struct text_poke_buffer poke_bufs[POKE_CNT_MAX];
> > +static int poke_cnt;
> > +
> > +static void poke_setup(struct text_poke_param *param, u8 *buf,
> > + int enable,
> > + struct jump_entry *entry)
> > {
> > - union jump_code_union code;
> > + union jump_code_union *code = (union jump_code_union *) buf;
> >
> > - if (type == JUMP_LABEL_ENABLE) {
> > - code.jump = 0xe9;
> > - code.offset = entry->target -
> > - (entry->code + JUMP_LABEL_NOP_SIZE);
> > + if (enable == JUMP_LABEL_ENABLE) {
> > + code->jump = 0xe9;
> > + code->offset = entry->target -
> > + (entry->code + JUMP_LABEL_NOP_SIZE);
> > } else
> > - memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
> > + memcpy(code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
> > +
> > + param->addr = (void *) entry->code;
> > + param->opcode = code;
> > + param->len = JUMP_LABEL_NOP_SIZE;
> > +}
> > +
> > +static void poke_process(void)
> > +{
> > get_online_cpus();
> > mutex_lock(&text_mutex);
> > - text_poke_smp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
> > +
> > + text_poke_smp_batch(poke_pars, poke_cnt);
> > + poke_cnt = 0;
> > +
> > mutex_unlock(&text_mutex);
> > put_online_cpus();
> > }
> >
> > +static void poke_end(void)
> > +{
> > + if (!poke_cnt)
> > + return;
> > +
> > + poke_process();
> > +}
> > +
> > +void arch_jump_label_transform(struct jump_entry *entry,
> > + enum jump_label_type enable)
> > +{
> > + if (poke_cnt == POKE_CNT_MAX)
> > + poke_process();
> > +
> > + poke_setup(&poke_pars[poke_cnt], poke_bufs[poke_cnt].code,
> > + enable, entry);
> > + poke_cnt++;
> > +}
> > +
> > +/*
> > + * Called after arch_jump_label_transform is called for
> > + * all entries of a single key.
> > + */
> > +void arch_jump_label_update_end(void)
> > +{
> > + poke_end();
> > +}
> > +
> > void arch_jump_label_text_poke_early(jump_label_t addr)
> > {
> > text_poke_early((void *)addr, ideal_nops[NOP_ATOMIC5],
> > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> > index 83e745f..e7a8fa3 100644
> > --- a/include/linux/jump_label.h
> > +++ b/include/linux/jump_label.h
> > @@ -46,6 +46,7 @@ extern void jump_label_unlock(void);
> > extern void arch_jump_label_transform(struct jump_entry *entry,
> > enum jump_label_type type);
> > extern void arch_jump_label_text_poke_early(jump_label_t addr);
> > +extern void arch_jump_label_update_end(void);
> > extern int jump_label_text_reserved(void *start, void *end);
> > extern void jump_label_inc(struct jump_label_key *key);
> > extern void jump_label_dec(struct jump_label_key *key);
> > diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> > index 74d1c09..6657a37 100644
> > --- a/kernel/jump_label.c
> > +++ b/kernel/jump_label.c
> > @@ -125,6 +125,15 @@ void __weak arch_jump_label_text_poke_early(jump_label_t addr)
> > {
> > }
> >
> > +void __weak arch_jump_label_update_end(void)
> > +{
> > +}
> > +
> > +void jump_label_update_end(void)
> > +{
> > + arch_jump_label_update_end();
> > +}
> > +
> > static __init int jump_label_init(void)
> > {
> > struct jump_entry *iter_start = __start___jump_table;
> > @@ -244,10 +253,11 @@ static int jump_label_add_module(struct module *mod)
> > jlm->next = key->next;
> > key->next = jlm;
> >
> > - if (jump_label_enabled(key))
> > + if (jump_label_enabled(key)) {
> > __jump_label_update(key, iter, JUMP_LABEL_ENABLE);
> > + jump_label_update_end();
> > + }
> > }
> > -
> > return 0;
> > }
> >
> > @@ -376,6 +386,8 @@ static void jump_label_update(struct jump_label_key *key, int enable)
> > #ifdef CONFIG_MODULES
> > __jump_label_mod_update(key, enable);
> > #endif
> > +
> > + jump_label_update_end();
> > }
> >
> > #endif
> > --
> > 1.7.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists