[PATCH v2 0/1] Tracing: Compile error with qla2xxx

[PATCH v2 0/1] Tracing: Compile error with qla2xxx

[Arun Easi]

Hi Steve, et.al,

Please find a patch to fix compile error coming from qla2xxx driver
when CONFIG_TRACING is disabled.

Hi Martin,

Please apply this patch to the SCSI tree once Steve approves it. Here
is a link to the discussion:
    https://lore.kernel.org/linux-scsi/YxdZ%2F9XOsWilvVSd@debian/T/#m6efb601ed65c907124a03cfd5f3f38f1eb8c5925

Changes from V1:
    * Incorporated Steve's review comments (change to inline etc.)

Regards,
-Arun

Arun Easi (1):
  tracing: Fix compile error in trace_array calls when TRACING is
    disabled

 include/linux/trace.h | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

-- 
2.9.5

base-commit: efca52749564601de2045eb71dbe756b7bade4e8

[PATCH v2 1/1] tracing: Fix compile error in trace_array calls when TRACING is disabled

[Arun Easi]

Fix this compilation error seen when CONFIG_TRACING is not enabled:

drivers/scsi/qla2xxx/qla_os.c: In function 'qla_trace_init':
drivers/scsi/qla2xxx/qla_os.c:2854:25: error: implicit declaration of function
'trace_array_get_by_name'; did you mean 'trace_array_set_clr_event'?
[-Werror=implicit-function-declaration]
 2854 |         qla_trc_array = trace_array_get_by_name("qla2xxx");
      |                         ^~~~~~~~~~~~~~~~~~~~~~~
      |                         trace_array_set_clr_event

drivers/scsi/qla2xxx/qla_os.c: In function 'qla_trace_uninit':
drivers/scsi/qla2xxx/qla_os.c:2869:9: error: implicit declaration of function
'trace_array_put' [-Werror=implicit-function-declaration]
 2869 |         trace_array_put(qla_trc_array);
      |         ^~~~~~~~~~~~~~~

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Arun Easi <aeasi@marvell.com>
---
 include/linux/trace.h | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/include/linux/trace.h b/include/linux/trace.h
index bf16961..7138990 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -2,8 +2,6 @@
 #ifndef _LINUX_TRACE_H
 #define _LINUX_TRACE_H
 
-#ifdef CONFIG_TRACING
-
 #define TRACE_EXPORT_FUNCTION	BIT(0)
 #define TRACE_EXPORT_EVENT	BIT(1)
 #define TRACE_EXPORT_MARKER	BIT(2)
@@ -28,6 +26,8 @@ struct trace_export {
 	int flags;
 };
 
+#ifdef CONFIG_TRACING
+
 int register_ftrace_export(struct trace_export *export);
 int unregister_ftrace_export(struct trace_export *export);
 
@@ -48,6 +48,40 @@ void osnoise_arch_unregister(void);
 void osnoise_trace_irq_entry(int id);
 void osnoise_trace_irq_exit(int id, const char *desc);
 
+#else	/* CONFIG_TRACING */
+static inline int register_ftrace_export(struct trace_export *export)
+{
+	return -EINVAL;
+}
+static inline int unregister_ftrace_export(struct trace_export *export)
+{
+	return 0;
+}
+static inline void trace_printk_init_buffers(void)
+{
+}
+static inline int
+trace_array_printk(struct trace_array *tr, unsigned long ip,
+		       const char *fmt, ...)
+{
+	return 0;
+}
+static inline int trace_array_init_printk(struct trace_array *tr)
+{
+	return -EINVAL;
+}
+static inline void trace_array_put(struct trace_array *tr)
+{
+}
+static inline struct trace_array *
+trace_array_get_by_name(const char *name)
+{
+	return NULL;
+}
+static inline int trace_array_destroy(struct trace_array *tr)
+{
+	return 0;
+}
 #endif	/* CONFIG_TRACING */
 
 #endif	/* _LINUX_TRACE_H */
