diff mbox

[1/7] trace/events: Add gup trace events

Message ID 1449011177-30686-2-git-send-email-yang.shi@linaro.org
State New
Headers show

Commit Message

Yang Shi Dec. 1, 2015, 11:06 p.m. UTC
page-faults events record the invoke to handle_mm_fault, but the invoke
may come from do_page_fault or gup. In some use cases, the finer event count
mey be needed, so add trace events support for:

__get_user_pages
__get_user_pages_fast
fixup_user_fault

Signed-off-by: Yang Shi <yang.shi@linaro.org>

---
 include/trace/events/gup.h | 77 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 include/trace/events/gup.h

-- 
2.0.2

--
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/

Comments

Yang Shi Dec. 2, 2015, 12:07 a.m. UTC | #1
On 12/1/2015 3:56 PM, Steven Rostedt wrote:
> On Tue,  1 Dec 2015 15:06:11 -0800

> Yang Shi <yang.shi@linaro.org> wrote:

>

>> page-faults events record the invoke to handle_mm_fault, but the invoke

>> may come from do_page_fault or gup. In some use cases, the finer event count

>> mey be needed, so add trace events support for:

>>

>> __get_user_pages

>> __get_user_pages_fast

>> fixup_user_fault

>>

>> Signed-off-by: Yang Shi <yang.shi@linaro.org>

>> ---

>>   include/trace/events/gup.h | 77 ++++++++++++++++++++++++++++++++++++++++++++++

>>   1 file changed, 77 insertions(+)

>>   create mode 100644 include/trace/events/gup.h

>>

>> diff --git a/include/trace/events/gup.h b/include/trace/events/gup.h

>> new file mode 100644

>> index 0000000..37d18f9

>> --- /dev/null

>> +++ b/include/trace/events/gup.h

>> @@ -0,0 +1,77 @@

>> +#undef TRACE_SYSTEM

>> +#define TRACE_SYSTEM gup

>> +

>> +#if !defined(_TRACE_GUP_H) || defined(TRACE_HEADER_MULTI_READ)

>> +#define _TRACE_GUP_H

>> +

>> +#include <linux/types.h>

>> +#include <linux/tracepoint.h>

>> +

>> +TRACE_EVENT(gup_fixup_user_fault,

>> +

>> +	TP_PROTO(struct task_struct *tsk, struct mm_struct *mm,

>> +			unsigned long address, unsigned int fault_flags),

>> +

>> +	TP_ARGS(tsk, mm, address, fault_flags),

>> +

>> +	TP_STRUCT__entry(

>> +		__array(	char,	comm,	TASK_COMM_LEN	)

>

> Why save the comm? The tracing infrastructure should keep track of that.


The code is referred to kmem.h which has comm copied. If it is 
unnecessary, it definitely could be removed.

>

>> +		__field(	unsigned long,	address		)

>> +	),

>> +

>> +	TP_fast_assign(

>> +		memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);

>> +		__entry->address	= address;

>> +	),

>> +

>> +	TP_printk("comm=%s address=%lx", __entry->comm, __entry->address)

>> +);

>> +

>> +TRACE_EVENT(gup_get_user_pages,

>> +

>> +	TP_PROTO(struct task_struct *tsk, struct mm_struct *mm,

>> +			unsigned long start, unsigned long nr_pages,

>> +			unsigned int gup_flags, struct page **pages,

>> +			struct vm_area_struct **vmas, int *nonblocking),

>> +

>> +	TP_ARGS(tsk, mm, start, nr_pages, gup_flags, pages, vmas, nonblocking),

>

> Why so many arguments? Most are not used.


My understanding to TP_ARGS may be not right. Doesn't it require all the 
args defined by the function? If not, it could definitely be shrunk. 
Just need keep the args used by TP_printk?

Thanks,
Yang

>

> -- Steve

>

>> +

>> +	TP_STRUCT__entry(

>> +		__array(	char,	comm,	TASK_COMM_LEN	)

>> +		__field(	unsigned long,	start		)

>> +		__field(	unsigned long,	nr_pages	)

>> +	),

>> +

>> +	TP_fast_assign(

>> +		memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);

>> +		__entry->start		= start;

>> +		__entry->nr_pages	= nr_pages;

>> +	),

