[memcg:since-3.6 493/499] include/trace/events/filemap.h:14:1: sparse: incompatible types for operation (<)

[memcg:since-3.6 493/499] include/trace/events/filemap.h:14:1: sparse: incompatible types for operation (<)

[kbuild test robot]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
commit: 07b81da5f80b27543ddbe3164170c64e0941a812 [493/499] mm: trace filemap add and del


sparse warnings:

+ include/trace/events/filemap.h:14:1: sparse: incompatible types for operation (<)
include/trace/events/filemap.h:14:1:    left side has type struct page *<noident>
include/trace/events/filemap.h:14:1:    right side has type int
include/trace/events/filemap.h:45:1: sparse: incompatible types for operation (<)
include/trace/events/filemap.h:45:1:    left side has type struct page *<noident>
include/trace/events/filemap.h:45:1:    right side has type int
include/linux/radix-tree.h:152:16: sparse: incompatible types in comparison expression (different address spaces)
include/linux/radix-tree.h:152:16: sparse: incompatible types in comparison expression (different address spaces)
include/linux/radix-tree.h:152:16: sparse: incompatible types in comparison expression (different address spaces)
include/linux/radix-tree.h:152:16: sparse: incompatible types in comparison expression (different address spaces)

vim +14 include/trace/events/filemap.h

