Messages in this thread | | | Date | Thu, 1 Oct 2009 08:58:21 -0400 | From | Mathieu Desnoyers <> | Subject | Re: [PATCH 1/4] jump label - make init_kernel_text() global |
| |
* Ingo Molnar (mingo@elte.hu) wrote: > > * Jason Baron <jbaron@redhat.com> wrote: > > > allow usage of init_kernel_text - we need this in jump labeling to > > avoid attemtpting to patch code that has been freed as in the __init > > sections > > s/attemtpting/attempting > > > Signed-off-by: Jason Baron <jbaron@redhat.com> > > --- > > include/linux/kernel.h | 1 + > > kernel/extable.c | 2 +- > > 2 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > > index f61039e..9d3419f 100644 > > --- a/include/linux/kernel.h > > +++ b/include/linux/kernel.h > > @@ -295,6 +295,7 @@ extern int get_option(char **str, int *pint); > > extern char *get_options(const char *str, int nints, int *ints); > > extern unsigned long long memparse(const char *ptr, char **retptr); > > > > +extern int init_kernel_text(unsigned long addr); > > extern int core_kernel_text(unsigned long addr); > > extern int __kernel_text_address(unsigned long addr); > > extern int kernel_text_address(unsigned long addr); > > diff --git a/kernel/extable.c b/kernel/extable.c > > index 7f8f263..f6893ad 100644 > > --- a/kernel/extable.c > > +++ b/kernel/extable.c > > @@ -52,7 +52,7 @@ const struct exception_table_entry *search_exception_tables(unsigned long addr) > > return e; > > } > > > > -static inline int init_kernel_text(unsigned long addr) > > +int init_kernel_text(unsigned long addr) > > { > > if (addr >= (unsigned long)_sinittext && > > addr <= (unsigned long)_einittext) > > i'm confused. Later on jump_label_update() does: > > + if (!(system_state == SYSTEM_RUNNING && > + (init_kernel_text(iter->code)))) > + jump_label_transform(iter, type); > > which is: > > + if (system_state != SYSTEM_RUNNING || > + !init_kernel_text(iter->code))) > + jump_label_transform(iter, type); > > What is the logic behind that? System going into SYSTEM_RUNNING does not > coincide with free_initmem() precisely. > > Also, do we ever want to patch init-text tracepoints? I think we want to > stay away from them as much as possible.
My experience with tracepoint instruentation is that as soon as we start using them in inline functions (e.g. memory allocation, interrupt entry/exit), we start making these functions unusable in init code. That a very unwelcome side-effect that I'd like to prevent by making sure tracepoints and the patching mechanism underneath supports init code patching. Node that it does not have to be complicated.
But I think you are right that the logic seems fuzzy in this specific spot of the patch.
Thanks,
Mathieu
> > It appears to me that what we want here is a straight: > > if (kernel_text(iter->code)) > jump_label_transform(iter, type); > > Also, maybe a WARN_ONCE(!kernel_text()) - we should never even attempt > to transform non-patchable code. If yes then we want to know about that > in a noisy way and not skip it silently. > > Ingo
-- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |