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


Groups > linux.kernel > #1272302 > unrolled thread

Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

Started byVlastimil Babka <vbabka@suse.cz>
First post2015-11-18 16:40 +0100
Last post2015-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.


Contents

  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

#1272302 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

FromVlastimil Babka <vbabka@suse.cz>
Date2015-11-18 16:40 +0100
SubjectRe: [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]


#1272886

FromMinchan Kim <minchan@kernel.org>
Date2015-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]


#1273748

FromJoonsoo Kim <iamjoonsoo.kim@lge.com>
Date2015-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]


#1274255

FromSteven Rostedt <rostedt@goodmis.org>
Date2015-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]


#1275117

FromJoonsoo Kim <iamjoonsoo.kim@lge.com>
Date2015-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]


#1275426

FromSteven Rostedt <rostedt@goodmis.org>
Date2015-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]


#1276014

FromJoonsoo Kim <iamjoonsoo.kim@lge.com>
Date2015-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]


#1276017

FromJoonsoo Kim <iamjoonsoo.kim@lge.com>
Date2015-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