-- 
2.9.5

Re: [PATCH v2 1/1] tracing: Fix compile error in trace_array calls when TRACING is disabled

[Bart Van Assche]

On 9/7/22 11:57, Arun Easi wrote:
> +#else	/* CONFIG_TRACING */
> +static inline int register_ftrace_export(struct trace_export *export)
> +{
> +	return -EINVAL;
> +}
> +static inline int unregister_ftrace_export(struct trace_export *export)
> +{
> +	return 0;
> +}

Isn't it recommended to leave a blank line between function definitions?

> +static inline int
> +trace_array_printk(struct trace_array *tr, unsigned long ip,
> +		       const char *fmt, ...)

This is not the recommended way to format a function definition. 
Consider running git clang-format HEAD^.

> +static inline struct trace_array *
> +trace_array_get_by_name(const char *name)

Same comment here.

Thanks,

Bart.

Re: [PATCH v2 1/1] tracing: Fix compile error in trace_array calls when TRACING is disabled

[Steven Rostedt]

On Wed, 7 Sep 2022 12:27:33 -0700
Bart Van Assche <bvanassche@acm.org> wrote:

> On 9/7/22 11:57, Arun Easi wrote:
> > +#else	/* CONFIG_TRACING */
> > +static inline int register_ftrace_export(struct trace_export *export)
> > +{
> > +	return -EINVAL;
> > +}
> > +static inline int unregister_ftrace_export(struct trace_export *export)
> > +{
> > +	return 0;
> > +}  
> 
> Isn't it recommended to leave a blank line between function definitions?

Not really for stub functions in header files.

-- Steve

Re: [EXT] Re: [PATCH v2 1/1] tracing: Fix compile error in trace_array calls when TRACING is disabled

[Arun Easi]

On Wed, 7 Sep 2022, 12:27pm, Bart Van Assche wrote:

> External Email
> 
> ----------------------------------------------------------------------
> On 9/7/22 11:57, Arun Easi wrote:
> > +#else	/* CONFIG_TRACING */
> > +static inline int register_ftrace_export(struct trace_export *export)
> > +{
> > +	return -EINVAL;
> > +}
> > +static inline int unregister_ftrace_export(struct trace_export *export)
> > +{
> > +	return 0;
> > +}
> 
> Isn't it recommended to leave a blank line between function definitions?
> 
> > +static inline int
> > +trace_array_printk(struct trace_array *tr, unsigned long ip,
> > +		       const char *fmt, ...)
> 
> This is not the recommended way to format a function definition.

That was mostly a Y&P from the prototype earlier in the file. Is it the 
linebreak after "int" you are referring to, or are there more?

> Consider running git clang-format HEAD^.

It is a bit cryptic to me what it is complaining about (sorry 
clang-format newbie here):

# git clang-format -v HEAD^
Running clang-format on the following files:
    include/linux/trace.h
YAML:671:20: error: unknown enumerated scalar
SpaceBeforeParens: ControlStatementsExceptForEachMacros
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Error reading /root/aeasi/src/mkp.git/.clang-format: Invalid argument
error: `clang-format -lines=29:30 -lines=51:84 include/linux/trace.h` failed

Perhaps my clang-tools are not recent enough.

# clang-format --version
clang-format version 10.0.1 (Red Hat 10.0.1-1.module+el8.3.0+7459+90c24896)

Still digging..

Regards,
-Arun


> 
> > +static inline struct trace_array *
> > +trace_array_get_by_name(const char *name)
> 
> Same comment here.
> 
> Thanks,
> 
> Bart.
> 

Re: [EXT] Re: [PATCH v2 1/1] tracing: Fix compile error in trace_array calls when TRACING is disabled

[Arun Easi]

On Wed, 7 Sep 2022, 3:52pm, Arun Easi wrote:

