Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1272302 > unrolled thread
| Started by | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| First post | 2015-11-18 16:40 +0100 |
| Last post | 2015-11-24 03:20 +0100 |
| Articles | 8 — 4 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 Vlastimil Babka <vbabka@suse.cz> - 2015-11-18 16:40 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Minchan Kim <minchan@kernel.org> - 2015-11-19 08:00 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-11-20 07:40 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2015-11-20 17:50 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-11-23 09:30 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2015-11-23 15:30 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-11-24 03:00 +0100
Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-11-24 03:20 +0100
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2015-11-18 16:40 +0100 |
| Subject | Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation |
| Message-ID | <qwcZY-4Lu-23@gated-at.bofh.it> |
On 11/09/2015 08:23 AM, Joonsoo Kim wrote:
> CMA allocation should be guaranteed to succeed by definition, but,
> unfortunately, it would be failed sometimes. It is hard to track down
> the problem, because it is related to page reference manipulation and
> we don't have any facility to analyze it.
Reminds me of the PeterZ's VM_PINNED patchset. What happened to it?
https://lwn.net/Articles/600502/
> This patch adds tracepoints to track down page reference manipulation.
> With it, we can find exact reason of failure and can fix the problem.
> Following is an example of tracepoint output.
>
> <...>-9018 [004] 92.678375: page_ref_set: pfn=0x17ac9 flags=0x0 count=1 mapcount=0 mapping=(nil) mt=4 val=1
> <...>-9018 [004] 92.678378: kernel_stack:
> => get_page_from_freelist (ffffffff81176659)
> => __alloc_pages_nodemask (ffffffff81176d22)
> => alloc_pages_vma (ffffffff811bf675)
> => handle_mm_fault (ffffffff8119e693)
> => __do_page_fault (ffffffff810631ea)
> => trace_do_page_fault (ffffffff81063543)
> => do_async_page_fault (ffffffff8105c40a)
> => async_page_fault (ffffffff817581d8)
> [snip]
> <...>-9018 [004] 92.678379: page_ref_mod: pfn=0x17ac9 flags=0x40048 count=2 mapcount=1 mapping=0xffff880015a78dc1 mt=4 val=1
> [snip]
> ...
> ...
> <...>-9131 [001] 93.174468: test_pages_isolated: start_pfn=0x17800 end_pfn=0x17c00 fin_pfn=0x17ac9 ret=fail
> [snip]
> <...>-9018 [004] 93.174843: page_ref_mod_and_test: pfn=0x17ac9 flags=0x40068 count=0 mapcount=0 mapping=0xffff880015a78dc1 mt=4 val=-1 ret=1
> => release_pages (ffffffff8117c9e4)
> => free_pages_and_swap_cache (ffffffff811b0697)
> => tlb_flush_mmu_free (ffffffff81199616)
> => tlb_finish_mmu (ffffffff8119a62c)
> => exit_mmap (ffffffff811a53f7)
> => mmput (ffffffff81073f47)
> => do_exit (ffffffff810794e9)
> => do_group_exit (ffffffff81079def)
> => SyS_exit_group (ffffffff81079e74)
> => entry_SYSCALL_64_fastpath (ffffffff817560b6)
>
> This output shows that problem comes from exit path. In exit path,
> to improve performance, pages are not freed immediately. They are gathered
> and processed by batch. During this process, migration cannot be possible
> and CMA allocation is failed. This problem is hard to find without this
> page reference tracepoint facility.
Yeah but when you realized it was this problem, what was the fix? Probably not
remove batching from exit path? Shouldn't CMA in this case just try waiting for
the pins to go away, which would eventually happen? And for long-term pins,
VM_PINNED would make sure the pages are migrated away from CMA pageblocks first?
So I'm worried that this is quite nontrivial change for a very specific usecase.
> Enabling this feature bloat kernel text 20 KB in my configuration.
It's not just that, see below.
[...]
> static inline int page_ref_freeze(struct page *page, int count)
> {
> - return likely(atomic_cmpxchg(&page->_count, count, 0) == count);
> + int ret = likely(atomic_cmpxchg(&page->_count, count, 0) == count);
The "likely" mean makes no sense anymore, doe it?
> diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
> index 957d3da..71d2399 100644
> --- a/mm/Kconfig.debug
> +++ b/mm/Kconfig.debug
> @@ -28,3 +28,7 @@ config DEBUG_PAGEALLOC
>
> config PAGE_POISONING
> bool
> +
> +config DEBUG_PAGE_REF
> + bool "Enable tracepoint to track down page reference manipulation"
So you should probably state the costs. Which is the extra memory, and also that
all the page ref manipulations are now turned to function calls, even if the
tracepoints are disabled. Patch 1 didn't change that many callsites, so maybe it
would be feasible to have the tracepoints inline, where being disabled has
near-zero overhead?
--
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 | Minchan Kim <minchan@kernel.org> |
|---|---|
| Date | 2015-11-19 08:00 +0100 |
| Message-ID | <qwrmh-5OP-1@gated-at.bofh.it> |
| In reply to | #1272302 |
On Wed, Nov 18, 2015 at 04:34:30PM +0100, Vlastimil Babka wrote: > On 11/09/2015 08:23 AM, Joonsoo Kim wrote: > > CMA allocation should be guaranteed to succeed by definition, but, > > unfortunately, it would be failed sometimes. It is hard to track down > > the problem, because it is related to page reference manipulation and > > we don't have any facility to analyze it. > > Reminds me of the PeterZ's VM_PINNED patchset. What happened to it? > https://lwn.net/Articles/600502/ > > > This patch adds tracepoints to track down page reference manipulation. > > With it, we can find exact reason of failure and can fix the problem. > > Following is an example of tracepoint output. > > > > <...>-9018 [004] 92.678375: page_ref_set: pfn=0x17ac9 flags=0x0 count=1 mapcount=0 mapping=(nil) mt=4 val=1 > > <...>-9018 [004] 92.678378: kernel_stack: > > => get_page_from_freelist (ffffffff81176659) > > => __alloc_pages_nodemask (ffffffff81176d22) > > => alloc_pages_vma (ffffffff811bf675) > > => handle_mm_fault (ffffffff8119e693) > > => __do_page_fault (ffffffff810631ea) > > => trace_do_page_fault (ffffffff81063543) > > => do_async_page_fault (ffffffff8105c40a) > > => async_page_fault (ffffffff817581d8) > > [snip] > > <...>-9018 [004] 92.678379: page_ref_mod: pfn=0x17ac9 flags=0x40048 count=2 mapcount=1 mapping=0xffff880015a78dc1 mt=4 val=1 > > [snip] > > ... > > ... > > <...>-9131 [001] 93.174468: test_pages_isolated: start_pfn=0x17800 end_pfn=0x17c00 fin_pfn=0x17ac9 ret=fail > > [snip] > > <...>-9018 [004] 93.174843: page_ref_mod_and_test: pfn=0x17ac9 flags=0x40068 count=0 mapcount=0 mapping=0xffff880015a78dc1 mt=4 val=-1 ret=1 > > => release_pages (ffffffff8117c9e4) > > => free_pages_and_swap_cache (ffffffff811b0697) > > => tlb_flush_mmu_free (ffffffff81199616) > > => tlb_finish_mmu (ffffffff8119a62c) > > => exit_mmap (ffffffff811a53f7) > > => mmput (ffffffff81073f47) > > => do_exit (ffffffff810794e9) > > => do_group_exit (ffffffff81079def) > > => SyS_exit_group (ffffffff81079e74) > > => entry_SYSCALL_64_fastpath (ffffffff817560b6) > > > > This output shows that problem comes from exit path. In exit path, > > to improve performance, pages are not freed immediately. They are gathered > > and processed by batch. During this process, migration cannot be possible > > and CMA allocation is failed. This problem is hard to find without this > > page reference tracepoint facility. > > Yeah but when you realized it was this problem, what was the fix? Probably not > remove batching from exit path? Shouldn't CMA in this case just try waiting for > the pins to go away, which would eventually happen? And for long-term pins, > VM_PINNED would make sure the pages are migrated away from CMA pageblocks first? > > So I'm worried that this is quite nontrivial change for a very specific usecase. This patch is not to solve the problem but just to expose what is culprit. For using VM_PINNED, firstly, we should know where are long-term pins. There are obviously clear places which will be first target if we use VM_PINNED but somewhere are vague. For vague places, this patch will help finding there. Even we don't use VM_PINNED, this patch will expose current obstacle which can help to understand current problems. -- 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-11-20 07:40 +0100 |
| Message-ID | <qwNwu-3zS-1@gated-at.bofh.it> |
| In reply to | #1272302 |
Ccing Steven.
Hello,
On Wed, Nov 18, 2015 at 04:34:30PM +0100, Vlastimil Babka wrote:
> On 11/09/2015 08:23 AM, Joonsoo Kim wrote:
> > CMA allocation should be guaranteed to succeed by definition, but,
> > unfortunately, it would be failed sometimes. It is hard to track down
> > the problem, because it is related to page reference manipulation and
> > we don't have any facility to analyze it.
>
> Reminds me of the PeterZ's VM_PINNED patchset. What happened to it?
> https://lwn.net/Articles/600502/
>
> > This patch adds tracepoints to track down page reference manipulation.
> > With it, we can find exact reason of failure and can fix the problem.
> > Following is an example of tracepoint output.
> >
> > <...>-9018 [004] 92.678375: page_ref_set: pfn=0x17ac9 flags=0x0 count=1 mapcount=0 mapping=(nil) mt=4 val=1
> > <...>-9018 [004] 92.678378: kernel_stack:
> > => get_page_from_freelist (ffffffff81176659)
> > => __alloc_pages_nodemask (ffffffff81176d22)
> > => alloc_pages_vma (ffffffff811bf675)
> > => handle_mm_fault (ffffffff8119e693)
> > => __do_page_fault (ffffffff810631ea)
> > => trace_do_page_fault (ffffffff81063543)
> > => do_async_page_fault (ffffffff8105c40a)
> > => async_page_fault (ffffffff817581d8)
> > [snip]
> > <...>-9018 [004] 92.678379: page_ref_mod: pfn=0x17ac9 flags=0x40048 count=2 mapcount=1 mapping=0xffff880015a78dc1 mt=4 val=1
> > [snip]
> > ...
> > ...
> > <...>-9131 [001] 93.174468: test_pages_isolated: start_pfn=0x17800 end_pfn=0x17c00 fin_pfn=0x17ac9 ret=fail
> > [snip]
> > <...>-9018 [004] 93.174843: page_ref_mod_and_test: pfn=0x17ac9 flags=0x40068 count=0 mapcount=0 mapping=0xffff880015a78dc1 mt=4 val=-1 ret=1
> > => release_pages (ffffffff8117c9e4)
> > => free_pages_and_swap_cache (ffffffff811b0697)
> > => tlb_flush_mmu_free (ffffffff81199616)
> > => tlb_finish_mmu (ffffffff8119a62c)
> > => exit_mmap (ffffffff811a53f7)
> > => mmput (ffffffff81073f47)
> > => do_exit (ffffffff810794e9)
> > => do_group_exit (ffffffff81079def)
> > => SyS_exit_group (ffffffff81079e74)
> > => entry_SYSCALL_64_fastpath (ffffffff817560b6)
> >
> > This output shows that problem comes from exit path. In exit path,
> > to improve performance, pages are not freed immediately. They are gathered
> > and processed by batch. During this process, migration cannot be possible
> > and CMA allocation is failed. This problem is hard to find without this
> > page reference tracepoint facility.
>
> Yeah but when you realized it was this problem, what was the fix? Probably not
> remove batching from exit path? Shouldn't CMA in this case just try waiting for
> the pins to go away, which would eventually happen? And for long-term pins,
> VM_PINNED would make sure the pages are migrated away from CMA pageblocks first?
>
> So I'm worried that this is quite nontrivial change for a very specific usecase.
Minchan already answered it. Thanks, Minchan.
This also can be used for memory-offlining and compaction.
> > Enabling this feature bloat kernel text 20 KB in my configuration.
>
> It's not just that, see below.
>
> [...]
>
>
> > static inline int page_ref_freeze(struct page *page, int count)
> > {
> > - return likely(atomic_cmpxchg(&page->_count, count, 0) == count);
> > + int ret = likely(atomic_cmpxchg(&page->_count, count, 0) == count);
>
> The "likely" mean makes no sense anymore, doe it?
I don't know how compiler uses this hint exactly but it will have same
effect as before. Why do you think it makes no sense now?
>
> > diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
> > index 957d3da..71d2399 100644
> > --- a/mm/Kconfig.debug
> > +++ b/mm/Kconfig.debug
> > @@ -28,3 +28,7 @@ config DEBUG_PAGEALLOC
> >
> > config PAGE_POISONING
> > bool
> > +
> > +config DEBUG_PAGE_REF
> > + bool "Enable tracepoint to track down page reference manipulation"
>
> So you should probably state the costs. Which is the extra memory, and also that
> all the page ref manipulations are now turned to function calls, even if the
> tracepoints are disabled.
Yes, will do.
> Patch 1 didn't change that many callsites, so maybe it
> would be feasible to have the tracepoints inline, where being disabled has
> near-zero overhead?
Hmm...Although I changed just one get_page() in previous patch, it would
be used in many places. So, it's not feasible to inline tracepoints to
each callsites directly.
In fact, I tried to inline tracepoint into wrapper function directly,
but, it fails due to (maybe) header file dependency.
Steven, is it possible to add tracepoint to inlined fucntion such as
get_page() in include/linux/mm.h?
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-11-20 17:50 +0100 |
| Message-ID | <qwX2P-1hA-41@gated-at.bofh.it> |
| In reply to | #1273748 |
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 makes things difficult if this file is included in other files that create tracepoints, which I could easily imagine would be the case. That is, if a tracepoint file in include/trace/events/foo.h needs to include include/linux/mm.h, when you do CREATE_TRACEPOINTS for foo.h, it will create tracepoints for mm.h as to use tracepoints there you would need to include the include/trace/events/mm.h (or whatever its name is), and that has caused issues in the past. Now, if you still want to have these tracepoints in the inlined function, it would be best to add a new file mm_trace.h? or something that would include it, and then have only the .c files include that directly. Do not put it into mm.h as that would definitely cause tracepoint include troubles. -- 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-11-23 09:30 +0100 |
| Message-ID | <qxUFA-7Ir-19@gated-at.bofh.it> |
| In reply to | #1274255 |
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()
}
> makes things difficult if this file is included in other files that
> create tracepoints, which I could easily imagine would be the case.
> That is, if a tracepoint file in include/trace/events/foo.h needs to
> include include/linux/mm.h, when you do CREATE_TRACEPOINTS for foo.h,
> it will create tracepoints for mm.h as to use tracepoints there you
> would need to include the include/trace/events/mm.h (or whatever its
> name is), and that has caused issues in the past.
>
> Now, if you still want to have these tracepoints in the inlined
> function, it would be best to add a new file mm_trace.h? or something
> that would include it, and then have only the .c files include that
> directly. Do not put it into mm.h as that would definitely cause
> tracepoint include troubles.
Okay. If I choose this way, I have to change too many places and churn
the code. If bloat of my implementation is similar with this suggestion,
I prefer my implementation.
Thanks for good advice.
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-11-23 15:30 +0100 |
| Message-ID | <qy0hX-2Wb-7@gated-at.bofh.it> |
| In reply to | #1275117 |
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.
-- 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-11-24 03:00 +0100 |
| Message-ID | <qyb3H-1sG-9@gated-at.bofh.it> |
| In reply to | #1275426 |
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 include dependency problem so I can't use
trace_page_ref_get_page_enabled() in mm.h.
BTW, I try to open code trace_page_ref_get_page_enabled() in
get_page() as following 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()
}
I know that it's not good coding practice to use raw data structure,
but, page reference management functions has complex dependency
so I'm not sure I can solve it completely. For this special case, can
I use it?
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 | Joonsoo Kim <iamjoonsoo.kim@lge.com> |
|---|---|
| Date | 2015-11-24 03:20 +0100 |
| Message-ID | <qybn5-1PG-9@gated-at.bofh.it> |
| In reply to | #1275426 |
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?
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