| 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: <20111202091805.7c74d0a4.kamezawa.hiroyu@jp.fujitsu.com>
Date: Fri, 2 Dec 2011 09:18:05 +0900
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Jason Baron <jbaron@...hat.com>
Subject: Re: [PATCH] jump_label: jump_label for boot options.
On Thu, 01 Dec 2011 09:48:47 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:
> On Thu, 2011-12-01 at 11:53 +0900, KAMEZAWA Hiroyuki wrote:
> > I tried to use jump_label for handling memcg's boot options which sets
> > global variable true/false and never changes after boot. And found jump_table
> > is larger than expected. This patch is a trial to allow to place jump_table
> > in .init section. How do you think ?
> >
> > This patch is based on linux-next.
> >
> > ==
> > >From ed8996892c21d4fec642e4fc80bd3ebdd8a48836 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
> > Date: Thu, 1 Dec 2011 11:08:23 +0900
> > Subject: [PATCH] jump_label for boot options.
> >
> > Some boot options exists for enable/disable a functionality which cannot be modified
> > after boot. Using jump_label for such behavior seems atractive but if caller
> > of statch_branch() is too much, jump_table can be unexpectedly big.
>
> s/statch/static/
>
will fix.
> >
> > This patch adds static_branch_init_once(), which places its jump_table
> > in init section and never be updated after boot.
>
> I don't like the name static_branch_init_once(). Although I suck at
> picking names myself :-p Maybe just remove the 'init'.
> static_branch_once(), or remove the 'once'. static_branch_init()
>
ok.
> Your name may be the best, but I'm hoping another name will come up.
> static_branch_init_once() just doesn't seem right.
>
I don't think my sense of naming is good.
>
> > For MODULES, using usual static_branch().
>
> Why not for modules? It can be forced at module load time, and then
> removed.
>
It can be. I just don't investigate how module's init section is handled.
<snip>
> > +/*
> > + * Use this when you call static_branch() in __init function or
> > + * jump_label is only modified by initcalls. jump_label information
> > + * will be discarded after boot. But please be careful. jump_label_key
> > + * definition itself should *not* be in __init section because a MODULE
> > + * may call static_branch_init_once().
> > + *
> > + * Useful for changing behavior by boot option.
> > + */
> > +#ifndef MODULE
> > +static __always_inline bool static_branch_init_once(struct jump_label_key *key)
> > +{
> > + return arch_static_branch_init_once(key);
> > +}
> > +#else
> > +static __always_inline bool static_branch_init_once(struct jump_label_key *key)
> > +{
> > + return static_branch(key);
> > +}
> > +#endif
>
> I still think modules should be able to do this, and then just remove it
> later.
>
Hm, ok, I'll look into and prepare an another patch.
> > +
> > extern struct jump_entry __start___jump_table[];
> > extern struct jump_entry __stop___jump_table[];
> > +extern struct jump_entry __start___jump_table_at_init[];
> > +extern struct jump_entry __stop___jump_table_at_init[];
> >
> > extern void jump_label_init(void);
> > extern void jump_label_lock(void);
> > @@ -54,6 +77,12 @@ extern void jump_label_dec(struct jump_label_key *key);
> > extern bool jump_label_enabled(struct jump_label_key *key);
> > extern void jump_label_apply_nops(struct module *mod);
> >
> > +/*
> > + * For jump_label in init section.
> > + * This will call jump_label_inc() for usual section, too.
> > + */
> > +extern void jump_label_inc_once(struct jump_label_key *key);
> > +
> > #else /* !HAVE_JUMP_LABEL */
> >
> > #include <linux/atomic.h>
> > @@ -75,6 +104,11 @@ static __always_inline bool static_branch(struct jump_label_key *key)
> > return false;
> > }
> >
> > +static __always_inline bool static_branch_init_once(struct jump_label_key *key)
> > +{
> > + return static_branch(key);
> > +}
> > +
> > static inline void jump_label_inc(struct jump_label_key *key)
> > {
> > atomic_inc(&key->enabled);
> > @@ -102,6 +136,11 @@ static inline int jump_label_apply_nops(struct module *mod)
> > {
> > return 0;
> > }
> > +
> > +static inline void jump_label_inc_once(struct jump_label_key *key)
> > +{
> > + jump_label_inc(key);
> > +}
> > #endif /* HAVE_JUMP_LABEL */
> >
> > #endif /* _LINUX_JUMP_LABEL_H */
> > diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> > index 66ff710..f0e8231 100644
> > --- a/kernel/jump_label.c
> > +++ b/kernel/jump_label.c
> > @@ -134,15 +134,11 @@ static void __jump_label_update(struct jump_label_key *key,
> > }
> > }
> >
> > -void __init jump_label_init(void)
> > +void __init jump_label_transform_all(struct jump_entry *iter_start,
> > + struct jump_entry *iter_stop)
> > {
> > - struct jump_entry *iter_start = __start___jump_table;
> > - struct jump_entry *iter_stop = __stop___jump_table;
> > - struct jump_label_key *key = NULL;
> > struct jump_entry *iter;
> > -
> > - jump_label_lock();
> > - jump_label_sort_entries(iter_start, iter_stop);
> > + struct jump_label_key *key = NULL;
> >
> > for (iter = iter_start; iter < iter_stop; iter++) {
> > struct jump_label_key *iterk;
> > @@ -159,6 +155,24 @@ void __init jump_label_init(void)
> > key->next = NULL;
> > #endif
> > }
> > +
> > +}
> > +
> > +
> > +void __init jump_label_init(void)
> > +{
> > + struct jump_entry *iter_start = __start___jump_table;
> > + struct jump_entry *iter_stop = __stop___jump_table;
> > +
> > +
> > + jump_label_lock();
> > + jump_label_sort_entries(iter_start, iter_stop);
> > + jump_label_transform_all(iter_start, iter_stop);
>
> Nit, I'd add a space here.
>
ok. I'll fix.
Thanks,
-Kame
--
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