2013-10-23 10:39:41

by Chen Gang

[permalink] [raw]
Subject: [PATCH] kernel: context_tracking: use extern function instead of static inline function for user_enter/exit()

The related assemble code can not find the static inline function. The
related commit "ad65782 context_tracking: Optimize main APIs off case
with static key" causes this issue.

The related error (for arm, with allmodconfig):

LD init/built-in.o
arch/arm/kernel/built-in.o: In function `ret_fast_syscall':
arch/arm/kernel/entry-common.S:42: undefined reference to `user_enter'
arch/arm/kernel/built-in.o: In function `no_work_pending':
arch/arm/kernel/entry-common.S:77: undefined reference to `user_enter'
arch/arm/kernel/built-in.o: In function `vector_swi':
arch/arm/kernel/entry-common.S:376: undefined reference to `user_exit'
arch/arm/kernel/built-in.o: In function `__dabt_usr':
arch/arm/kernel/entry-armv.S:365: undefined reference to `user_exit'
arch/arm/kernel/built-in.o: In function `__irq_usr':
arch/arm/kernel/entry-armv.S:375: undefined reference to `user_exit'
arch/arm/kernel/built-in.o: In function `__und_usr':
arch/arm/kernel/entry-armv.S:388: undefined reference to `user_exit'
arch/arm/kernel/built-in.o: In function `__pabt_usr':
arch/arm/kernel/entry-armv.S:662: undefined reference to `user_exit'


Signed-off-by: Chen Gang <[email protected]>
---
include/linux/context_tracking.h | 14 ++------------
kernel/context_tracking.c | 12 ++++++++++++
2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index 1581587..bf220f7 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -10,23 +10,13 @@
#ifdef CONFIG_CONTEXT_TRACKING
extern void context_tracking_cpu_set(int cpu);

+extern void user_enter(void);
+extern void user_exit(void);
extern void context_tracking_user_enter(void);
extern void context_tracking_user_exit(void);
extern void __context_tracking_task_switch(struct task_struct *prev,
struct task_struct *next);

