diff mbox series

[v3,4/4] kernel/trace: remove calling regs_* when compiling HEXAGON

Message ID 1670229017-4106-1-git-send-email-chensong_2000@189.cn (mailing list archive)
State Superseded
Headers show
Series reorganize trace_peobe_tmpl.h | expand

Commit Message

Song Chen Dec. 5, 2022, 8:30 a.m. UTC 
kernel test robot reports below errors:

In file included from kernel/trace/trace_events_synth.c:22:
>> kernel/trace/trace_probe_kernel.h:203:9: error: call to
undeclared function 'regs_get_register'; ISO C99 and later
do not support implicit function declarations
[-Wimplicit-function-declaration]
                   val = regs_get_register(regs, code->param);

HEXAGON doesn't define and implement those reg_* functions
underneath arch/hexagon as well as other archs. To remove
those errors, i have to include those function calls in
"CONFIG_HEXAGON"

It looks ugly, but i don't know any other way to fix it,
this patch can be reverted after reg_* have been in place
in arch/hexagon.

Signed-off-by: Song Chen <chensong_2000@189.cn>
Reported-by: kernel test robot <lkp@intel.com>
---
 kernel/trace/trace_probe_kernel.h | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Comments

Masami Hiramatsu (Google) Dec. 23, 2022, 2:15 p.m. UTC | #1 
On Mon,  5 Dec 2022 16:30:17 +0800
Song Chen <chensong_2000@189.cn> wrote:

> kernel test robot reports below errors:
> 
> In file included from kernel/trace/trace_events_synth.c:22:
> >> kernel/trace/trace_probe_kernel.h:203:9: error: call to
> undeclared function 'regs_get_register'; ISO C99 and later
> do not support implicit function declarations
> [-Wimplicit-function-declaration]
>                    val = regs_get_register(regs, code->param);
> 
> HEXAGON doesn't define and implement those reg_* functions
> underneath arch/hexagon as well as other archs. To remove
> those errors, i have to include those function calls in
> "CONFIG_HEXAGON"
> 
> It looks ugly, but i don't know any other way to fix it,
> this patch can be reverted after reg_* have been in place
> in arch/hexagon.
> 

Sorry, NACK. This is too add-hoc patch and this is introduced
by your patch. Do not introduce an issue and fix it later in
the same series.
Please fix it in your first patch. Maybe you should make another
header file for those APIs.

Thank you, 

