Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Shi, Yang]

On 12/3/2015 11:06 AM, Steven Rostedt wrote:

On Thu, 03 Dec 2015 10:36:18 -0800
"Shi, Yang"  wrote:


called directly that calls these functions internally and the tracepoint
can trap the return value.


This will incur more changes in other subsystems (futex, kvm, etc), I'm
not sure if it is worth making such changes to get return value.


No, it wouldn't require any changes outside of this.

-long __get_user_pages(..)
+static long __get_user_pages_internal(..)
{
   [..]
}
+
+long __get_user_pages(..)
+{
+   long ret;
+   ret = __get_user_pages_internal(..);
+   trace_get_user_pages(.., ret)
+}


Thanks for this. I just checked the fast version, it looks it just has 
single return path, so this should be just needed by slow version.







I can probably make function_graph tracer give return values, although
it will give a return value for void functions as well. And it may give
long long returns for int returns that may have bogus data in the
higher bits.


If the return value requirement is not limited to gup, the approach
sounds more reasonable.



Others have asked about it. Maybe I should do it.


If you are going to add return value in common trace code, I won't do 
the gup specific one in V3.


Thanks,
Yang



-- Steve



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Steven Rostedt]

On Thu, 03 Dec 2015 10:36:18 -0800
"Shi, Yang"  wrote:

> > called directly that calls these functions internally and the tracepoint
> > can trap the return value.  
> 
> This will incur more changes in other subsystems (futex, kvm, etc), I'm 
> not sure if it is worth making such changes to get return value.

No, it wouldn't require any changes outside of this.

-long __get_user_pages(..)
+static long __get_user_pages_internal(..)
{
  [..]
}
+
+long __get_user_pages(..)
+{
+   long ret;
+   ret = __get_user_pages_internal(..);
+   trace_get_user_pages(.., ret)
+}

> 
> > I can probably make function_graph tracer give return values, although
> > it will give a return value for void functions as well. And it may give
> > long long returns for int returns that may have bogus data in the
> > higher bits.  
> 
> If the return value requirement is not limited to gup, the approach 
> sounds more reasonable.
>

Others have asked about it. Maybe I should do it.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Shi, Yang]

On 12/2/2015 8:13 PM, Steven Rostedt wrote:

On Wed, 2 Dec 2015 15:36:50 -0800
Dave Hansen  wrote:


On 12/02/2015 02:53 PM, Yang Shi wrote:

diff --git a/mm/gup.c b/mm/gup.c
index deafa2c..10245a4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -13,6 +13,9 @@
  #include 
  #include 

+#define CREATE_TRACE_POINTS
+#include 
+
  #include 
  #include 


This needs to be _the_ last thing that gets #included.  Otherwise, you
risk colliding with any other trace header that gets implicitly included
below.


Agreed.




@@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
nr_pages, int write,
start, len)))
return 0;

