2022-09-07 19:12:49

by Arun Easi

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

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


2022-09-07 19:50:25

by Arun Easi

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

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 <[email protected]>
Signed-off-by: Arun Easi <[email protected]>
---
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

2022-09-07 19:57:48

by Bart Van Assche

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

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.

2022-09-07 20:58:02

by Steven Rostedt

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

On Wed, 7 Sep 2022 12:27:33 -0700
Bart Van Assche <[email protected]> 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

2022-09-07 22:59:30

by Arun Easi

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

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.
>

2022-09-07 23:30:42

by Bart Van Assche

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

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.

2022-09-08 00:29:05

by Arun Easi

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

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.
> >
>