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

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

[Chen Gang]

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 <gang.chen@asianux.com>
---
 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

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

[Steven Rostedt]

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

> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  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

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

[Chen Gang]

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 <gang.chen@asianux.com>
>> ---
>>  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

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

[Frederic Weisbecker]

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 <gang.chen@asianux.com>

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.

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

[Steven Rostedt]

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

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

[Chen Gang]

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 <gang.chen@asianux.com>
> 
> 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

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

[Chen Gang]

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