Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1282691 > unrolled thread
| Started by | Joonsoo Kim <iamjoonsoo.kim@lge.com> |
|---|---|
| First post | 2015-12-03 05:20 +0100 |
| Last post | 2015-12-10 05:30 +0100 |
| Articles | 5 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-12-03 05:20 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2015-12-09 21:10 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-12-10 04:00 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2015-12-10 04:40 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-12-10 05:30 +0100
| From | Joonsoo Kim <iamjoonsoo.kim@lge.com> |
|---|---|
| Date | 2015-12-03 05:20 +0100 |
| Subject | Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation |
| Message-ID | <qBtx7-8sd-7@gated-at.bofh.it> |
On Tue, Nov 24, 2015 at 10:45:28AM +0900, Joonsoo Kim wrote:
> On Mon, Nov 23, 2015 at 09:26:04AM -0500, Steven Rostedt wrote:
> > On Mon, 23 Nov 2015 17:28:05 +0900
> > Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> >
> > > On Fri, Nov 20, 2015 at 11:42:25AM -0500, Steven Rostedt wrote:
> > > > On Fri, 20 Nov 2015 15:33:25 +0900
> > > > Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > > >
> > > >
> > > > > Steven, is it possible to add tracepoint to inlined fucntion such as
> > > > > get_page() in include/linux/mm.h?
> > > >
> > > > I highly recommend against it. The tracepoint code adds a bit of bloat,
> > > > and if you inline it, you add that bloat to every use case. Also, it
> > >
> > > Is it worse than adding function call to my own stub function into
> > > inlined function such as get_page(). I implemented it as following.
> > >
> > > get_page()
> > > {
> > > atomic_inc()
> > > stub_get_page()
> > > }
> > >
> > > stub_get_page() in foo.c
> > > {
> > > trace_page_ref_get_page()
> > > }
> >
> > Now you just slowed down the fast path. But what you could do is:
> >
> > get_page()
> > {
> > atomic_inc();
> > if (trace_page_ref_get_page_enabled())
> > stub_get_page();
> > }
> >
> > Now that "trace_page_ref_get_page_enabled()" will turn into:
> >
> > if (static_key_false(&__tracepoint_page_ref_get_page.key)) {
> >
> > which is a jump label (nop when disabled, a jmp when enabled). That's
> > less bloat but doesn't solve the include problem. You still need to add
> > the include of that will cause havoc with other tracepoints.
>
> Yes, It also has a include dependency problem so I can't use
> trace_page_ref_get_page_enabled() in mm.h. BTW, I tested following
> implementation and it works fine.
>
> extern struct tracepoint __tracepoint_page_ref_get_page;
>
> get_page()
> {
> atomic_inc()
> if (static_key_false(&__tracepoint_page_ref_get_page.key))
> stub_get_page()
> }
>
> This would not slow down fast path although it can't prevent bloat.
> I know that it isn't good code practice, but, this page reference
> handling functions have complex include dependency so I'm not sure
> I can solve it completely. For this special case, can I use
> this raw data structure?
>
Steven, any comment?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2015-12-09 21:10 +0100 |
| Message-ID | <qDTdN-4yA-33@gated-at.bofh.it> |
| In reply to | #1282691 |
On Thu, 3 Dec 2015 13:16:58 +0900
Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> On Tue, Nov 24, 2015 at 10:45:28AM +0900, Joonsoo Kim wrote:
> > On Mon, Nov 23, 2015 at 09:26:04AM -0500, Steven Rostedt wrote:
> > > On Mon, 23 Nov 2015 17:28:05 +0900
> > > Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > >
> > > > On Fri, Nov 20, 2015 at 11:42:25AM -0500, Steven Rostedt wrote:
> > > > > On Fri, 20 Nov 2015 15:33:25 +0900
> > > > > Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > > > >
> > > > >
> > > > > > Steven, is it possible to add tracepoint to inlined fucntion such as
> > > > > > get_page() in include/linux/mm.h?
> > > > >
> > > > > I highly recommend against it. The tracepoint code adds a bit of bloat,
> > > > > and if you inline it, you add that bloat to every use case. Also, it
> > > >
> > > > Is it worse than adding function call to my own stub function into
> > > > inlined function such as get_page(). I implemented it as following.
> > > >
> > > > get_page()
> > > > {
> > > > atomic_inc()
> > > > stub_get_page()
> > > > }
> > > >
> > > > stub_get_page() in foo.c
> > > > {
> > > > trace_page_ref_get_page()
> > > > }
> > >
> > > Now you just slowed down the fast path. But what you could do is:
> > >
> > > get_page()
> > > {
> > > atomic_inc();
> > > if (trace_page_ref_get_page_enabled())
> > > stub_get_page();
> > > }
> > >
> > > Now that "trace_page_ref_get_page_enabled()" will turn into:
> > >
> > > if (static_key_false(&__tracepoint_page_ref_get_page.key)) {
> > >
> > > which is a jump label (nop when disabled, a jmp when enabled). That's
> > > less bloat but doesn't solve the include problem. You still need to add
> > > the include of that will cause havoc with other tracepoints.
> >
> > Yes, It also has a include dependency problem so I can't use
> > trace_page_ref_get_page_enabled() in mm.h. BTW, I tested following
> > implementation and it works fine.
> >
> > extern struct tracepoint __tracepoint_page_ref_get_page;
> >
> > get_page()
> > {
> > atomic_inc()
> > if (static_key_false(&__tracepoint_page_ref_get_page.key))
> > stub_get_page()
> > }
> >
> > This would not slow down fast path although it can't prevent bloat.
> > I know that it isn't good code practice, but, this page reference
> > handling functions have complex include dependency so I'm not sure
> > I can solve it completely. For this special case, can I use
> > this raw data structure?
> >
>
> Steven, any comment?
Sorry for the later reply, I was going to reply but then got called off
to do something else, and then forgot about this message :-/
I wanted you to look at what Andi has done here:
http://lkml.kernel.org/r/1449018060-1742-2-git-send-email-andi@firstfloor.org
and here
http://lkml.kernel.org/r/1449018060-1742-3-git-send-email-andi@firstfloor.org
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joonsoo Kim <iamjoonsoo.kim@lge.com> |
|---|---|
| Date | 2015-12-10 04:00 +0100 |
| Message-ID | <qDZCx-8tE-5@gated-at.bofh.it> |
| In reply to | #1287806 |
On Wed, Dec 09, 2015 at 03:01:54PM -0500, Steven Rostedt wrote:
> On Thu, 3 Dec 2015 13:16:58 +0900
> Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
>
> > On Tue, Nov 24, 2015 at 10:45:28AM +0900, Joonsoo Kim wrote:
> > > On Mon, Nov 23, 2015 at 09:26:04AM -0500, Steven Rostedt wrote:
> > > > On Mon, 23 Nov 2015 17:28:05 +0900
> > > > Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > > >
> > > > > On Fri, Nov 20, 2015 at 11:42:25AM -0500, Steven Rostedt wrote:
> > > > > > On Fri, 20 Nov 2015 15:33:25 +0900
> > > > > > Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > > > > >
> > > > > >
> > > > > > > Steven, is it possible to add tracepoint to inlined fucntion such as
> > > > > > > get_page() in include/linux/mm.h?
> > > > > >
> > > > > > I highly recommend against it. The tracepoint code adds a bit of bloat,
> > > > > > and if you inline it, you add that bloat to every use case. Also, it
> > > > >
> > > > > Is it worse than adding function call to my own stub function into
> > > > > inlined function such as get_page(). I implemented it as following.
> > > > >
> > > > > get_page()
> > > > > {
> > > > > atomic_inc()
> > > > > stub_get_page()
> > > > > }
> > > > >
> > > > > stub_get_page() in foo.c
> > > > > {
> > > > > trace_page_ref_get_page()
> > > > > }
> > > >
> > > > Now you just slowed down the fast path. But what you could do is:
> > > >
> > > > get_page()
> > > > {
> > > > atomic_inc();
> > > > if (trace_page_ref_get_page_enabled())
> > > > stub_get_page();
> > > > }
> > > >
> > > > Now that "trace_page_ref_get_page_enabled()" will turn into:
> > > >
> > > > if (static_key_false(&__tracepoint_page_ref_get_page.key)) {
> > > >
> > > > which is a jump label (nop when disabled, a jmp when enabled). That's
> > > > less bloat but doesn't solve the include problem. You still need to add
> > > > the include of that will cause havoc with other tracepoints.
> > >
> > > Yes, It also has a include dependency problem so I can't use
> > > trace_page_ref_get_page_enabled() in mm.h. BTW, I tested following
> > > implementation and it works fine.
> > >
> > > extern struct tracepoint __tracepoint_page_ref_get_page;
> > >
> > > get_page()
> > > {
> > > atomic_inc()
> > > if (static_key_false(&__tracepoint_page_ref_get_page.key))
> > > stub_get_page()
> > > }
> > >
> > > This would not slow down fast path although it can't prevent bloat.
> > > I know that it isn't good code practice, but, this page reference
> > > handling functions have complex include dependency so I'm not sure
> > > I can solve it completely. For this special case, can I use
> > > this raw data structure?
> > >
> >
> > Steven, any comment?
>
> Sorry for the later reply, I was going to reply but then got called off
> to do something else, and then forgot about this message :-/
No problem. :)
>
> I wanted you to look at what Andi has done here:
>
> http://lkml.kernel.org/r/1449018060-1742-2-git-send-email-andi@firstfloor.org
>
> and here
>
> http://lkml.kernel.org/r/1449018060-1742-3-git-send-email-andi@firstfloor.org
Wow...They look like what I'm looking for. Nice!
Thanks for the pointer!
I have one more question about trace-cmd.
'trace-cmd report' shows time-sorted output even stack trace. See
following example.
trace-cmd-6338 [003] 54.046508: page_ref_mod: ...
trace-cmd-6583 [007] 54.046509: page_ref_mod: ...
trace-cmd-6338 [003] 54.046515: kernel_stack: <stack trace>
=> do_wp_page (ffffffff811a0c6f)
=> handle_mm_fault (ffffffff811a34e2)
=> __do_page_fault (ffffffff810632da)
=> trace_do_page_fault (ffffffff81063633)
=> do_async_page_fault (ffffffff8105c3ea)
=> async_page_fault (ffffffff817733f8)
trace-cmd-6583 [007] 54.046515: kernel_stack: <stack trace>
=> do_wp_page (ffffffff811a0c6f)
=> handle_mm_fault (ffffffff811a34e2)
...
Output of cpu 3, 7 are mixed and it's not easy to analyze it.
I think that it'd be better not to sort stack trace. How do
you think about it? Could you fix it, please?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2015-12-10 04:40 +0100 |
| Message-ID | <qE0ff-AZ-7@gated-at.bofh.it> |
| In reply to | #1288152 |
On Thu, 10 Dec 2015 11:50:15 +0900 Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote: > Output of cpu 3, 7 are mixed and it's not easy to analyze it. > > I think that it'd be better not to sort stack trace. How do > you think about it? Could you fix it, please? It may not be that easy to fix because of the sorting algorithm. That would require looking going ahead one more event each time and then checking if its a stacktrace. I may look at it and see if I can come up with something that's not too invasive in the algorithms. That said, for now you can use the --cpu option. I'm not sure I ever documented it as it was originally added for debugging, but I use it enough that it may be worth while to officially support it. trace-cmd report --cpu 3 Will show you just cpu 3 and nothing else. Which is what I use a lot. But doing the stack trace thing may be something to fix as well. I'll see what I can do, but no guarantees. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joonsoo Kim <iamjoonsoo.kim@lge.com> |
|---|---|
| Date | 2015-12-10 05:30 +0100 |
| Message-ID | <qE11D-17Q-7@gated-at.bofh.it> |
| In reply to | #1288170 |
On Wed, Dec 09, 2015 at 10:36:48PM -0500, Steven Rostedt wrote: > On Thu, 10 Dec 2015 11:50:15 +0900 > Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote: > > > Output of cpu 3, 7 are mixed and it's not easy to analyze it. > > > > I think that it'd be better not to sort stack trace. How do > > you think about it? Could you fix it, please? > > It may not be that easy to fix because of the sorting algorithm. That > would require looking going ahead one more event each time and then > checking if its a stacktrace. I may look at it and see if I can come up > with something that's not too invasive in the algorithms. Okay. > That said, for now you can use the --cpu option. I'm not sure I ever > documented it as it was originally added for debugging, but I use it > enough that it may be worth while to officially support it. > > trace-cmd report --cpu 3 > > Will show you just cpu 3 and nothing else. Which is what I use a lot. Thanks for the input. It works but it's not sufficient to me. Page reference is manipulated by multiple cpus so it's better to analyze unified output. > > But doing the stack trace thing may be something to fix as well. I'll > see what I can do, but no guarantees. Okay. Don't be hurry. :) trace-cmd is excellent and works well for me as it is. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web