-static inline void user_enter(void)
-{
- if (static_key_false(&context_tracking_enabled))
- context_tracking_user_enter();
-
-}
-static inline void user_exit(void)
-{
- if (static_key_false(&context_tracking_enabled))
- context_tracking_user_exit();
-}
-
static inline enum ctx_state exception_enter(void)
{
enum ctx_state prev_ctx;
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 013161f..c03d0d6 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -29,6 +29,18 @@ EXPORT_SYMBOL_GPL(context_tracking_enabled);
DEFINE_PER_CPU(struct context_tracking, context_tracking);
EXPORT_SYMBOL_GPL(context_tracking);

+void user_enter(void)
+{
+ if (static_key_false(&context_tracking_enabled))
+ context_tracking_user_enter();
+}
+
+void user_exit(void)
+{
+ if (static_key_false(&context_tracking_enabled))
+ context_tracking_user_exit();
+}
+
void context_tracking_cpu_set(int cpu)
{
if (!per_cpu(context_tracking.active, cpu)) {
--
1.7.7.6


2013-10-23 10:47:34

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] kernel: context_tracking: use extern function instead of static inline function for user_enter/exit()

On Wed, 2013-10-23 at 18:38 +0800, Chen Gang wrote:

>
> Signed-off-by: Chen Gang <[email protected]>
> ---
> include/linux/context_tracking.h | 14 ++------------
> kernel/context_tracking.c | 12 ++++++++++++
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
> index 1581587..bf220f7 100644
> --- a/include/linux/context_tracking.h
> +++ b/include/linux/context_tracking.h
> @@ -10,23 +10,13 @@
> #ifdef CONFIG_CONTEXT_TRACKING
> extern void context_tracking_cpu_set(int cpu);
>
> +extern void user_enter(void);
> +extern void user_exit(void);
> extern void context_tracking_user_enter(void);
> extern void context_tracking_user_exit(void);
> extern void __context_tracking_task_switch(struct task_struct *prev,
> struct task_struct *next);
>
> -static inline void user_enter(void)
> -{
> - if (static_key_false(&context_tracking_enabled))
> - context_tracking_user_enter();
> -
> -}
> -static inline void user_exit(void)
> -{
> - if (static_key_false(&context_tracking_enabled))
> - context_tracking_user_exit();
> -}
> -
> static inline enum ctx_state exception_enter(void)
> {
> enum ctx_state prev_ctx;
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 013161f..c03d0d6 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -29,6 +29,18 @@ EXPORT_SYMBOL_GPL(context_tracking_enabled);
> DEFINE_PER_CPU(struct context_tracking, context_tracking);
> EXPORT_SYMBOL_GPL(context_tracking);
>
> +void user_enter(void)
> +{
> + if (static_key_false(&context_tracking_enabled))
> + context_tracking_user_enter();
> +}
> +
> +void user_exit(void)
> +{
> + if (static_key_false(&context_tracking_enabled))
> + context_tracking_user_exit();
> +}
> +
> void context_tracking_cpu_set(int cpu)
> {
> if (!per_cpu(context_tracking.active, cpu)) {

Ug, you just put an unneeded function call into a fast path because of a
failure with a particular arch.

If you need the assembly to call this code, then make yourself a wrapper
function to call that does the user_enter/exit() calls.

void arch_user_enter(void)
{
user_enter();
}

-- Steve

2013-10-23 11:02:24

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] kernel: context_tracking: use extern function instead of static inline function for user_enter/exit()

On 10/23/2013 06:47 PM, Steven Rostedt wrote:
> On Wed, 2013-10-23 at 18:38 +0800, Chen Gang wrote:
>
>>
>> Signed-off-by: Chen Gang <[email protected]>
>> ---
>> include/linux/context_tracking.h | 14 ++------------
>> kernel/context_tracking.c | 12 ++++++++++++
>> 2 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
>> index 1581587..bf220f7 100644
>> --- a/include/linux/context_tracking.h
>> +++ b/include/linux/context_tracking.h
>> @@ -10,23 +10,13 @@
>> #ifdef CONFIG_CONTEXT_TRACKING
>> extern void context_tracking_cpu_set(int cpu);
>>
>> +extern void user_enter(void);
>> +extern void user_exit(void);
>> extern void context_tracking_user_enter(void);
>> extern void context_tracking_user_exit(void);
>> extern void __context_tracking_task_switch(struct task_struct *prev,
>> struct task_struct *next);
>>
>> -static inline void user_enter(void)
>> -{
>> - if (static_key_false(&context_tracking_enabled))
>> - context_tracking_user_enter();
>> -
>> -}
>> -static inline void user_exit(void)
>> -{
>> - if (static_key_false(&context_tracking_enabled))
>> - context_tracking_user_exit();
>> -}
>> -
>> static inline enum ctx_state exception_enter(void)
>> {
>> enum ctx_state prev_ctx;
>> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
>> index 013161f..c03d0d6 100644
>> --- a/kernel/context_tracking.c
>> +++ b/kernel/context_tracking.c
>> @@ -29,6 +29,18 @@ EXPORT_SYMBOL_GPL(context_tracking_enabled);
>> DEFINE_PER_CPU(struct context_tracking, context_tracking);
>> EXPORT_SYMBOL_GPL(context_tracking);
>>
>> +void user_enter(void)
>> +{
>> + if (static_key_false(&context_tracking_enabled))
>> + context_tracking_user_enter();
>> +}
>> +
>> +void user_exit(void)
>> +{
>> + if (static_key_false(&context_tracking_enabled))
>> + context_tracking_user_exit();
>> +}
>> +
>> void context_tracking_cpu_set(int cpu)
>> {
>> if (!per_cpu(context_tracking.active, cpu)) {
>
> Ug, you just put an unneeded function call into a fast path because of a
> failure with a particular arch.
>
> If you need the assembly to call this code, then make yourself a wrapper
> function to call that does the user_enter/exit() calls.
>
> void arch_user_enter(void)
> {
> user_enter();
> }
>

OK, thanks. That sounds reasonable to me. I will send related patch to
arm guys. :-)


Thanks.
--
Chen Gang

2013-10-23 11:11:53

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH] kernel: context_tracking: use extern function instead of static inline function for user_enter/exit()

On Wed, Oct 23, 2013 at 06:38:35PM +0800, Chen Gang wrote:
> The related assemble code can not find the static inline function. The
> related commit "ad65782 context_tracking: Optimize main APIs off case
> with static key" causes this issue.
>
> The related error (for arm, with allmodconfig):
>
> LD init/built-in.o
> arch/arm/kernel/built-in.o: In function `ret_fast_syscall':
> arch/arm/kernel/entry-common.S:42: undefined reference to `user_enter'
> arch/arm/kernel/built-in.o: In function `no_work_pending':
> arch/arm/kernel/entry-common.S:77: undefined reference to `user_enter'
> arch/arm/kernel/built-in.o: In function `vector_swi':
> arch/arm/kernel/entry-common.S:376: undefined reference to `user_exit'
> arch/arm/kernel/built-in.o: In function `__dabt_usr':
> arch/arm/kernel/entry-armv.S:365: undefined reference to `user_exit'
> arch/arm/kernel/built-in.o: In function `__irq_usr':
> arch/arm/kernel/entry-armv.S:375: undefined reference to `user_exit'
> arch/arm/kernel/built-in.o: In function `__und_usr':
> arch/arm/kernel/entry-armv.S:388: undefined reference to `user_exit'
> arch/arm/kernel/built-in.o: In function `__pabt_usr':
> arch/arm/kernel/entry-armv.S:662: undefined reference to `user_exit'
>
>
> Signed-off-by: Chen Gang <[email protected]>

Doesn't 0c06a5d4b13cd66c833805a0d1db76b977944aac
("arm: Fix build error with context tracking calls") fix the issue for you?
Or may be it's another problem I missed?

Thanks.

2013-10-23 11:15:17

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] kernel: context_tracking: use extern function instead of static inline function for user_enter/exit()

On Wed, 2013-10-23 at 19:01 +0800, Chen Gang wrote:

> > void arch_user_enter(void)
> > {
> > user_enter();
> > }
> >
>
> OK, thanks. That sounds reasonable to me. I will send related patch to
> arm guys. :-)

If you do so, don't call it "arch_user_enter()", maybe call it
"asm_user_enter()", as "arch_*" is something used to be called by the
generic code. My mistake in using that as an example.

-- Steve

2013-10-23 11:20:49

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] kernel: context_tracking: use extern function instead of static inline function for user_enter/exit()

On 10/23/2013 07:11 PM, Frederic Weisbecker wrote:
> On Wed, Oct 23, 2013 at 06:38:35PM +0800, Chen Gang wrote:
>> The related assemble code can not find the static inline function. The
>> related commit "ad65782 context_tracking: Optimize main APIs off case
>> with static key" causes this issue.
>>
>> The related error (for arm, with allmodconfig):
>>
>> LD init/built-in.o
>> arch/arm/kernel/built-in.o: In function `ret_fast_syscall':
>> arch/arm/kernel/entry-common.S:42: undefined reference to `user_enter'
>> arch/arm/kernel/built-in.o: In function `no_work_pending':
>> arch/arm/kernel/entry-common.S:77: undefined reference to `user_enter'
>> arch/arm/kernel/built-in.o: In function `vector_swi':
>> arch/arm/kernel/entry-common.S:376: undefined reference to `user_exit'
>> arch/arm/kernel/built-in.o: In function `__dabt_usr':
>> arch/arm/kernel/entry-armv.S:365: undefined reference to `user_exit'
>> arch/arm/kernel/built-in.o: In function `__irq_usr':
>> arch/arm/kernel/entry-armv.S:375: undefined reference to `user_exit'
>> arch/arm/kernel/built-in.o: In function `__und_usr':
>> arch/arm/kernel/entry-armv.S:388: undefined reference to `user_exit'
>> arch/arm/kernel/built-in.o: In function `__pabt_usr':
>> arch/arm/kernel/entry-armv.S:662: undefined reference to `user_exit'
>>
>>
>> Signed-off-by: Chen Gang <[email protected]>
>
> Doesn't 0c06a5d4b13cd66c833805a0d1db76b977944aac
> ("arm: Fix build error with context tracking calls") fix the issue for you?
> Or may be it's another problem I missed?
>

Oh, yes it is, it is my fault, I still use original next-20130927 tree
(I really need reference another next tree or torvalds tree). :-)

Next, I will/should reference another next tree, at least.

Thanks again.

> Thanks.
>
>

--
Chen Gang

2013-10-23 11:32:11

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] kernel: context_tracking: use extern function instead of static inline function for user_enter/exit()

On 10/23/2013 07:15 PM, Steven Rostedt wrote:
> On Wed, 2013-10-23 at 19:01 +0800, Chen Gang wrote:
>
>>> void arch_user_enter(void)
>>> {
>>> user_enter();
>>> }
>>>
>>
>> OK, thanks. That sounds reasonable to me. I will send related patch to
>> arm guys. :-)
>
> If you do so, don't call it "arch_user_enter()", maybe call it
> "asm_user_enter()", as "arch_*" is something used to be called by the
> generic code. My mistake in using that as an example.
>

Oh, yes, what you said is an example, I need reference it, but should
not depend on it.

Hmm... for me "asm_user_enter" seems still a little 'generic' (e.g.
"asm-generic", "arch/*/include/asm/"), maybe just use common extern
functions' name is enough (e.g. "wrap_user_enter").

But all together, I feel your original fix patch is well enough to me,
it seems don't need additional trying. :-)

> -- Steve
>
>
>
>

Thanks.
--
Chen Gang