[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <4B29B132-6D19-475C-8E08-1232AAE8AD31@gmail.com>
Date: Tue, 21 Jul 2015 23:56:31 +0900
From: Jungseok Lee <jungseoklee85@...il.com>
To: rostedt@...dmis.org, mingo@...hat.com
Cc: linux-kernel@...r.kernel.org,
Byungchul Park <byungchul.park@....com>
Subject: Re: [PATCH 2/2] tracing: Introduce two additional marks for delay
On Jul 11, 2015, at 11:51 PM, Jungseok Lee wrote:
> A fine granulity support for delay would be very useful when profiling
> VM logics, such as page allocation including page reclaim and memory
> compaction with function graph.
>
> Thus, this patch adds two additional marks with two changes.
>
> - An equal sign in mark selection function is removed to align code
> behavior with comments and documentation.
>
> - The function graph example related to delay in ftrace.txt is updated
> to cover all supported marks.
>
> Cc: Byungchul Park <byungchul.park@....com>
> Signed-off-by: Jungseok Lee <jungseoklee85@...il.com>
> ---
> Documentation/trace/ftrace.txt | 51 ++++++++++++++++------
> kernel/trace/trace_output.c | 4 +-
> 2 files changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/trace/ftrace.txt b/Documentation/trace/ftrace.txt
> index 7ddb1e3..072d3c4 100644
> --- a/Documentation/trace/ftrace.txt
> +++ b/Documentation/trace/ftrace.txt
> @@ -686,6 +686,8 @@ The above is mostly meaningful for kernel developers.
> The marks are determined by the difference between this
> current trace and the next trace.
> '$' - greater than 1 second
> + '@' - greater than 100 milisecond
> + '*' - greater than 10 milisecond
> '#' - greater than 1000 microsecond
> '!' - greater than 100 microsecond
> '+' - greater than 10 microsecond
> @@ -1939,26 +1941,49 @@ want, depending on your needs.
>
> ie:
>
> - 0) | up_write() {
> - 0) 0.646 us | _spin_lock_irqsave();
> - 0) 0.684 us | _spin_unlock_irqrestore();
> - 0) 3.123 us | }
> - 0) 0.548 us | fput();
> - 0) + 58.628 us | }
> + 3) # 1837.709 us | } /* __switch_to */
> + 3) | finish_task_switch() {
> + 3) 0.313 us | _raw_spin_unlock_irq();
> + 3) 3.177 us | }
> + 3) # 1889.063 us | } /* __schedule */
> + 3) ! 140.417 us | } /* __schedule */
> + 3) # 2034.948 us | } /* schedule */
> + 3) * 33998.59 us | } /* schedule_preempt_disabled */
>
> [...]
>
> - 0) | putname() {
> - 0) | kmem_cache_free() {
> - 0) 0.518 us | __phys_addr();
> - 0) 1.757 us | }
> - 0) 2.861 us | }
> - 0) ! 115.305 us | }
> - 0) ! 116.402 us | }
> + 1) 0.260 us | msecs_to_jiffies();
> + 1) 0.313 us | __rcu_read_unlock();
> + 1) + 61.770 us | }
> + 1) + 64.479 us | }
> + 1) 0.313 us | rcu_bh_qs();
> + 1) 0.313 us | __local_bh_enable();
> + 1) ! 217.240 us | }
> + 1) 0.365 us | idle_cpu();
> + 1) | rcu_irq_exit() {
> + 1) 0.417 us | rcu_eqs_enter_common.isra.47();
> + 1) 3.125 us | }
> + 1) ! 227.812 us | }
> + 1) ! 457.395 us | }
> + 1) @ 119760.2 us | }
> +
> + [...]
> +
> + 2) | handle_IPI() {
> + 1) 6.979 us | }
> + 2) 0.417 us | scheduler_ipi();
> + 1) 9.791 us | }
> + 1) + 12.917 us | }
> + 2) 3.490 us | }
> + 1) + 15.729 us | }
> + 1) + 18.542 us | }
> + 2) $ 3594274 us | }
>
> + means that the function exceeded 10 usecs.
> ! means that the function exceeded 100 usecs.
> # means that the function exceeded 1000 usecs.
> + * means that the function exceeded 10 msecs.
> + @ means that the function exceeded 100 msecs.
> $ means that the function exceeded 1 sec.
>
>
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index dfab253..8e481a8 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -496,6 +496,8 @@ static const struct trace_mark {
> char sym;
> } mark[] = {
> MARK(1000000000ULL , '$'), /* 1 sec */
> + MARK(100000000ULL , '@'), /* 100 msec */
> + MARK(10000000ULL , '*'), /* 10 msec */
Hi, Steve,
Recently, I've noticed that there is a minor, not logical, flaw in the
above hunk as reviewing patches which do not get any signals yet. It is
a lack of 's' in msec in comment. That is, my latest one is as follows.
+ MARK(100000000ULL , '@'), /* 100 msecs */
+ MARK(10000000ULL , '*'), /* 10 msecs */
I know it's definitely trivial, but I've updated it to align with other
comments and planned to resend the patch. Surprisingly, this patch
has been already staged on your repo. So, I leave a whole patch again
including your SOB since I don't know how to handle this.
Thanks you for reviewing my patch!
Best Regards
Jungseok Lee
----8<-----
>From b431680d4c94a8b54db8b297f8e93329857dc191 Mon Sep 17 00:00:00 2001
From: Jungseok Lee <jungseoklee85@...il.com>
Date: Sat, 11 Jul 2015 14:51:40 +0000
Subject: [PATCH] tracing: Introduce two additional marks for delay
A fine granulity support for delay would be very useful when profiling
VM logics, such as page allocation including page reclaim and memory
compaction, with function graph.
Thus, this patch adds two additional marks with two changes.
- An equal sign in mark selection function is removed to align code
behavior with comments and documentation.
- The function graph example related to delay in ftrace.txt is updated
to cover all supported marks.
Link: http://lkml.kernel.org/r/1436626300-1679-3-git-send-email-jungseoklee85@gmail.com
Cc: Byungchul Park <byungchul.park@....com>
Signed-off-by: Jungseok Lee <jungseoklee85@...il.com>
Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
---
Documentation/trace/ftrace.txt | 51 +++++++++++++++++++++++++++++++-----------
kernel/trace/trace_output.c | 4 +++-
2 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/Documentation/trace/ftrace.txt b/Documentation/trace/ftrace.txt
index 572ca92..eaa8473 100644
--- a/Documentation/trace/ftrace.txt
+++ b/Documentation/trace/ftrace.txt
@@ -686,6 +686,8 @@ The above is mostly meaningful for kernel developers.
The marks are determined by the difference between this
current trace and the next trace.
'$' - greater than 1 second
+ '@' - greater than 100 milisecond
+ '*' - greater than 10 milisecond
'#' - greater than 1000 microsecond
'!' - greater than 100 microsecond
'+' - greater than 10 microsecond
@@ -1939,26 +1941,49 @@ want, depending on your needs.
ie:
- 0) | up_write() {
- 0) 0.646 us | _spin_lock_irqsave();
- 0) 0.684 us | _spin_unlock_irqrestore();
- 0) 3.123 us | }
- 0) 0.548 us | fput();
- 0) + 58.628 us | }
+ 3) # 1837.709 us | } /* __switch_to */
+ 3) | finish_task_switch() {
+ 3) 0.313 us | _raw_spin_unlock_irq();
+ 3) 3.177 us | }
+ 3) # 1889.063 us | } /* __schedule */
+ 3) ! 140.417 us | } /* __schedule */
+ 3) # 2034.948 us | } /* schedule */
+ 3) * 33998.59 us | } /* schedule_preempt_disabled */
[...]
- 0) | putname() {
- 0) | kmem_cache_free() {
- 0) 0.518 us | __phys_addr();
- 0) 1.757 us | }
- 0) 2.861 us | }
- 0) ! 115.305 us | }
- 0) ! 116.402 us | }
+ 1) 0.260 us | msecs_to_jiffies();
+ 1) 0.313 us | __rcu_read_unlock();
+ 1) + 61.770 us | }
+ 1) + 64.479 us | }
+ 1) 0.313 us | rcu_bh_qs();
+ 1) 0.313 us | __local_bh_enable();
+ 1) ! 217.240 us | }
+ 1) 0.365 us | idle_cpu();
+ 1) | rcu_irq_exit() {
+ 1) 0.417 us | rcu_eqs_enter_common.isra.47();
+ 1) 3.125 us | }
+ 1) ! 227.812 us | }
+ 1) ! 457.395 us | }
+ 1) @ 119760.2 us | }
+
+ [...]
+
+ 2) | handle_IPI() {
+ 1) 6.979 us | }
+ 2) 0.417 us | scheduler_ipi();
+ 1) 9.791 us | }
+ 1) + 12.917 us | }
+ 2) 3.490 us | }
+ 1) + 15.729 us | }
+ 1) + 18.542 us | }
+ 2) $ 3594274 us | }
+ means that the function exceeded 10 usecs.
! means that the function exceeded 100 usecs.
# means that the function exceeded 1000 usecs.
+ * means that the function exceeded 10 msecs.
+ @ means that the function exceeded 100 msecs.
$ means that the function exceeded 1 sec.
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 25a086b..46ce046 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -496,6 +496,8 @@ static const struct trace_mark {
char sym;
} mark[] = {
MARK(1000000000ULL , '$'), /* 1 sec */
+ MARK(100000000ULL , '@'), /* 100 msecs */
+ MARK(10000000ULL , '*'), /* 10 msecs */
MARK(1000000ULL , '#'), /* 1000 usecs */
MARK(100000ULL , '!'), /* 100 usecs */
MARK(10000ULL , '+'), /* 10 usecs */
@@ -508,7 +510,7 @@ char trace_find_mark(unsigned long long d)
int size = ARRAY_SIZE(mark);
for (i = 0; i < size; i++) {
- if (d >= mark[i].val)
+ if (d > mark[i].val)
break;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists