Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1666637 > unrolled thread

[PATCH] oom, trace: Remove ENUM evaluation of COMPACTION_FEEDBACK

Back to article view | Back to linux.kernel

Contents

  [PATCH] oom, trace: Remove ENUM evaluation of COMPACTION_FEEDBACK Steven Rostedt <rostedt@goodmis.org> - 2017-06-15 12:50 +0200
    Re: [PATCH] oom, trace: Remove ENUM evaluation of COMPACTION_FEEDBACK Michal Hocko <mhocko@kernel.org> - 2017-06-15 13:00 +0200
      Re: [PATCH] oom, trace: Remove ENUM evaluation of  COMPACTION_FEEDBACK Steven Rostedt <rostedt@goodmis.org> - 2017-06-15 14:00 +0200

#1666637 — [PATCH] oom, trace: Remove ENUM evaluation of COMPACTION_FEEDBACK

From: Steven Rostedt (VMware) <rostedt@goodmis.org>

After enabling CONFIG_TRACE_ENUM_MAP_FILE (which will soon be renamed
to CONFIG_TRACE_EVAL_MAP_FILE), I am able to examine the enums that
have been evaluated:

 # cat /sys/kernel/debug/tracing/enum_map

(which will soon be renamed to eval_map)

And it showed some interesting results:

[..]
ZONE_MOVABLE 3 (oom)
ZONE_NORMAL 2 (oom)
ZONE_DMA32 1 (oom)
ZONE_DMA 0 (oom)
3 3 (oom)
2 2 (oom)
1 1 (oom)
COMPACT_PRIO_ASYNC 2 (oom)
COMPACT_PRIO_SYNC_LIGHT 1 (oom)
COMPACT_PRIO_SYNC_FULL 0 (oom)
[..]
ZONE_DMA 0 (vmscan)
3 3 (vmscan)
2 2 (vmscan)
1 1 (vmscan)
COMPACT_PRIO_ASYNC 2 (vmscan)
[..]
ZONE_DMA 0 (kmem)
3 3 (kmem)
2 2 (kmem)
1 1 (kmem)
COMPACT_PRIO_ASYNC 2 (kmem)
[..]
ZONE_DMA 0 (compaction)
3 3 (compaction)
2 2 (compaction)
1 1 (compaction)
COMPACT_PRIO_ASYNC 2 (compaction)
[..]

The name within the parenthesis are the trace systems that the
enum/eval maps are associated with. When there's a number evaluated to
another number, that tells me that the TRACE_DEFINE_ENUM() was used on
a #define and not an enum. As #defines get converted normally, they are
not needed to be evaluated.

Each of the above trace systems with the number to number evaluation
included the file include/trace/events/mmflags.h which has:

 /* High-level compaction status feedback */
 #define COMPACTION_FAILED       1
 #define COMPACTION_WITHDRAWN    2
 #define COMPACTION_PROGRESS     3

[..]

 #define COMPACTION_FEEDBACK             \
        EM(COMPACTION_FAILED,           "failed")       \
        EM(COMPACTION_WITHDRAWN,        "withdrawn")    \
        EMe(COMPACTION_PROGRESS,        "progress")

Which is still needed for the __print_symbolic() usage in the
trace_event. But it is not needed to be evaluated.

Removing the evaluation part removes the unnecessary evaluations of
numbers to numbers.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
diff --git a/include/trace/events/mmflags.h
b/include/trace/events/mmflags.h index 304ff94..10e3663 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -257,7 +257,7 @@
IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY,	"softdirty"
)		\ COMPACTION_STATUS
 COMPACTION_PRIORITY
-COMPACTION_FEEDBACK
+/* COMPACTION_FEEDBACK are defines not enums. Not needed here. */
 ZONE_TYPE
 LRU_NAMES
 

[toc] | [next] | [standalone]

#1666653

