Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On Thu, Dec 10, 2015 at 11:03:34AM +0100, Vlastimil Babka wrote:
> On 12/10/2015 05:04 AM, Steven Rostedt wrote:
> >On Thu, Dec 10, 2015 at 11:59:44AM +0900, Joonsoo Kim wrote:
> >>Ccing, Steven to ask trace-cmd problem.
> >>
> >>I'd like to use %pgp in tracepoint output. It works well when I do
> >>'cat /sys/kernel/debug/tracing/trace' but not works well when I do
> >>'./trace-cmd report'. It prints following error log.
> >>
> >> [page_ref:page_ref_unfreeze] bad op token &
> >> [page_ref:page_ref_set] bad op token &
> >> [page_ref:page_ref_mod_unless] bad op token &
> >> [page_ref:page_ref_mod_and_test] bad op token &
> >> [page_ref:page_ref_mod_and_return] bad op token &
> >> [page_ref:page_ref_mod] bad op token &
> >> [page_ref:page_ref_freeze] bad op token &
> >>
> >>Following is the format I used.
> >>
> >>TP_printk("pfn=0x%lx flags=%pgp count=%d mapcount=%d mapping=%p mt=%d
> >>val=%d ret=%d",
> >> __entry->pfn, &__entry->flags, __entry->count,
> >> __entry->mapcount, __entry->mapping, __entry->mt,
> >> __entry->val, __entry->ret)
> >>
> >>Could it be solved by 'trace-cmd' itself?
>
> You mean that trace-cmd/parse-events.c would interpret the raw value
> of flags by itself? That would mean the flags became fixed ABI, not
> a good idea...
>
> >>Or it's better to pass flags by value?
>
> If it's value (as opposed to a pointer in %pgp), that doesn't change
> much wrt. having to intepret them?
>
> >>Or should I use something like show_gfp_flags()?
>
> Sounds like least pain to me, at least for now. We just need to have
> the translation tables available as #define with __print_flags() in
> some trace/events header, like the existing trace/events/gfpflags.h
> for gfp flags. These tables can still be reused within mm/debug.c or
> printk code without copy/paste, like I did in "[PATCH v2 6/9] mm,
> debug: introduce dump_gfpflag_names() for symbolic printing of
> gfp_flags" [1]. Maybe it's not the most elegant solution, but works
> without changing parse-events.c using the existing format export.
>
> So if you agree, I can do this in the next spin.
>
Okay. I'm okay with this approach.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On 9 December 2015 at 11:29, Arnd Bergmann wrote:
> On Friday 04 December 2015 16:16:33 Vlastimil Babka wrote:
>> --- a/include/linux/mmdebug.h
>> +++ b/include/linux/mmdebug.h
>> @@ -2,15 +2,20 @@
>> #define LINUX_MM_DEBUG_H 1
>>
>> #include
>> +#include
>> +#include
>
> 8<-
> Subject: mm: fix generated/bounds.h
>
> The inclusion of linux/tracepoint.h is causing build errors for me in ARM
> randconfig:
>
> In file included from /git/arm-soc/include/linux/ktime.h:25:0,
> from /git/arm-soc/include/linux/rcupdate.h:47,
> from /git/arm-soc/include/linux/tracepoint.h:19,
> from /git/arm-soc/include/linux/mmdebug.h:6,
> from /git/arm-soc/include/linux/page-flags.h:10,
> from /git/arm-soc/kernel/bounds.c:9:
> /git/arm-soc/include/linux/jiffies.h:10:33: fatal error:
> generated/timeconst.h: No such file or directory
> compilation terminated.
>
> To work around this, we can stop including linux/mmdebug.h from
> linux/page_flags.h
> while generating bounds.h, as we do for mm_types.h already.
>
> Signed-off-by: Arnd Bergmann
> Fixes: 8c0d593d0f8f ("mm, printk: introduce new format string for flags")
>
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 19724e6ebd26..4efad0578a28 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -7,8 +7,8 @@
>
> #include
> #include
> -#include
> #ifndef __GENERATING_BOUNDS_H
> +#include
> #include
> #include
> #endif /* !__GENERATING_BOUNDS_H */
Same build issue observed for metag too.
Tested-by: James Hogan
Cheers
James
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On 12/10/2015 05:04 AM, Steven Rostedt wrote:
On Thu, Dec 10, 2015 at 11:59:44AM +0900, Joonsoo Kim wrote:
Ccing, Steven to ask trace-cmd problem.
I'd like to use %pgp in tracepoint output. It works well when I do
'cat /sys/kernel/debug/tracing/trace' but not works well when I do
'./trace-cmd report'. It prints following error log.
[page_ref:page_ref_unfreeze] bad op token &
[page_ref:page_ref_set] bad op token &
[page_ref:page_ref_mod_unless] bad op token &
[page_ref:page_ref_mod_and_test] bad op token &
[page_ref:page_ref_mod_and_return] bad op token &
[page_ref:page_ref_mod] bad op token &
[page_ref:page_ref_freeze] bad op token &
Following is the format I used.
TP_printk("pfn=0x%lx flags=%pgp count=%d mapcount=%d mapping=%p mt=%d val=%d
ret=%d",
__entry->pfn, &__entry->flags, __entry->count,
__entry->mapcount, __entry->mapping, __entry->mt,
__entry->val, __entry->ret)
Could it be solved by 'trace-cmd' itself?
You mean that trace-cmd/parse-events.c would interpret the raw value of
flags by itself? That would mean the flags became fixed ABI, not a good
idea...
Or it's better to pass flags by value?
If it's value (as opposed to a pointer in %pgp), that doesn't change
much wrt. having to intepret them?
Or should I use something like show_gfp_flags()?
Sounds like least pain to me, at least for now. We just need to have the
translation tables available as #define with __print_flags() in some
trace/events header, like the existing trace/events/gfpflags.h for gfp
flags. These tables can still be reused within mm/debug.c or printk code
without copy/paste, like I did in "[PATCH v2 6/9] mm, debug: introduce
dump_gfpflag_names() for symbolic printing of gfp_flags" [1]. Maybe it's
not the most elegant solution, but works without changing parse-events.c
using the existing format export.
So if you agree, I can do this in the next spin.
[1] https://lkml.org/lkml/2015/11/24/354
Yes this can be solved in perf and trace-cmd via the parse-events.c file. And
as soon as that happens, whatever method we decide upon becomes a userspace
ABI. So don't think you can change it later.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On 12/10/2015 04:51 AM, Steven Rostedt wrote:
I should have been Cc'd on this as I'm the maintainer of a few of the files
here that is being modified.
Sorry about that.
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -15,16 +15,6 @@ struct tracer;
struct dentry;
struct bpf_prog;
-struct trace_print_flags {
- unsigned long mask;
- const char *name;
-};
-
-struct trace_print_flags_u64 {
- unsigned long long mask;
- const char *name;
-};
-
Ingo took some patches from Andi Kleen that creates a tracepoint-defs.h file
If anything, these should be moved there. That code is currently in tip.
Yeah I noticed that yesterday and seems like a good idea. Rasmus
suggested types.h but these didn't seem general enough for that one. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On Thu, Dec 10 2015, Steven Rostedt wrote:
> On Thu, Dec 10, 2015 at 11:59:44AM +0900, Joonsoo Kim wrote:
>>
>> [page_ref:page_ref_unfreeze] bad op token &
>> [page_ref:page_ref_set] bad op token &
>> [page_ref:page_ref_mod_unless] bad op token &
>> [page_ref:page_ref_mod_and_test] bad op token &
>> [page_ref:page_ref_mod_and_return] bad op token &
>> [page_ref:page_ref_mod] bad op token &
>> [page_ref:page_ref_freeze] bad op token &
>>
>> Following is the format I used.
>>
>> TP_printk("pfn=0x%lx flags=%pgp count=%d mapcount=%d mapping=%p mt=%d val=%d
>> ret=%d",
>> __entry->pfn, &__entry->flags, __entry->count,
>> __entry->mapcount, __entry->mapping, __entry->mt,
>> __entry->val, __entry->ret)
>>
>> Could it be solved by 'trace-cmd' itself?
>> Or it's better to pass flags by value?
>> Or should I use something like show_gfp_flags()?
>
> Yes this can be solved in perf and trace-cmd via the parse-events.c file. And
> as soon as that happens, whatever method we decide upon becomes a userspace
> ABI. So don't think you can change it later.
So somewhat off-topic, but this reminds me of a question I've been
meaning to ask: What makes it safe to stash the pointer values in
vbin_printf and only dereference them later in bstr_printf? For plain
pointer printing (%p) it's of course not a problem, but quite a few of
the %p extensions do dereference the pointer in one way or another (at
least %p[dD], %p[mM], %p[iI], %ph, %pE, %pC, %pNF, %pU, %pa and probably
soon %pg).
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On Wed, Dec 09, 2015 at 11:04:56PM -0500, Steven Rostedt wrote:
> On Thu, Dec 10, 2015 at 11:59:44AM +0900, Joonsoo Kim wrote:
> > Ccing, Steven to ask trace-cmd problem.
>
> Thanks, I should have been Cc'd for the rest as well.
>
> >
> > On Fri, Dec 04, 2015 at 04:16:33PM +0100, Vlastimil Babka wrote:
> > > In mm we use several kinds of flags bitfields that are sometimes printed
> > > for
> > > debugging purposes, or exported to userspace via sysfs. To make them
> > > easier to
> > > interpret independently on kernel version and config, we want to dump
> > > also the
> > > symbolic flag names. So far this has been done with repeated calls to
> > > pr_cont(), which is unreliable on SMP, and not usable for e.g. sysfs
> > > export.
> > >
> > > To get a more reliable and universal solution, this patch extends printk()
> > > format string for pointers to handle the page flags (%pgp), gfp_flags
> > > (%pgg)
> > > and vma flags (%pgv). Existing users of dump_flag_names() are converted
> > > and
> > > simplified.
> > >
> > > It would be possible to pass flags by value instead of pointer, but the %p
> > > format string for pointers already has extensions for various kernel
> > > structures, so it's a good fit, and the extra indirection in a
> > > non-critical
> > > path is negligible.
> >
> > I'd like to use %pgp in tracepoint output. It works well when I do
> > 'cat /sys/kernel/debug/tracing/trace' but not works well when I do
> > './trace-cmd report'. It prints following error log.
> >
> > [page_ref:page_ref_unfreeze] bad op token &
> > [page_ref:page_ref_set] bad op token &
> > [page_ref:page_ref_mod_unless] bad op token &
> > [page_ref:page_ref_mod_and_test] bad op token &
> > [page_ref:page_ref_mod_and_return] bad op token &
> > [page_ref:page_ref_mod] bad op token &
> > [page_ref:page_ref_freeze] bad op token &
> >
> > Following is the format I used.
> >
> > TP_printk("pfn=0x%lx flags=%pgp count=%d mapcount=%d mapping=%p mt=%d
> > val=%d ret=%d",
> > __entry->pfn, &__entry->flags, __entry->count,
> > __entry->mapcount, __entry->mapping, __entry->mt,
> > __entry->val, __entry->ret)
> >
> > Could it be solved by 'trace-cmd' itself?
> > Or it's better to pass flags by value?
> > Or should I use something like show_gfp_flags()?
>
> Yes this can be solved in perf and trace-cmd via the parse-events.c file. And
> as soon as that happens, whatever method we decide upon becomes a userspace
> ABI. So don't think you can change it later.
Okay. Because it can be solved by perf and trace-cmd via the
parse-events.c, I have no preference whether it is passed by value or
not.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On Thu, Dec 10, 2015 at 11:59:44AM +0900, Joonsoo Kim wrote:
> Ccing, Steven to ask trace-cmd problem.
Thanks, I should have been Cc'd for the rest as well.
>
> On Fri, Dec 04, 2015 at 04:16:33PM +0100, Vlastimil Babka wrote:
> > In mm we use several kinds of flags bitfields that are sometimes printed for
> > debugging purposes, or exported to userspace via sysfs. To make them easier
> > to
> > interpret independently on kernel version and config, we want to dump also
> > the
> > symbolic flag names. So far this has been done with repeated calls to
> > pr_cont(), which is unreliable on SMP, and not usable for e.g. sysfs export.
> >
> > To get a more reliable and universal solution, this patch extends printk()
> > format string for pointers to handle the page flags (%pgp), gfp_flags (%pgg)
> > and vma flags (%pgv). Existing users of dump_flag_names() are converted and
> > simplified.
> >
> > It would be possible to pass flags by value instead of pointer, but the %p
> > format string for pointers already has extensions for various kernel
> > structures, so it's a good fit, and the extra indirection in a non-critical
> > path is negligible.
>
> I'd like to use %pgp in tracepoint output. It works well when I do
> 'cat /sys/kernel/debug/tracing/trace' but not works well when I do
> './trace-cmd report'. It prints following error log.
>
> [page_ref:page_ref_unfreeze] bad op token &
> [page_ref:page_ref_set] bad op token &
> [page_ref:page_ref_mod_unless] bad op token &
> [page_ref:page_ref_mod_and_test] bad op token &
> [page_ref:page_ref_mod_and_return] bad op token &
> [page_ref:page_ref_mod] bad op token &
> [page_ref:page_ref_freeze] bad op token &
>
> Following is the format I used.
>
> TP_printk("pfn=0x%lx flags=%pgp count=%d mapcount=%d mapping=%p mt=%d val=%d
> ret=%d",
> __entry->pfn, &__entry->flags, __entry->count,
> __entry->mapcount, __entry->mapping, __entry->mt,
> __entry->val, __entry->ret)
>
> Could it be solved by 'trace-cmd' itself?
> Or it's better to pass flags by value?
> Or should I use something like show_gfp_flags()?
Yes this can be solved in perf and trace-cmd via the parse-events.c file. And
as soon as that happens, whatever method we decide upon becomes a userspace
ABI. So don't think you can change it later.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
I should have been Cc'd on this as I'm the maintainer of a few of the files
here that is being modified.
On Fri, Dec 04, 2015 at 04:16:33PM +0100, Vlastimil Babka wrote:
> In mm we use several kinds of flags bitfields that are sometimes printed for
> debugging purposes, or exported to userspace via sysfs. To make them easier to
> interpret independently on kernel version and config, we want to dump also the
> symbolic flag names. So far this has been done with repeated calls to
> pr_cont(), which is unreliable on SMP, and not usable for e.g. sysfs export.
>
> To get a more reliable and universal solution, this patch extends printk()
> format string for pointers to handle the page flags (%pgp), gfp_flags (%pgg)
> and vma flags (%pgv). Existing users of dump_flag_names() are converted and
> simplified.
>
> It would be possible to pass flags by value instead of pointer, but the %p
> format string for pointers already has extensions for various kernel
> structures, so it's a good fit, and the extra indirection in a non-critical
> path is negligible.
>
> [li...@rasmusvillemoes.dk: lots of good implementation suggestions]
> Signed-off-by: Vlastimil Babka
> Cc: Rasmus Villemoes
> Cc: Joonsoo Kim
> Cc: Minchan Kim
> Cc: Sasha Levin
> Cc: "Kirill A. Shutemov"
> Cc: Mel Gorman
> Cc: Michal Hocko
> ---
> Should hopefully apply to mmots (release mmotm doesn't have the 3 -fix
> patches yet?)
>
> Documentation/printk-formats.txt | 14
> include/linux/mmdebug.h | 7 +-
> include/linux/trace_events.h | 10 ---
> include/linux/tracepoint.h | 10 +++
> lib/vsprintf.c | 75 ++
> mm/debug.c | 135
> ++-
> mm/oom_kill.c| 5 +-
> mm/page_alloc.c | 5 +-
> mm/page_owner.c | 6 +-
> 9 files changed, 160 insertions(+), 107 deletions(-)
>
> diff --git a/Documentation/printk-formats.txt
> b/Documentation/printk-formats.txt
> index b784c270105f..8b6ab00fcfc9 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.txt
> @@ -292,6 +292,20 @@ Raw pointer value SHOULD be printed with %p. The kernel
> supports
>
> Passed by reference.
>
> +Flags bitfields such as page flags, gfp_flags:
> +
> + %pgpreferenced|uptodate|lru|active|private
> + %pggGFP_USER|GFP_DMA32|GFP_NOWARN
> + %pgvread|exec|mayread|maywrite|mayexec|denywrite
> +
> + For printing flags bitfields as a collection of symbolic constants that
> + would construct the value. The type of flags is given by the third
> + character. Currently supported are [p]age flags, [g]fp_flags and
> + [v]ma_flags. The flag names and print order depends on the particular
> + type.
> +
> + Passed by reference.
> +
> Network device features:
>
> %pNF0xc000
> diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
> index 3b77fab7ad28..2c8286cf162e 100644
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -2,15 +2,20 @@
> #define LINUX_MM_DEBUG_H 1
>
> #include
> +#include
> +#include
>
> struct page;
> struct vm_area_struct;
> struct mm_struct;
>
> +extern const struct trace_print_flags pageflag_names[];
> +extern const struct trace_print_flags vmaflag_names[];
> +extern const struct trace_print_flags gfpflag_names[];
> +
> extern void dump_page(struct page *page, const char *reason);
> extern void dump_page_badflags(struct page *page, const char *reason,
> unsigned long badflags);
> -extern void dump_gfpflag_names(unsigned long gfp_flags);
> void dump_vma(const struct vm_area_struct *vma);
> void dump_mm(const struct mm_struct *mm);
>
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index 429fdfc3baf5..d91404f89ff2 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -15,16 +15,6 @@ struct tracer;
> struct dentry;
> struct bpf_prog;
>
> -struct trace_print_flags {
> - unsigned long mask;
> - const char *name;
> -};
> -
> -struct trace_print_flags_u64 {
> - unsigned long long mask;
> - const char *name;
> -};
> -
Ingo took some patches from Andi Kleen that creates a tracepoint-defs.h file
If anything, these should be moved there. That code is currently in tip.
-- Steve
> const char *trace_print_flags_seq(struct trace_seq *p, const char *delim,
> unsigned long flags,
> const struct trace_print_flags *flag_array);
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 7834a8a8bf1e..a5d0ab46724d 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -43,6 +43,16 @@ struct trace_enum_map {
> unsigned long enum_value;
> };
>
> +struct trace_print_flags {
> + un
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
Ccing, Steven to ask trace-cmd problem.
On Fri, Dec 04, 2015 at 04:16:33PM +0100, Vlastimil Babka wrote:
> In mm we use several kinds of flags bitfields that are sometimes printed for
> debugging purposes, or exported to userspace via sysfs. To make them easier to
> interpret independently on kernel version and config, we want to dump also the
> symbolic flag names. So far this has been done with repeated calls to
> pr_cont(), which is unreliable on SMP, and not usable for e.g. sysfs export.
>
> To get a more reliable and universal solution, this patch extends printk()
> format string for pointers to handle the page flags (%pgp), gfp_flags (%pgg)
> and vma flags (%pgv). Existing users of dump_flag_names() are converted and
> simplified.
>
> It would be possible to pass flags by value instead of pointer, but the %p
> format string for pointers already has extensions for various kernel
> structures, so it's a good fit, and the extra indirection in a non-critical
> path is negligible.
I'd like to use %pgp in tracepoint output. It works well when I do
'cat /sys/kernel/debug/tracing/trace' but not works well when I do
'./trace-cmd report'. It prints following error log.
[page_ref:page_ref_unfreeze] bad op token &
[page_ref:page_ref_set] bad op token &
[page_ref:page_ref_mod_unless] bad op token &
[page_ref:page_ref_mod_and_test] bad op token &
[page_ref:page_ref_mod_and_return] bad op token &
[page_ref:page_ref_mod] bad op token &
[page_ref:page_ref_freeze] bad op token &
Following is the format I used.
TP_printk("pfn=0x%lx flags=%pgp count=%d mapcount=%d mapping=%p mt=%d val=%d
ret=%d",
__entry->pfn, &__entry->flags, __entry->count,
__entry->mapcount, __entry->mapping, __entry->mt,
__entry->val, __entry->ret)
Could it be solved by 'trace-cmd' itself?
Or it's better to pass flags by value?
Or should I use something like show_gfp_flags()?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On 12/09/2015 12:29 PM, Arnd Bergmann wrote:
> On Friday 04 December 2015 16:16:33 Vlastimil Babka wrote:
>> --- a/include/linux/mmdebug.h
>> +++ b/include/linux/mmdebug.h
>> @@ -2,15 +2,20 @@
>> #define LINUX_MM_DEBUG_H 1
>>
>> #include
>> +#include
>> +#include
>
> 8<-
> Subject: mm: fix generated/bounds.h
>
> The inclusion of linux/tracepoint.h is causing build errors for me in ARM
> randconfig:
>
> In file included from /git/arm-soc/include/linux/ktime.h:25:0,
> from /git/arm-soc/include/linux/rcupdate.h:47,
> from /git/arm-soc/include/linux/tracepoint.h:19,
> from /git/arm-soc/include/linux/mmdebug.h:6,
> from /git/arm-soc/include/linux/page-flags.h:10,
> from /git/arm-soc/kernel/bounds.c:9:
> /git/arm-soc/include/linux/jiffies.h:10:33: fatal error:
> generated/timeconst.h: No such file or directory
> compilation terminated.
>
> To work around this, we can stop including linux/mmdebug.h from
> linux/page_flags.h
> while generating bounds.h, as we do for mm_types.h already.
>
> Signed-off-by: Arnd Bergmann
Thanks and sorry. Andrew can you please include it in mmotm as -fix for now?
I plan to respin the whole of this later with some patch splitting and
reordering to reduce churn and follow Rasmus' advice.
Also I've just learned that there's a new lightweight tracepoint-defs.h in -tip
thanks to Andi, which would be a better place for struct trace_print_flags than
tracepoint.h is, so I'll look into using it for the respin, which should make
this temporary -fix redundant.
> Fixes: 8c0d593d0f8f ("mm, printk: introduce new format string for flags")
Note that the linux-next commit id is volatile here (regenerated from quilt
series).
>
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 19724e6ebd26..4efad0578a28 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -7,8 +7,8 @@
>
> #include
> #include
> -#include
> #ifndef __GENERATING_BOUNDS_H
> +#include
> #include
> #include
> #endif /* !__GENERATING_BOUNDS_H */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On Friday 04 December 2015 16:16:33 Vlastimil Babka wrote:
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -2,15 +2,20 @@
> #define LINUX_MM_DEBUG_H 1
>
> #include
> +#include
> +#include
8<-
Subject: mm: fix generated/bounds.h
The inclusion of linux/tracepoint.h is causing build errors for me in ARM
randconfig:
In file included from /git/arm-soc/include/linux/ktime.h:25:0,
from /git/arm-soc/include/linux/rcupdate.h:47,
from /git/arm-soc/include/linux/tracepoint.h:19,
from /git/arm-soc/include/linux/mmdebug.h:6,
from /git/arm-soc/include/linux/page-flags.h:10,
from /git/arm-soc/kernel/bounds.c:9:
/git/arm-soc/include/linux/jiffies.h:10:33: fatal error: generated/timeconst.h:
No such file or directory
compilation terminated.
To work around this, we can stop including linux/mmdebug.h from
linux/page_flags.h
while generating bounds.h, as we do for mm_types.h already.
Signed-off-by: Arnd Bergmann
Fixes: 8c0d593d0f8f ("mm, printk: introduce new format string for flags")
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 19724e6ebd26..4efad0578a28 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -7,8 +7,8 @@
#include
#include
-#include
#ifndef __GENERATING_BOUNDS_H
+#include
#include
#include
#endif /* !__GENERATING_BOUNDS_H */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] mm, printk: introduce new format string for flags
On Fri, Dec 04 2015, Vlastimil Babka wrote:
> In mm we use several kinds of flags bitfields that are sometimes printed for
> debugging purposes, or exported to userspace via sysfs. To make them easier to
> interpret independently on kernel version and config, we want to dump also the
> symbolic flag names. So far this has been done with repeated calls to
> pr_cont(), which is unreliable on SMP, and not usable for e.g. sysfs export.
>
> To get a more reliable and universal solution, this patch extends printk()
> format string for pointers to handle the page flags (%pgp), gfp_flags (%pgg)
> and vma flags (%pgv).
Hm, with that $subject, I'd expect the patch to touch little beyond
lib/vsprintf.c and Documentation/printk-formats.txt (plus whatever might
be needed to make the name arrays accessible).
> Existing users of dump_flag_names() are converted and simplified.
If you do a respin, please do that part in a separate patch. That would
make it a lot easier to review.
> diff --git a/Documentation/printk-formats.txt
> b/Documentation/printk-formats.txt
> index b784c270105f..8b6ab00fcfc9 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.txt
> @@ -292,6 +292,20 @@ Raw pointer value SHOULD be printed with %p. The kernel
> supports
>
> Passed by reference.
>
> +Flags bitfields such as page flags, gfp_flags:
> +
> + %pgpreferenced|uptodate|lru|active|private
> + %pggGFP_USER|GFP_DMA32|GFP_NOWARN
> + %pgvread|exec|mayread|maywrite|mayexec|denywrite
> +
> + For printing flags bitfields as a collection of symbolic constants that
> + would construct the value. The type of flags is given by the third
> + character. Currently supported are [p]age flags, [g]fp_flags and
> + [v]ma_flags. The flag names and print order depends on the particular
> + type.
> +
> + Passed by reference.
> +
It should probably be emphasized that %pgp and %pgv expect (unsigned
long*), while %pgg expect (gfp_t*), just as you do in vsprintf.c.
> Network device features:
>
> %pNF0xc000
> diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
> index 3b77fab7ad28..2c8286cf162e 100644
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -2,15 +2,20 @@
> #define LINUX_MM_DEBUG_H 1
>
> #include
> +#include
> +#include
>
How much header bloat does it cause by making all users of mmdebug.h
also include tracepoint.h? Couldn't we put the struct definition in
types.h instead?
> struct page;
> struct vm_area_struct;
> struct mm_struct;
>
> +extern const struct trace_print_flags pageflag_names[];
> +extern const struct trace_print_flags vmaflag_names[];
> +extern const struct trace_print_flags gfpflag_names[];
> +
> extern void dump_page(struct page *page, const char *reason);
> extern void dump_page_badflags(struct page *page, const char *reason,
> unsigned long badflags);
> -extern void dump_gfpflag_names(unsigned long gfp_flags);
> void dump_vma(const struct vm_area_struct *vma);
> void dump_mm(const struct mm_struct *mm);
>
>
> extern int
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index f9cee8e1233c..9a0697b14ea3 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
>
> #include /* for PAGE_SIZE */
> #include /* for dereference_function_descriptor() */
> @@ -1361,6 +1362,73 @@ char *clock(char *buf, char *end, struct clk *clk,
> struct printf_spec spec,
> }
> }
>
> +static
> +char *format_flags(char *buf, char *end, unsigned long flags,
> + const struct trace_print_flags *names)
> +{
> + unsigned long mask;
> + const struct printf_spec strspec = {
> + .field_width = -1,
> + .precision = -1,
> + };
> + const struct printf_spec numspec = {
> + .flags = SPECIAL|SMALL,
> + .field_width = -1,
> + .precision = -1,
> + .base = 16,
> + };
> +
> + for ( ; flags && (names->mask || names->name); names++) {
Why test both ->mask and ->name? I could imagine some constant being
#defined to 0 due to some CONFIG_* (and stuff that tested "flag & THAT"
would then get compiled away), so maybe ->mask is insufficient. But
->name will always be non-NULL for all but the sentinel entry, right?
> + mask = names->mask;
> + if ((flags & mask) != mask)
> + continue;
And if we have some constant which is 0, this will then actually cause
us to print the corresponding string. Do we want that? If not we should
have a "if (!mask) continue;". And how helpful are these strings really
if their meaning might be .config dependent?
> + buf = string(buf, end, names->name, strspec);
So string() is robust against a NULL string (printing the string
"(null)"), but that seems silly to rely on. So I'd st