07b81da5 Robert Jarzmik 2012-11-29   1  #undef TRACE_SYSTEM
07b81da5 Robert Jarzmik 2012-11-29   2  #define TRACE_SYSTEM filemap
07b81da5 Robert Jarzmik 2012-11-29   3  
07b81da5 Robert Jarzmik 2012-11-29   4  #if !defined(_TRACE_FILEMAP_H) || defined(TRACE_HEADER_MULTI_READ)
07b81da5 Robert Jarzmik 2012-11-29   5  #define _TRACE_FILEMAP_H
07b81da5 Robert Jarzmik 2012-11-29   6  
07b81da5 Robert Jarzmik 2012-11-29   7  #include <linux/types.h>
07b81da5 Robert Jarzmik 2012-11-29   8  #include <linux/tracepoint.h>
07b81da5 Robert Jarzmik 2012-11-29   9  #include <linux/mm.h>
07b81da5 Robert Jarzmik 2012-11-29  10  #include <linux/memcontrol.h>
07b81da5 Robert Jarzmik 2012-11-29  11  #include <linux/device.h>
07b81da5 Robert Jarzmik 2012-11-29  12  #include <linux/kdev_t.h>
07b81da5 Robert Jarzmik 2012-11-29  13  
07b81da5 Robert Jarzmik 2012-11-29 @14  TRACE_EVENT(mm_filemap_delete_from_page_cache,
07b81da5 Robert Jarzmik 2012-11-29  15  
07b81da5 Robert Jarzmik 2012-11-29  16  	TP_PROTO(struct page *page),
07b81da5 Robert Jarzmik 2012-11-29  17  
07b81da5 Robert Jarzmik 2012-11-29  18  	TP_ARGS(page),
07b81da5 Robert Jarzmik 2012-11-29  19  
07b81da5 Robert Jarzmik 2012-11-29  20  	TP_STRUCT__entry(
07b81da5 Robert Jarzmik 2012-11-29  21  		__field(struct page *, page)
07b81da5 Robert Jarzmik 2012-11-29  22  		__field(unsigned long, i_ino)

---
0-DAY kernel build testing backend         Open Source Technology Center
Fengguang Wu, Yuanhan Liu                              Intel Corporation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [memcg:since-3.6 493/499] include/trace/events/filemap.h:14:1: sparse: incompatible types for operation (<)

[Robert Jarzmik]

kbuild test robot <fengguang.wu@intel.com> writes:

> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
> head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
> commit: 07b81da5f80b27543ddbe3164170c64e0941a812 [493/499] mm: trace filemap add and del
>
>
> sparse warnings:
>
> + include/trace/events/filemap.h:14:1: sparse: incompatible types for operation (<)
> include/trace/events/filemap.h:14:1:    left side has type struct page *<noident>
> include/trace/events/filemap.h:14:1:    right side has type int
> include/trace/events/filemap.h:45:1: sparse: incompatible types for operation (<)
> include/trace/events/filemap.h:45:1:    left side has type struct page *<noident>
> include/trace/events/filemap.h:45:1:    right side has type int

Hi Steven, Frederic and Ingo,

I just drop this note to make you aware (as FTRACE maintainers) of the sparse
warning I received.

This sparse warning will touch all submission to the FTRACE events API AFAIK.
The node of the problem :
 - in include/linux/ftrace_event.h, we have :
   #define is_signed_type(type)	(((type)(-1)) < 0)
 - this is used by kernel/trace/trace_events.c
   #define __common_field(type, item)
   ...
     is_signed_type(type)
   ...

Here, if a trace field is a pointer (for example struct page *), we end up with
this in my case :

static int __attribute__((no_instrument_function))
ftrace_define_fields_mm_filemap_delete_from_page_cache(struct ftrace_event_call
*event_call) { struct ftrace_raw_mm_filemap_delete_from_page_cache field; int
ret; ret = trace_define_field(event_call, "struct page *", "page",
__builtin_offsetof(typeof(field),page), sizeof(field.page), (((struct page
*)(-1)) < 0), FILTER_OTHER); if (ret) return ret; ret =
trace_define_field(event_call, "unsigned long", "i_ino",
__builtin_offsetof(typeof(field),i_ino), sizeof(field.i_ino), (((unsigned
long)(-1)) < 0), FILTER_OTHER); if (ret) return ret; ret =
trace_define_field(event_call, "unsigned long", "index",
__builtin_offsetof(typeof(field),index), sizeof(field.index), (((unsigned
long)(-1)) < 0), FILTER_OTHER); if (ret) return ret; ret =
trace_define_field(event_call, "dev_t", "s_dev",
__builtin_offsetof(typeof(field),s_dev), sizeof(field.s_dev), (((dev_t)(-1)) <
0), FILTER_OTHER); if (ret) return ret;; return ret; }; ;;

And I think, (((struct page *)(-1)) < 0) gives the warning. I don't know if
"is_signed_type()" makes sense on a pointer, but I think you'll get other
reports of that kind for any new event added to trace API.

Cheers.

-- 
Robert

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [memcg:since-3.6 493/499] include/trace/events/filemap.h:14:1: sparse: incompatible types for operation (<)

[Steven Rostedt]

On Sun, 2012-12-02 at 15:36 +0100, Robert Jarzmik wrote:
> kbuild test robot <fengguang.wu@intel.com> writes:
> 
> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
> > head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
> > commit: 07b81da5f80b27543ddbe3164170c64e0941a812 [493/499] mm: trace filemap add and del
> >
> >
> > sparse warnings:
> >
> > + include/trace/events/filemap.h:14:1: sparse: incompatible types for operation (<)
> > include/trace/events/filemap.h:14:1:    left side has type struct page *<noident>
> > include/trace/events/filemap.h:14:1:    right side has type int
> > include/trace/events/filemap.h:45:1: sparse: incompatible types for operation (<)
> > include/trace/events/filemap.h:45:1:    left side has type struct page *<noident>
> > include/trace/events/filemap.h:45:1:    right side has type int
> 
> Hi Steven, Frederic and Ingo,

Sorry for the late reply, It's end of year and I'm getting a lot of
pressure at work to get things done.

> 
> I just drop this note to make you aware (as FTRACE maintainers) of the sparse
> warning I received.
> 
> This sparse warning will touch all submission to the FTRACE events API AFAIK.
> The node of the problem :
>  - in include/linux/ftrace_event.h, we have :
>    #define is_signed_type(type)	(((type)(-1)) < 0)
>  - this is used by kernel/trace/trace_events.c
>    #define __common_field(type, item)
>    ...
>      is_signed_type(type)
>    ...
> 
> Here, if a trace field is a pointer (for example struct page *), we end up with
> this in my case :
> 
> static int __attribute__((no_instrument_function))
> ftrace_define_fields_mm_filemap_delete_from_page_cache(struct ftrace_event_call
> *event_call) { struct ftrace_raw_mm_filemap_delete_from_page_cache field; int
> ret; ret = trace_define_field(event_call, "struct page *", "page",
> __builtin_offsetof(typeof(field),page), sizeof(field.page), (((struct page
> *)(-1)) < 0), FILTER_OTHER); if (ret) return ret; ret =
> trace_define_field(event_call, "unsigned long", "i_ino",
> __builtin_offsetof(typeof(field),i_ino), sizeof(field.i_ino), (((unsigned
> long)(-1)) < 0), FILTER_OTHER); if (ret) return ret; ret =
> trace_define_field(event_call, "unsigned long", "index",
> __builtin_offsetof(typeof(field),index), sizeof(field.index), (((unsigned
> long)(-1)) < 0), FILTER_OTHER); if (ret) return ret; ret =
> trace_define_field(event_call, "dev_t", "s_dev",
> __builtin_offsetof(typeof(field),s_dev), sizeof(field.s_dev), (((dev_t)(-1)) <
> 0), FILTER_OTHER); if (ret) return ret;; return ret; }; ;;
> 
> And I think, (((struct page *)(-1)) < 0) gives the warning. I don't know if
> "is_signed_type()" makes sense on a pointer, but I think you'll get other
> reports of that kind for any new event added to trace API.

Hmm, this is mostly automated via the macros. Not sure how we can
differentiate a pointer from other fields. Would this be fixed if we
did:

#define is_signed_type(type) (((type)(-1) < (type)0)

?

-- Steve


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [memcg:since-3.6 493/499] include/trace/events/filemap.h:14:1: sparse: incompatible types for operation (<)

[Robert Jarzmik]

Steven Rostedt <rostedt@goodmis.org> writes:

> Sorry for the late reply, It's end of year and I'm getting a lot of
> pressure at work to get things done.
Don't worry, no hurry in here.

> Hmm, this is mostly automated via the macros. Not sure how we can
> differentiate a pointer from other fields. Would this be fixed if we
> did:
>
> #define is_signed_type(type) (((type)(-1) < (type)0)

Yes indeed, this quiesces the sparse warning, and keeps the original purpose
AFAIK. And I don't think C standard provides a way to typecheck for any kind of
pointer, so this looks the right fix to me .

Will you submit the patch or do you want me to send it ?

Cheers.

-- 
Robert

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>