> Signed-off-by: Song Chen <chensong_2000@189.cn>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>  kernel/trace/trace_probe_kernel.h | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_probe_kernel.h b/kernel/trace/trace_probe_kernel.h
> index 8c42abe0dacf..7e958b7f07e5 100644
> --- a/kernel/trace/trace_probe_kernel.h
> +++ b/kernel/trace/trace_probe_kernel.h
> @@ -130,8 +130,7 @@ probe_mem_read(void *dest, void *src, size_t size)
>  	return copy_from_kernel_nofault(dest, src, size);
>  }
>  
> -static nokprobe_inline unsigned long
> -get_event_field(struct fetch_insn *code, void *rec)
> +static unsigned long get_event_field(struct fetch_insn *code, void *rec)
>  {
>  	struct ftrace_event_field *field = code->data;
>  	unsigned long val;
> @@ -194,23 +193,41 @@ static int
>  process_fetch_insn(struct fetch_insn *code, void *rec, void *dest,
>  		   void *base)
>  {
> +#ifndef CONFIG_HEXAGON
>  	struct pt_regs *regs = rec;
> +#endif
>  	unsigned long val;
>  
>  retry:
>  	/* 1st stage: get value from context */
>  	switch (code->op) {
>  	case FETCH_OP_REG:
> +#ifdef CONFIG_HEXAGON
> +		val = 0;
> +#else
>  		val = regs_get_register(regs, code->param);
> +#endif
>  		break;
>  	case FETCH_OP_STACK:
> +#ifdef CONFIG_HEXAGON
> +		val = 0;
> +#else
>  		val = regs_get_kernel_stack_nth(regs, code->param);
> +#endif
>  		break;
>  	case FETCH_OP_STACKP:
> +#ifdef CONFIG_HEXAGON
> +		val = 0;
> +#else
>  		val = kernel_stack_pointer(regs);
> +#endif
>  		break;
>  	case FETCH_OP_RETVAL:
> +#ifdef CONFIG_HEXAGON
> +		val = 0;
> +#else
>  		val = regs_return_value(regs);
> +#endif
>  		break;
>  	case FETCH_OP_IMM:
>  		val = code->immediate;
> -- 
> 2.25.1
>
Song Chen Dec. 27, 2022, 9:49 a.m. UTC | #2 
在 2022/12/23 22:15, Masami Hiramatsu (Google) 写道:
> On Mon,  5 Dec 2022 16:30:17 +0800
> Song Chen <chensong_2000@189.cn> wrote:
> 
>> kernel test robot reports below errors:
>>
>> In file included from kernel/trace/trace_events_synth.c:22:
>>>> kernel/trace/trace_probe_kernel.h:203:9: error: call to
>> undeclared function 'regs_get_register'; ISO C99 and later
>> do not support implicit function declarations
>> [-Wimplicit-function-declaration]
>>                     val = regs_get_register(regs, code->param);
>>
>> HEXAGON doesn't define and implement those reg_* functions
>> underneath arch/hexagon as well as other archs. To remove
>> those errors, i have to include those function calls in
>> "CONFIG_HEXAGON"
>>
>> It looks ugly, but i don't know any other way to fix it,
>> this patch can be reverted after reg_* have been in place
>> in arch/hexagon.
>>
> 
> Sorry, NACK. This is too add-hoc patch and this is introduced
> by your patch. Do not introduce an issue and fix it later in
> the same series.
> Please fix it in your first patch. Maybe you should make another
> header file for those APIs.
> 
> Thank you,
I tried not no add a new header file, but looks like i have to, to avoid 
triggering warnings and errors of kernel robot.

Thanks

Song

> 
>> Signed-off-by: Song Chen <chensong_2000@189.cn>
>> Reported-by: kernel test robot <lkp@intel.com>
>> ---
>>   kernel/trace/trace_probe_kernel.h | 21 +++++++++++++++++++--
>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/trace_probe_kernel.h b/kernel/trace/trace_probe_kernel.h
>> index 8c42abe0dacf..7e958b7f07e5 100644
>> --- a/kernel/trace/trace_probe_kernel.h
>> +++ b/kernel/trace/trace_probe_kernel.h
>> @@ -130,8 +130,7 @@ probe_mem_read(void *dest, void *src, size_t size)
>>   	return copy_from_kernel_nofault(dest, src, size);
>>   }
>>   
>> -static nokprobe_inline unsigned long
>> -get_event_field(struct fetch_insn *code, void *rec)
>> +static unsigned long get_event_field(struct fetch_insn *code, void *rec)
>>   {
>>   	struct ftrace_event_field *field = code->data;
>>   	unsigned long val;
>> @@ -194,23 +193,41 @@ static int
>>   process_fetch_insn(struct fetch_insn *code, void *rec, void *dest,
>>   		   void *base)
>>   {
>> +#ifndef CONFIG_HEXAGON
>>   	struct pt_regs *regs = rec;
>> +#endif
>>   	unsigned long val;
>>   
>>   retry:
>>   	/* 1st stage: get value from context */
>>   	switch (code->op) {
>>   	case FETCH_OP_REG:
>> +#ifdef CONFIG_HEXAGON
>> +		val = 0;
>> +#else
>>   		val = regs_get_register(regs, code->param);
>> +#endif
>>   		break;
>>   	case FETCH_OP_STACK:
>> +#ifdef CONFIG_HEXAGON
>> +		val = 0;
>> +#else
>>   		val = regs_get_kernel_stack_nth(regs, code->param);
>> +#endif
>>   		break;
>>   	case FETCH_OP_STACKP:
>> +#ifdef CONFIG_HEXAGON
>> +		val = 0;
>> +#else
>>   		val = kernel_stack_pointer(regs);
>> +#endif
>>   		break;
>>   	case FETCH_OP_RETVAL:
>> +#ifdef CONFIG_HEXAGON
>> +		val = 0;
>> +#else
>>   		val = regs_return_value(regs);
>> +#endif
>>   		break;
>>   	case FETCH_OP_IMM:
>>   		val = code->immediate;
>> -- 
>> 2.25.1
>>
> 
>
diff mbox series

Patch

diff --git a/kernel/trace/trace_probe_kernel.h b/kernel/trace/trace_probe_kernel.h
index 8c42abe0dacf..7e958b7f07e5 100644
--- a/kernel/trace/trace_probe_kernel.h
+++ b/kernel/trace/trace_probe_kernel.h
@@ -130,8 +130,7 @@  probe_mem_read(void *dest, void *src, size_t size)
 	return copy_from_kernel_nofault(dest, src, size);
 }
 
-static nokprobe_inline unsigned long
-get_event_field(struct fetch_insn *code, void *rec)
+static unsigned long get_event_field(struct fetch_insn *code, void *rec)
 {
 	struct ftrace_event_field *field = code->data;
 	unsigned long val;
@@ -194,23 +193,41 @@  static int
 process_fetch_insn(struct fetch_insn *code, void *rec, void *dest,
 		   void *base)
 {
+#ifndef CONFIG_HEXAGON
 	struct pt_regs *regs = rec;
+#endif
 	unsigned long val;
 
 retry:
 	/* 1st stage: get value from context */
 	switch (code->op) {
 	case FETCH_OP_REG:
+#ifdef CONFIG_HEXAGON
+		val = 0;
+#else
 		val = regs_get_register(regs, code->param);
+#endif
 		break;
 	case FETCH_OP_STACK:
+#ifdef CONFIG_HEXAGON
+		val = 0;
+#else
 		val = regs_get_kernel_stack_nth(regs, code->param);
+#endif
 		break;
 	case FETCH_OP_STACKP:
+#ifdef CONFIG_HEXAGON
+		val = 0;
+#else
 		val = kernel_stack_pointer(regs);
+#endif
 		break;
 	case FETCH_OP_RETVAL:
+#ifdef CONFIG_HEXAGON
+		val = 0;
+#else
 		val = regs_return_value(regs);
+#endif
 		break;
 	case FETCH_OP_IMM:
 		val = code->immediate;