> On Wed, 7 Sep 2022, 12:27pm, Bart Van Assche wrote:
> 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On 9/7/22 11:57, Arun Easi wrote:
> > > +#else	/* CONFIG_TRACING */
> > > +static inline int register_ftrace_export(struct trace_export *export)
> > > +{
> > > +	return -EINVAL;
> > > +}
> > > +static inline int unregister_ftrace_export(struct trace_export *export)
> > > +{
> > > +	return 0;
> > > +}
> > 
> > Isn't it recommended to leave a blank line between function definitions?
> > 
> > > +static inline int
> > > +trace_array_printk(struct trace_array *tr, unsigned long ip,
> > > +		       const char *fmt, ...)
> > 
> > This is not the recommended way to format a function definition.
> 
> That was mostly a Y&P from the prototype earlier in the file. Is it the 
> linebreak after "int" you are referring to, or are there more?
> 
> > Consider running git clang-format HEAD^.
> 
> It is a bit cryptic to me what it is complaining about (sorry 
> clang-format newbie here):
> 
> # git clang-format -v HEAD^
> Running clang-format on the following files:
>     include/linux/trace.h
> YAML:671:20: error: unknown enumerated scalar
> SpaceBeforeParens: ControlStatementsExceptForEachMacros
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Error reading /root/aeasi/src/mkp.git/.clang-format: Invalid argument
> error: `clang-format -lines=29:30 -lines=51:84 include/linux/trace.h` failed
> 
> Perhaps my clang-tools are not recent enough.
> 
> # clang-format --version
> clang-format version 10.0.1 (Red Hat 10.0.1-1.module+el8.3.0+7459+90c24896)
> 
> Still digging..
> 

Never mind.

Moved to a different machine with newer git and "clang-format" is working 
fine. I will post v3 shortly.

Regards,
-Arun

> 
> 
> > 
> > > +static inline struct trace_array *
> > > +trace_array_get_by_name(const char *name)
> > 
> > Same comment here.
> > 
> > Thanks,
> > 
> > Bart.
> > 
> 

Re: [EXT] Re: [PATCH v2 1/1] tracing: Fix compile error in trace_array calls when TRACING is disabled

[Bart Van Assche]

On 9/7/22 15:52, Arun Easi wrote:
> On Wed, 7 Sep 2022, 12:27pm, Bart Van Assche wrote:
>>> +static inline int
>>> +trace_array_printk(struct trace_array *tr, unsigned long ip,
>>> +		       const char *fmt, ...)
>>
>> This is not the recommended way to format a function definition.
> 
> That was mostly a Y&P from the prototype earlier in the file. Is it the
> linebreak after "int" you are referring to, or are there more?

In allmost all kernel code I have seen the function name is on the same 
line as the return type. Additionally, a common style is to align the 
second line with arguments with the opening parenthesis. From 
Documentation/process/coding-style.rst: "A very commonly used style
is to align descendants to a function open parenthesis."

>> Consider running git clang-format HEAD^.
> 
> It is a bit cryptic to me what it is complaining about (sorry
> clang-format newbie here):
> 
> # git clang-format -v HEAD^
> Running clang-format on the following files:
>      include/linux/trace.h
> YAML:671:20: error: unknown enumerated scalar
> SpaceBeforeParens: ControlStatementsExceptForEachMacros
>                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Error reading /root/aeasi/src/mkp.git/.clang-format: Invalid argument
> error: `clang-format -lines=29:30 -lines=51:84 include/linux/trace.h` failed
> 
> Perhaps my clang-tools are not recent enough.
> 
> # clang-format --version
> clang-format version 10.0.1 (Red Hat 10.0.1-1.module+el8.3.0+7459+90c24896)
> 
> Still digging..

git clang-format HEAD^ reformats the topmost commit according to the 
rules in the .clang-format file in the top-level directory. Please 
review any changes made by that command before amending these to the 
original commit.

I think the error messages above indicate that your version of 
clang-format is too old.

Thanks,

Bart.