+   trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
+
/*
 * Disable interrupts.  We use the nested form as we can already have
 * interrupts disabled by get_futex_key.


It would be _really_ nice to be able to see return values from the
various gup calls as well.  Is that feasible?


Only if you rewrite the functions to have a single return code path
that we can add a tracepoint too. Or have a wrapper function that gets


Yes. My preliminary test just could cover the success case. gup could 
return errno from different a few code path.



called directly that calls these functions internally and the tracepoint
can trap the return value.


This will incur more changes in other subsystems (futex, kvm, etc), I'm 
not sure if it is worth making such changes to get return value.



I can probably make function_graph tracer give return values, although
it will give a return value for void functions as well. And it may give
long long returns for int returns that may have bogus data in the
higher bits.


If the return value requirement is not limited to gup, the approach 
sounds more reasonable.


Thanks,
Yang



-- Steve



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Steven Rostedt]

On Thu, 03 Dec 2015 10:36:18 -0800
"Shi, Yang"  wrote:

> > called directly that calls these functions internally and the tracepoint
> > can trap the return value.  
> 
> This will incur more changes in other subsystems (futex, kvm, etc), I'm 
> not sure if it is worth making such changes to get return value.

No, it wouldn't require any changes outside of this.

-long __get_user_pages(..)
+static long __get_user_pages_internal(..)
{
  [..]
}
+
+long __get_user_pages(..)
+{
+   long ret;
+   ret = __get_user_pages_internal(..);
+   trace_get_user_pages(.., ret)
+}

> 
> > I can probably make function_graph tracer give return values, although
> > it will give a return value for void functions as well. And it may give
> > long long returns for int returns that may have bogus data in the
> > higher bits.  
> 
> If the return value requirement is not limited to gup, the approach 
> sounds more reasonable.
>

Others have asked about it. Maybe I should do it.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Shi, Yang]

On 12/3/2015 11:06 AM, Steven Rostedt wrote:

On Thu, 03 Dec 2015 10:36:18 -0800
"Shi, Yang"  wrote:


called directly that calls these functions internally and the tracepoint
can trap the return value.


This will incur more changes in other subsystems (futex, kvm, etc), I'm
not sure if it is worth making such changes to get return value.


No, it wouldn't require any changes outside of this.

-long __get_user_pages(..)
+static long __get_user_pages_internal(..)
{
   [..]
}
+
+long __get_user_pages(..)
+{
+   long ret;
+   ret = __get_user_pages_internal(..);
+   trace_get_user_pages(.., ret)
+}


Thanks for this. I just checked the fast version, it looks it just has 
single return path, so this should be just needed by slow version.







I can probably make function_graph tracer give return values, although
it will give a return value for void functions as well. And it may give
long long returns for int returns that may have bogus data in the
higher bits.


If the return value requirement is not limited to gup, the approach
sounds more reasonable.



Others have asked about it. Maybe I should do it.


If you are going to add return value in common trace code, I won't do 
the gup specific one in V3.


Thanks,
Yang



-- Steve



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Shi, Yang]

On 12/2/2015 8:13 PM, Steven Rostedt wrote:

On Wed, 2 Dec 2015 15:36:50 -0800
Dave Hansen  wrote:


On 12/02/2015 02:53 PM, Yang Shi wrote:

diff --git a/mm/gup.c b/mm/gup.c
index deafa2c..10245a4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -13,6 +13,9 @@
  #include 
  #include 

+#define CREATE_TRACE_POINTS
+#include 
+
  #include 
  #include 


This needs to be _the_ last thing that gets #included.  Otherwise, you
risk colliding with any other trace header that gets implicitly included
below.


Agreed.




@@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
nr_pages, int write,
start, len)))
return 0;

+   trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
+
/*
 * Disable interrupts.  We use the nested form as we can already have
 * interrupts disabled by get_futex_key.


It would be _really_ nice to be able to see return values from the
various gup calls as well.  Is that feasible?


Only if you rewrite the functions to have a single return code path
that we can add a tracepoint too. Or have a wrapper function that gets


Yes. My preliminary test just could cover the success case. gup could 
return errno from different a few code path.



called directly that calls these functions internally and the tracepoint
can trap the return value.


This will incur more changes in other subsystems (futex, kvm, etc), I'm 
not sure if it is worth making such changes to get return value.



I can probably make function_graph tracer give return values, although
it will give a return value for void functions as well. And it may give
long long returns for int returns that may have bogus data in the
higher bits.


If the return value requirement is not limited to gup, the approach 
sounds more reasonable.


Thanks,
Yang



-- Steve



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Steven Rostedt]

On Wed, 2 Dec 2015 15:36:50 -0800
Dave Hansen  wrote:

> On 12/02/2015 02:53 PM, Yang Shi wrote:
> > diff --git a/mm/gup.c b/mm/gup.c
> > index deafa2c..10245a4 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -13,6 +13,9 @@
> >  #include 
> >  #include 
> >  
> > +#define CREATE_TRACE_POINTS
> > +#include 
> > +
> >  #include 
> >  #include   
> 
> This needs to be _the_ last thing that gets #included.  Otherwise, you
> risk colliding with any other trace header that gets implicitly included
> below.

Agreed.

> 
> > @@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
> > nr_pages, int write,
> > start, len)))
> > return 0;
> >  
> > +   trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
> > +
> > /*
> >  * Disable interrupts.  We use the nested form as we can already have
> >  * interrupts disabled by get_futex_key.  
> 
> It would be _really_ nice to be able to see return values from the
> various gup calls as well.  Is that feasible?

Only if you rewrite the functions to have a single return code path
that we can add a tracepoint too. Or have a wrapper function that gets
called directly that calls these functions internally and the tracepoint
can trap the return value.

I can probably make function_graph tracer give return values, although
it will give a return value for void functions as well. And it may give
long long returns for int returns that may have bogus data in the
higher bits.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Shi, Yang]

On 12/2/2015 4:11 PM, Shi, Yang wrote:

On 12/2/2015 3:36 PM, Dave Hansen wrote:

On 12/02/2015 02:53 PM, Yang Shi wrote:

diff --git a/mm/gup.c b/mm/gup.c
index deafa2c..10245a4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -13,6 +13,9 @@
  #include 
  #include 

+#define CREATE_TRACE_POINTS
+#include 
+
  #include 
  #include 


This needs to be _the_ last thing that gets #included.  Otherwise, you
risk colliding with any other trace header that gets implicitly included
below.


Thanks for the suggestion, will move it to the last.




@@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start,
int nr_pages, int write,
  start, len)))
  return 0;

+trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
+
  /*
   * Disable interrupts.  We use the nested form as we can
already have
   * interrupts disabled by get_futex_key.


It would be _really_ nice to be able to see return values from the
various gup calls as well.  Is that feasible?


I think it should be feasible. kmem_cache_alloc trace event could show
return value. I'm supposed gup trace events should be able to do the
same thing.


Just did a quick test, it is definitely feasible. Please check the below 
test log:


   trace-cmd-200   [000]99.221486: gup_get_user_pages: 
start=800ff0 nr_pages=1 ret=1
   trace-cmd-200   [000]99.223215: gup_get_user_pages: 
start=800fdb nr_pages=1 ret=1
   trace-cmd-200   [000]99.223298: gup_get_user_pages: 
start=800ed0 nr_pages=1 ret=1


nr_pages is the number of pages requested by the gup, ret is the return 
value.


If nobody has objection, I will add it into V3.

Regards,
Yang



Regards,
Yang







--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Shi, Yang]

On 12/2/2015 3:36 PM, Dave Hansen wrote:

On 12/02/2015 02:53 PM, Yang Shi wrote:

diff --git a/mm/gup.c b/mm/gup.c
index deafa2c..10245a4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -13,6 +13,9 @@
  #include 
  #include 

+#define CREATE_TRACE_POINTS
+#include 
+
  #include 
  #include 


This needs to be _the_ last thing that gets #included.  Otherwise, you
risk colliding with any other trace header that gets implicitly included
below.


Thanks for the suggestion, will move it to the last.




@@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
nr_pages, int write,
start, len)))
return 0;

+   trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
+
/*
 * Disable interrupts.  We use the nested form as we can already have
 * interrupts disabled by get_futex_key.


It would be _really_ nice to be able to see return values from the
various gup calls as well.  Is that feasible?


I think it should be feasible. kmem_cache_alloc trace event could show 
return value. I'm supposed gup trace events should be able to do the 
same thing.


Regards,
Yang





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Dave Hansen]

On 12/02/2015 02:53 PM, Yang Shi wrote:
> diff --git a/mm/gup.c b/mm/gup.c
> index deafa2c..10245a4 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -13,6 +13,9 @@
>  #include 
>  #include 
>  
> +#define CREATE_TRACE_POINTS
> +#include 
> +
>  #include 
>  #include 

This needs to be _the_ last thing that gets #included.  Otherwise, you
risk colliding with any other trace header that gets implicitly included
below.

> @@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
> nr_pages, int write,
>   start, len)))
>   return 0;
>  
> + trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
> +
>   /*
>* Disable interrupts.  We use the nested form as we can already have
>* interrupts disabled by get_futex_key.

It would be _really_ nice to be able to see return values from the
various gup calls as well.  Is that feasible?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2 2/7] mm/gup: add gup trace points

[Yang Shi]

For slow version, just add trace point for raw __get_user_pages since all
slow variants call it to do the real work finally.

Signed-off-by: Yang Shi 
---
 mm/gup.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index deafa2c..10245a4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -13,6 +13,9 @@
 #include 
 #include 
 
+#define CREATE_TRACE_POINTS
+#include 
+
 #include 
 #include 
 
@@ -462,6 +465,8 @@ long __get_user_pages(struct task_struct *tsk, struct 
mm_struct *mm,
if (!nr_pages)
return 0;
 
+   trace_gup_get_user_pages(tsk, mm, start, nr_pages);
+
VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
 
/*
@@ -599,6 +604,7 @@ int fixup_user_fault(struct task_struct *tsk, struct 
mm_struct *mm,
if (!(vm_flags & vma->vm_flags))
return -EFAULT;
 
+   trace_gup_fixup_user_fault(tsk, mm, address, fault_flags);
ret = handle_mm_fault(mm, vma, address, fault_flags);
if (ret & VM_FAULT_ERROR) {
if (ret & VM_FAULT_OOM)
@@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
nr_pages, int write,
start, len)))
return 0;
 
+   trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
+
/*
 * Disable interrupts.  We use the nested form as we can already have
 * interrupts disabled by get_futex_key.
-- 
2.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Shi, Yang]

On 12/2/2015 4:11 PM, Shi, Yang wrote:

On 12/2/2015 3:36 PM, Dave Hansen wrote:

On 12/02/2015 02:53 PM, Yang Shi wrote:

diff --git a/mm/gup.c b/mm/gup.c
index deafa2c..10245a4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -13,6 +13,9 @@
  #include 
  #include 

+#define CREATE_TRACE_POINTS
+#include 
+
  #include 
  #include 


This needs to be _the_ last thing that gets #included.  Otherwise, you
risk colliding with any other trace header that gets implicitly included
below.


Thanks for the suggestion, will move it to the last.




@@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start,
int nr_pages, int write,
  start, len)))
  return 0;

+trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
+
  /*
   * Disable interrupts.  We use the nested form as we can
already have
   * interrupts disabled by get_futex_key.


It would be _really_ nice to be able to see return values from the
various gup calls as well.  Is that feasible?


I think it should be feasible. kmem_cache_alloc trace event could show
return value. I'm supposed gup trace events should be able to do the
same thing.


Just did a quick test, it is definitely feasible. Please check the below 
test log:


   trace-cmd-200   [000]99.221486: gup_get_user_pages: 
start=800ff0 nr_pages=1 ret=1
   trace-cmd-200   [000]99.223215: gup_get_user_pages: 
start=800fdb nr_pages=1 ret=1
   trace-cmd-200   [000]99.223298: gup_get_user_pages: 
start=800ed0 nr_pages=1 ret=1


nr_pages is the number of pages requested by the gup, ret is the return 
value.


If nobody has objection, I will add it into V3.

Regards,
Yang



Regards,
Yang







--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Steven Rostedt]

On Wed, 2 Dec 2015 15:36:50 -0800
Dave Hansen  wrote:

> On 12/02/2015 02:53 PM, Yang Shi wrote:
> > diff --git a/mm/gup.c b/mm/gup.c
> > index deafa2c..10245a4 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -13,6 +13,9 @@
> >  #include 
> >  #include 
> >  
> > +#define CREATE_TRACE_POINTS
> > +#include 
> > +
> >  #include 
> >  #include   
> 
> This needs to be _the_ last thing that gets #included.  Otherwise, you
> risk colliding with any other trace header that gets implicitly included
> below.

Agreed.

> 
> > @@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
> > nr_pages, int write,
> > start, len)))
> > return 0;
> >  
> > +   trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
> > +
> > /*
> >  * Disable interrupts.  We use the nested form as we can already have
> >  * interrupts disabled by get_futex_key.  
> 
> It would be _really_ nice to be able to see return values from the
> various gup calls as well.  Is that feasible?

Only if you rewrite the functions to have a single return code path
that we can add a tracepoint too. Or have a wrapper function that gets
called directly that calls these functions internally and the tracepoint
can trap the return value.

I can probably make function_graph tracer give return values, although
it will give a return value for void functions as well. And it may give
long long returns for int returns that may have bogus data in the
higher bits.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2 2/7] mm/gup: add gup trace points

[Yang Shi]

For slow version, just add trace point for raw __get_user_pages since all
slow variants call it to do the real work finally.

Signed-off-by: Yang Shi 
---
 mm/gup.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index deafa2c..10245a4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -13,6 +13,9 @@
 #include 
 #include 
 
+#define CREATE_TRACE_POINTS
+#include 
+
 #include 
 #include 
 
@@ -462,6 +465,8 @@ long __get_user_pages(struct task_struct *tsk, struct 
mm_struct *mm,
if (!nr_pages)
return 0;
 
+   trace_gup_get_user_pages(tsk, mm, start, nr_pages);
+
VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
 
/*
@@ -599,6 +604,7 @@ int fixup_user_fault(struct task_struct *tsk, struct 
mm_struct *mm,
if (!(vm_flags & vma->vm_flags))
return -EFAULT;
 
+   trace_gup_fixup_user_fault(tsk, mm, address, fault_flags);
ret = handle_mm_fault(mm, vma, address, fault_flags);
if (ret & VM_FAULT_ERROR) {
if (ret & VM_FAULT_OOM)
@@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
nr_pages, int write,
start, len)))
return 0;
 
+   trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
+
/*
 * Disable interrupts.  We use the nested form as we can already have
 * interrupts disabled by get_futex_key.
-- 
2.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Shi, Yang]

On 12/2/2015 3:36 PM, Dave Hansen wrote:

On 12/02/2015 02:53 PM, Yang Shi wrote:

diff --git a/mm/gup.c b/mm/gup.c
index deafa2c..10245a4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -13,6 +13,9 @@
  #include 
  #include 

+#define CREATE_TRACE_POINTS
+#include 
+
  #include 
  #include 


This needs to be _the_ last thing that gets #included.  Otherwise, you
risk colliding with any other trace header that gets implicitly included
below.


Thanks for the suggestion, will move it to the last.




@@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
nr_pages, int write,
start, len)))
return 0;

+   trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
+
/*
 * Disable interrupts.  We use the nested form as we can already have
 * interrupts disabled by get_futex_key.


It would be _really_ nice to be able to see return values from the
various gup calls as well.  Is that feasible?


I think it should be feasible. kmem_cache_alloc trace event could show 
return value. I'm supposed gup trace events should be able to do the 
same thing.


Regards,
Yang





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2 2/7] mm/gup: add gup trace points

[Dave Hansen]

On 12/02/2015 02:53 PM, Yang Shi wrote:
> diff --git a/mm/gup.c b/mm/gup.c
> index deafa2c..10245a4 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -13,6 +13,9 @@
>  #include 
>  #include 
>  
> +#define CREATE_TRACE_POINTS
> +#include 
> +
>  #include 
>  #include 

This needs to be _the_ last thing that gets #included.  Otherwise, you
risk colliding with any other trace header that gets implicitly included
below.

> @@ -1340,6 +1346,8 @@ int __get_user_pages_fast(unsigned long start, int 
> nr_pages, int write,
>   start, len)))
>   return 0;
>  
> + trace_gup_get_user_pages_fast(start, nr_pages, write, pages);
> +
>   /*
>* Disable interrupts.  We use the nested form as we can already have
>* interrupts disabled by get_futex_key.

It would be _really_ nice to be able to see return values from the
various gup calls as well.  Is that feasible?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/