On Thu 15-06-17 06:48:23, Steven Rostedt wrote:
> From: Steven Rostedt (VMware) <rostedt@goodmis.org>
> 
> After enabling CONFIG_TRACE_ENUM_MAP_FILE (which will soon be renamed
> to CONFIG_TRACE_EVAL_MAP_FILE), I am able to examine the enums that
> have been evaluated:
> 
>  # cat /sys/kernel/debug/tracing/enum_map
> 
> (which will soon be renamed to eval_map)
> 
> And it showed some interesting results:
> 
> [..]
> ZONE_MOVABLE 3 (oom)
> ZONE_NORMAL 2 (oom)
> ZONE_DMA32 1 (oom)
> ZONE_DMA 0 (oom)
> 3 3 (oom)
> 2 2 (oom)
> 1 1 (oom)
> COMPACT_PRIO_ASYNC 2 (oom)
> COMPACT_PRIO_SYNC_LIGHT 1 (oom)
> COMPACT_PRIO_SYNC_FULL 0 (oom)
> [..]
> ZONE_DMA 0 (vmscan)
> 3 3 (vmscan)
> 2 2 (vmscan)
> 1 1 (vmscan)
> COMPACT_PRIO_ASYNC 2 (vmscan)
> [..]
> ZONE_DMA 0 (kmem)
> 3 3 (kmem)
> 2 2 (kmem)
> 1 1 (kmem)
> COMPACT_PRIO_ASYNC 2 (kmem)
> [..]
> ZONE_DMA 0 (compaction)
> 3 3 (compaction)
> 2 2 (compaction)
> 1 1 (compaction)
> COMPACT_PRIO_ASYNC 2 (compaction)
> [..]
> 
> The name within the parenthesis are the trace systems that the
> enum/eval maps are associated with. When there's a number evaluated to
> another number, that tells me that the TRACE_DEFINE_ENUM() was used on
> a #define and not an enum. As #defines get converted normally, they are
> not needed to be evaluated.
> 
> Each of the above trace systems with the number to number evaluation
> included the file include/trace/events/mmflags.h which has:
> 
>  /* High-level compaction status feedback */
>  #define COMPACTION_FAILED       1
>  #define COMPACTION_WITHDRAWN    2
>  #define COMPACTION_PROGRESS     3
> 
> [..]
> 
>  #define COMPACTION_FEEDBACK             \
>         EM(COMPACTION_FAILED,           "failed")       \
>         EM(COMPACTION_WITHDRAWN,        "withdrawn")    \
>         EMe(COMPACTION_PROGRESS,        "progress")
> 
> Which is still needed for the __print_symbolic() usage in the
> trace_event. But it is not needed to be evaluated.
> 
> Removing the evaluation part removes the unnecessary evaluations of
> numbers to numbers.

I will be honest with you. Even if I understood how this voodoo mapping
works I forgot everything. So if the COMPACTION_*->names keeps working
I have no objection to the patch.

> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> diff --git a/include/trace/events/mmflags.h
> b/include/trace/events/mmflags.h index 304ff94..10e3663 100644
> --- a/include/trace/events/mmflags.h
> +++ b/include/trace/events/mmflags.h
> @@ -257,7 +257,7 @@
> IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY,	"softdirty"
> )		\ COMPACTION_STATUS
>  COMPACTION_PRIORITY
> -COMPACTION_FEEDBACK
> +/* COMPACTION_FEEDBACK are defines not enums. Not needed here. */
>  ZONE_TYPE
>  LRU_NAMES
>  

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1666683 — Re: [PATCH] oom, trace: Remove ENUM evaluation of COMPACTION_FEEDBACK

On Thu, 15 Jun 2017 12:57:21 +0200
Michal Hocko <mhocko@kernel.org> wrote:

> >  #define COMPACTION_FEEDBACK             \
> >         EM(COMPACTION_FAILED,           "failed")       \
> >         EM(COMPACTION_WITHDRAWN,        "withdrawn")    \
> >         EMe(COMPACTION_PROGRESS,        "progress")
> > 
> > Which is still needed for the __print_symbolic() usage in the
> > trace_event. But it is not needed to be evaluated.
> > 
> > Removing the evaluation part removes the unnecessary evaluations of
> > numbers to numbers.  
> 
> I will be honest with you. Even if I understood how this voodoo mapping
> works I forgot everything. So if the COMPACTION_*->names keeps working
> I have no objection to the patch.

Yes it does. It you want to verify, simply boot the kernel without the
patch and do:

 # cat /sys/kernel/debug/tracing/events/*/*/format > ~/formats1

Add the patch, compile and boot that kernel.

 # cat /sys/kernel/debug/tracing/events/*/*/format > ~/formats2

then:

 # diff formats1 formats2

and they should be the same, with the exception that the ID:'s can
change.

Also, let me send a v2. Resending this patch (from the private one)
seems to broke the format of it.

-- Steve

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web