>> +

>> +	TP_printk("comm=%s start=%lx nr_pages=%lu", __entry->comm, __entry->start, __entry->nr_pages)

>> +);

>> +

>> +TRACE_EVENT(gup_get_user_pages_fast,

>> +

>> +	TP_PROTO(unsigned long start, int nr_pages, int write,

>> +			struct page **pages),

>> +

>> +	TP_ARGS(start, nr_pages, write, pages),

>> +

>> +	TP_STRUCT__entry(

>> +		__field(	unsigned long,	start		)

>> +		__field(	unsigned long,	nr_pages	)

>> +	),

>> +

>> +	TP_fast_assign(

>> +		__entry->start  	= start;

>> +		__entry->nr_pages	= nr_pages;

>> +	),

>> +

>> +	TP_printk("start=%lx nr_pages=%lu",  __entry->start, __entry->nr_pages)

>> +);

>> +

>> +#endif /* _TRACE_GUP_H */

>> +

>> +/* This part must be outside protection */

>> +#include <trace/define_trace.h>

>


--
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/
Yang Shi Dec. 2, 2015, 12:39 a.m. UTC | #2
On 12/1/2015 4:18 PM, Steven Rostedt wrote:
> On Tue, 01 Dec 2015 16:07:44 -0800

> "Shi, Yang" <yang.shi@linaro.org> wrote:

>

>> On 12/1/2015 3:56 PM, Steven Rostedt wrote:

>>> On Tue,  1 Dec 2015 15:06:11 -0800

>>> Yang Shi <yang.shi@linaro.org> wrote:

>>>

>>>> page-faults events record the invoke to handle_mm_fault, but the invoke

>>>> may come from do_page_fault or gup. In some use cases, the finer event count

>>>> mey be needed, so add trace events support for:

>>>>

>>>> __get_user_pages

>>>> __get_user_pages_fast

>>>> fixup_user_fault

>>>>

>>>> Signed-off-by: Yang Shi <yang.shi@linaro.org>

>>>> ---

>>>>    include/trace/events/gup.h | 77 ++++++++++++++++++++++++++++++++++++++++++++++

>>>>    1 file changed, 77 insertions(+)

>>>>    create mode 100644 include/trace/events/gup.h

>>>>

>>>> diff --git a/include/trace/events/gup.h b/include/trace/events/gup.h

>>>> new file mode 100644

>>>> index 0000000..37d18f9

>>>> --- /dev/null

>>>> +++ b/include/trace/events/gup.h

>>>> @@ -0,0 +1,77 @@

>>>> +#undef TRACE_SYSTEM

>>>> +#define TRACE_SYSTEM gup

>>>> +

>>>> +#if !defined(_TRACE_GUP_H) || defined(TRACE_HEADER_MULTI_READ)

>>>> +#define _TRACE_GUP_H

>>>> +

>>>> +#include <linux/types.h>

>>>> +#include <linux/tracepoint.h>

>>>> +

>>>> +TRACE_EVENT(gup_fixup_user_fault,

>>>> +

>>>> +	TP_PROTO(struct task_struct *tsk, struct mm_struct *mm,

>>>> +			unsigned long address, unsigned int fault_flags),

>>>> +

>>>> +	TP_ARGS(tsk, mm, address, fault_flags),

>>>> +

>>>> +	TP_STRUCT__entry(

>>>> +		__array(	char,	comm,	TASK_COMM_LEN	)

>>>

>>> Why save the comm? The tracing infrastructure should keep track of that.

>>

>> The code is referred to kmem.h which has comm copied. If it is

>> unnecessary, it definitely could be removed.

>

> Sometimes comm isn't that reliable. But really, the only tracepoint

> that should record it is sched_switch, and sched_wakeup. With those

> two, the rest of the trace points should be fine.

>

>>

>>>

>>>> +		__field(	unsigned long,	address		)

>>>> +	),

>>>> +

>>>> +	TP_fast_assign(

>>>> +		memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);

>>>> +		__entry->address	= address;

>>>> +	),

