2019-08-26 13:29:58

by Gao Xiang

[permalink] [raw]
Subject: [PATCH] erofs: fix compile warnings when moving out include/trace/events/erofs.h

As Stephon reported [1], many compile warnings are raised when
moving out include/trace/events/erofs.h:

In file included from include/trace/events/erofs.h:8,
from <command-line>:
include/trace/events/erofs.h:28:37: warning: 'struct dentry' declared inside parameter list will not be visible outside of this definition or declaration
TP_PROTO(struct inode *dir, struct dentry *dentry, unsigned int flags),
^~~~~~
include/linux/tracepoint.h:233:34: note: in definition of macro '__DECLARE_TRACE'
static inline void trace_##name(proto) \
^~~~~
include/linux/tracepoint.h:396:24: note: in expansion of macro 'PARAMS'
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
^~~~~~
include/linux/tracepoint.h:532:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^~~~~~~~~~~~~
include/linux/tracepoint.h:532:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^~~~~~
include/trace/events/erofs.h:26:1: note: in expansion of macro 'TRACE_EVENT'
TRACE_EVENT(erofs_lookup,
^~~~~~~~~~~
include/trace/events/erofs.h:28:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(struct inode *dir, struct dentry *dentry, unsigned int flags),
^~~~~~~~

That makes me very confused since most original EROFS tracepoint code
was taken from f2fs, and finally I found

commit 43c78d88036e ("kbuild: compile-test kernel headers to ensure they are self-contained")

It seems these warnings are generated from KERNEL_HEADER_TEST feature and
ext4/f2fs tracepoint files were in blacklist.

Anyway, let's fix these issues for KERNEL_HEADER_TEST feature instead
of adding to blacklist...

[1] https://lore.kernel.org/lkml/[email protected]/
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Gao Xiang <[email protected]>
---

Hi Chao and Greg,
It seems the root cause reported by Stephen is the following (sorry for
taking some time...) could you kindly review and merge this patch?

Thanks,
Gao Xiang

include/trace/events/erofs.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/include/trace/events/erofs.h b/include/trace/events/erofs.h
index bfb2da9c4eee..d239f39cbc8c 100644
--- a/include/trace/events/erofs.h
+++ b/include/trace/events/erofs.h
@@ -6,6 +6,9 @@
#define _TRACE_EROFS_H

#include <linux/tracepoint.h>
+#include <linux/fs.h>
+
+struct erofs_map_blocks;

#define show_dev(dev) MAJOR(dev), MINOR(dev)
#define show_dev_nid(entry) show_dev(entry->dev), entry->nid
--
2.17.1


2019-08-26 13:32:32

by Gao Xiang

[permalink] [raw]
Subject: [PATCH RESEND] erofs: fix compile warnings when moving out include/trace/events/erofs.h

As Stephon reported [1], many compile warnings are raised when
moving out include/trace/events/erofs.h:

In file included from include/trace/events/erofs.h:8,
from <command-line>:
include/trace/events/erofs.h:28:37: warning: 'struct dentry' declared inside parameter list will not be visible outside of this definition or declaration
TP_PROTO(struct inode *dir, struct dentry *dentry, unsigned int flags),
^~~~~~
include/linux/tracepoint.h:233:34: note: in definition of macro '__DECLARE_TRACE'
static inline void trace_##name(proto) \
^~~~~
include/linux/tracepoint.h:396:24: note: in expansion of macro 'PARAMS'
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
^~~~~~
include/linux/tracepoint.h:532:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^~~~~~~~~~~~~
include/linux/tracepoint.h:532:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^~~~~~
include/trace/events/erofs.h:26:1: note: in expansion of macro 'TRACE_EVENT'
TRACE_EVENT(erofs_lookup,
^~~~~~~~~~~
include/trace/events/erofs.h:28:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(struct inode *dir, struct dentry *dentry, unsigned int flags),
^~~~~~~~

That makes me very confused since most original EROFS tracepoint code
was taken from f2fs, and finally I found

commit 43c78d88036e ("kbuild: compile-test kernel headers to ensure they are self-contained")

It seems these warnings are generated from KERNEL_HEADER_TEST feature and
ext4/f2fs tracepoint files were in blacklist.

Anyway, let's fix these issues for KERNEL_HEADER_TEST feature instead
of adding to blacklist...

[1] https://lore.kernel.org/lkml/[email protected]/
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Gao Xiang <[email protected]>
---

[RESEND] Cc Stephen as well. no change at all...

Hi Chao and Greg,
It seems the root cause reported by Stephen is the following (sorry for
taking some time...) could you kindly review and merge this patch?

Thanks,
Gao Xiang

include/trace/events/erofs.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/include/trace/events/erofs.h b/include/trace/events/erofs.h
index bfb2da9c4eee..d239f39cbc8c 100644
--- a/include/trace/events/erofs.h
+++ b/include/trace/events/erofs.h
@@ -6,6 +6,9 @@
#define _TRACE_EROFS_H

#include <linux/tracepoint.h>
+#include <linux/fs.h>
+
+struct erofs_map_blocks;

#define show_dev(dev) MAJOR(dev), MINOR(dev)
#define show_dev_nid(entry) show_dev(entry->dev), entry->nid
--
2.17.1

2019-08-26 13:54:13

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH RESEND] erofs: fix compile warnings when moving out include/trace/events/erofs.h

On 2019-8-26 21:26, Gao Xiang wrote:
> As Stephon reported [1], many compile warnings are raised when
> moving out include/trace/events/erofs.h:
>
> In file included from include/trace/events/erofs.h:8,
> from <command-line>:
> include/trace/events/erofs.h:28:37: warning: 'struct dentry' declared inside parameter list will not be visible outside of this definition or declaration
> TP_PROTO(struct inode *dir, struct dentry *dentry, unsigned int flags),
> ^~~~~~
> include/linux/tracepoint.h:233:34: note: in definition of macro '__DECLARE_TRACE'
> static inline void trace_##name(proto) \
> ^~~~~
> include/linux/tracepoint.h:396:24: note: in expansion of macro 'PARAMS'
> __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> ^~~~~~
> include/linux/tracepoint.h:532:2: note: in expansion of macro 'DECLARE_TRACE'
> DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
> ^~~~~~~~~~~~~
> include/linux/tracepoint.h:532:22: note: in expansion of macro 'PARAMS'
> DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
> ^~~~~~
> include/trace/events/erofs.h:26:1: note: in expansion of macro 'TRACE_EVENT'
> TRACE_EVENT(erofs_lookup,
> ^~~~~~~~~~~
> include/trace/events/erofs.h:28:2: note: in expansion of macro 'TP_PROTO'
> TP_PROTO(struct inode *dir, struct dentry *dentry, unsigned int flags),
> ^~~~~~~~
>
> That makes me very confused since most original EROFS tracepoint code
> was taken from f2fs, and finally I found
>
> commit 43c78d88036e ("kbuild: compile-test kernel headers to ensure they are self-contained")
>
> It seems these warnings are generated from KERNEL_HEADER_TEST feature and
> ext4/f2fs tracepoint files were in blacklist.

For f2fs.h, it will be only used by f2fs module, I guess it's okay to let it
stay in blacklist...

>
> Anyway, let's fix these issues for KERNEL_HEADER_TEST feature instead
> of adding to blacklist...
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> Reported-by: Stephen Rothwell <[email protected]>
> Signed-off-by: Gao Xiang <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,

2019-08-26 15:29:11

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH RESEND] erofs: fix compile warnings when moving out include/trace/events/erofs.h

Hi Chao,

On Mon, Aug 26, 2019 at 09:51:35PM +0800, Chao Yu wrote:
> On 2019-8-26 21:26, Gao Xiang wrote:

[]

> > TRACE_EVENT(erofs_lookup,
> > ^~~~~~~~~~~
> > include/trace/events/erofs.h:28:2: note: in expansion of macro 'TP_PROTO'
> > TP_PROTO(struct inode *dir, struct dentry *dentry, unsigned int flags),
> > ^~~~~~~~
> >
> > That makes me very confused since most original EROFS tracepoint code
> > was taken from f2fs, and finally I found
> >
> > commit 43c78d88036e ("kbuild: compile-test kernel headers to ensure they are self-contained")
> >
> > It seems these warnings are generated from KERNEL_HEADER_TEST feature and
> > ext4/f2fs tracepoint files were in blacklist.
>
> For f2fs.h, it will be only used by f2fs module, I guess it's okay to let it
> stay in blacklist...


Yes, it depends on you f2fs folks selection...
Anyway, this file is a new file, therefore it should be better not to add to
blacklist...


>
> >
> > Anyway, let's fix these issues for KERNEL_HEADER_TEST feature instead
> > of adding to blacklist...
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> > Reported-by: Stephen Rothwell <[email protected]>
> > Signed-off-by: Gao Xiang <[email protected]>
>
> Reviewed-by: Chao Yu <[email protected]>


Thanks for reviewing :)


Thanks,
Gao Xiang

>
> Thanks,