>>>> +

>>>> +	TP_printk("comm=%s address=%lx", __entry->comm, __entry->address)

>>>> +);

>>>> +

>>>> +TRACE_EVENT(gup_get_user_pages,

>>>> +

>>>> +	TP_PROTO(struct task_struct *tsk, struct mm_struct *mm,

>>>> +			unsigned long start, unsigned long nr_pages,

>>>> +			unsigned int gup_flags, struct page **pages,

>>>> +			struct vm_area_struct **vmas, int *nonblocking),

>>>> +

>>>> +	TP_ARGS(tsk, mm, start, nr_pages, gup_flags, pages, vmas, nonblocking),

>>>

>>> Why so many arguments? Most are not used.

>>

>> My understanding to TP_ARGS may be not right. Doesn't it require all the

>> args defined by the function? If not, it could definitely be shrunk.

>> Just need keep the args used by TP_printk?

>

> It only needs what is used by TP_fast_assign().


Thanks, will fix them in V2.

Yang

>

> -- 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/
diff mbox

Patch

diff --git a/include/trace/events/gup.h b/include/trace/events/gup.h
new file mode 100644
index 0000000..37d18f9
--- /dev/null
+++ b/include/trace/events/gup.h
@@ -0,0 +1,77 @@ 
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM gup
+
+#if !defined(_TRACE_GUP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_GUP_H
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(gup_fixup_user_fault,
+
+	TP_PROTO(struct task_struct *tsk, struct mm_struct *mm,
+			unsigned long address, unsigned int fault_flags),
+
+	TP_ARGS(tsk, mm, address, fault_flags),
+
+	TP_STRUCT__entry(
+		__array(	char,	comm,	TASK_COMM_LEN	)
+		__field(	unsigned long,	address		)
+	),
+
+	TP_fast_assign(
+		memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);
+		__entry->address	= address;
+	),
+
+	TP_printk("comm=%s address=%lx", __entry->comm, __entry->address)
+);
+
+TRACE_EVENT(gup_get_user_pages,
+
+	TP_PROTO(struct task_struct *tsk, struct mm_struct *mm,
+			unsigned long start, unsigned long nr_pages,
+			unsigned int gup_flags, struct page **pages,
+			struct vm_area_struct **vmas, int *nonblocking),
+
+	TP_ARGS(tsk, mm, start, nr_pages, gup_flags, pages, vmas, nonblocking),
+
+	TP_STRUCT__entry(
+		__array(	char,	comm,	TASK_COMM_LEN	)
+		__field(	unsigned long,	start		)
+		__field(	unsigned long,	nr_pages	)
+	),
+
+	TP_fast_assign(
+		memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);
+		__entry->start		= start;
+		__entry->nr_pages	= nr_pages;
+	),
+
+	TP_printk("comm=%s start=%lx nr_pages=%lu", __entry->comm, __entry->start, __entry->nr_pages)
+);
+
+TRACE_EVENT(gup_get_user_pages_fast,
+
+	TP_PROTO(unsigned long start, int nr_pages, int write,
+			struct page **pages),
+
+	TP_ARGS(start, nr_pages, write, pages),
+
+	TP_STRUCT__entry(
+		__field(	unsigned long,	start		)
+		__field(	unsigned long,	nr_pages	)
+	),
+
+	TP_fast_assign(
+		__entry->start  	= start;
+		__entry->nr_pages	= nr_pages;
+	),
+
+	TP_printk("start=%lx nr_pages=%lu",  __entry->start, __entry->nr_pages)
+);
+
+#endif /* _TRACE_GUP_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>