All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-12  8:21 ` yalin wang
  0 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-12  8:21 UTC (permalink / raw)
  To: rostedt, mingo, akpm, ebru.akagunduz, riel, kirill.shutemov,
	vbabka, yalin.wang2010, jmarchan, mgorman, willy, linux-kernel,
	linux-mm

This crash is caused by NULL pointer deference, in page_to_pfn() marco,
when page == NULL :

[  182.639154 ] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[  182.639491 ] pgd = ffffffc00077a000
[  182.639761 ] [00000000] *pgd=00000000b9422003, *pud=00000000b9422003, *pmd=00000000b9423003, *pte=0060000008000707
[  182.640749 ] Internal error: Oops: 94000006 [#1] SMP
[  182.641197 ] Modules linked in:
[  182.641580 ] CPU: 1 PID: 26 Comm: khugepaged Tainted: G        W       4.3.0-rc6-next-20151022ajb-00001-g32f3386-dirty #3
[  182.642077 ] Hardware name: linux,dummy-virt (DT)
[  182.642227 ] task: ffffffc07957c080 ti: ffffffc079638000 task.ti: ffffffc079638000
[  182.642598 ] PC is at khugepaged+0x378/0x1af8
[  182.642826 ] LR is at khugepaged+0x418/0x1af8
[  182.643047 ] pc : [<ffffffc0001980ac>] lr : [<ffffffc00019814c>] pstate: 60000145
[  182.643490 ] sp : ffffffc07963bca0
[  182.643650 ] x29: ffffffc07963bca0 x28: ffffffc00075c000
[  182.644024 ] x27: ffffffc00f275040 x26: ffffffc0006c7000
[  182.644334 ] x25: 00e8000048800f51 x24: 0000000006400000
[  182.644687 ] x23: 0000000000000002 x22: 0000000000000000
[  182.644972 ] x21: 0000000000000000 x20: 0000000000000000
[  182.645446 ] x19: 0000000000000000 x18: 0000007ff86d0990
[  182.645931 ] x17: 00000000007ef9c8 x16: ffffffc000098390
[  182.646236 ] x15: ffffffffffffffff x14: 00000000ffffffff
[  182.646649 ] x13: 000000000000016a x12: 0000000000000000
[  182.647046 ] x11: ffffffc07f025020 x10: 0000000000000000
[  182.647395 ] x9 : 0000000000000048 x8 : ffffffc000721e28
[  182.647872 ] x7 : 0000000000000000 x6 : ffffffc07f02d000
[  182.648261 ] x5 : fffffffffffffe00 x4 : ffffffc00f275040
[  182.648611 ] x3 : 0000000000000000 x2 : ffffffc00f2ad000
[  182.648908 ] x1 : 0000000000000000 x0 : ffffffc000727000
[  182.649147 ]
[  182.649252 ] Process khugepaged (pid: 26, stack limit = 0xffffffc079638020)
[  182.649724 ] Stack: (0xffffffc07963bca0 to 0xffffffc07963c000)
[  182.650141 ] bca0: ffffffc07963be30 ffffffc0000b5044 ffffffc07961fb80 ffffffc00072e630
[  182.650587 ] bcc0: ffffffc0005d5090 0000000000000000 ffffffc000197d34 0000000000000000
[  182.651009 ] bce0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.651446 ] bd00: ffffffc07963bd90 ffffffc07f1cbf80 000000004f3be003 ffffffc00f2750a4
[  182.651956 ] bd20: ffffffc00f3bf000 ffffffc000000001 0000000000000001 ffffffc07f085740
[  182.652520 ] bd40: ffffffc00f2ad188 ffffffc000000000 0000000006200000 ffffffc00f275040
[  182.652972 ] bd60: ffffffc0006b1a90 ffffffc079638000 ffffffc07963be20 ffffffc00f0144d0
[  182.653357 ] bd80: ffffffc000000000 0000000006400000 ffffffc00f0144d0 00000a0800000001
[  182.653793 ] bda0: 0000100000000001 ffffffc000000001 ffffffc07f025000 ffffffc00f2750a8
[  182.654226 ] bdc0: 00000001000005f8 ffffffc00075a000 0000000006a00000 ffffffc000727000
[  182.654522 ] bde0: ffffffc0006e8478 ffffffc000000000 0000000100000000 ffffffc078fb9000
[  182.654869 ] be00: ffffffc07963be30 ffffffc000000000 ffffffc07957c080 ffffffc0000cfc4c
[  182.655225 ] be20: ffffffc07963be20 ffffffc07963be20 0000000000000000 ffffffc000085c50
[  182.655588 ] be40: ffffffc0000b4f64 ffffffc07961fb80 0000000000000000 0000000000000000
[  182.656138 ] be60: 0000000000000000 ffffffc0000bee2c ffffffc0000b4f64 0000000000000000
[  182.656609 ] be80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.657145 ] bea0: ffffffc07963bea0 ffffffc07963bea0 0000000000000000 ffffffc000000000
[  182.657475 ] bec0: ffffffc07963bec0 ffffffc07963bec0 0000000000000000 0000000000000000
[  182.657922 ] bee0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.658558 ] bf00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.658972 ] bf20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.659291 ] bf40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.659722 ] bf60: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.660122 ] bf80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.660654 ] bfa0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.661064 ] bfc0: 0000000000000000 0000000000000000 0000000000000000 0000000000000005
[  182.661466 ] bfe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.661848 ] Call trace:
[  182.662050 ] [<ffffffc0001980ac>] khugepaged+0x378/0x1af8
[  182.662294 ] [<ffffffc0000b5040>] kthread+0xdc/0xf4
[  182.662605 ] [<ffffffc000085c4c>] ret_from_fork+0xc/0x40
[  182.663046 ] Code: 35001700 f0002c60 aa0703e3 f9009fa0 (f94000e0)
[  182.663901 ] ---[ end trace 637503d8e28ae69e  ]---
[  182.664160 ] Kernel panic - not syncing: Fatal exception
[  182.664571 ] CPU2: stopping
[  182.664794 ] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G      D W       4.3.0-rc6-next-20151022ajb-00001-g32f3386-dirty #3
[  182.665248 ] Hardware name: linux,dummy-virt (DT)

add the trace point with TP_CONDITION(page),
avoid trace NULL page.

Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
---
 include/trace/events/huge_memory.h | 20 ++++++++++++--------
 mm/huge_memory.c                   |  6 +++---
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index 11c59ca..727647b 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -45,12 +45,14 @@ SCAN_STATUS
 #define EM(a, b)	{a, b},
 #define EMe(a, b)	{a, b}
 
-TRACE_EVENT(mm_khugepaged_scan_pmd,
+TRACE_EVENT_CONDITION(mm_khugepaged_scan_pmd,
 
-	TP_PROTO(struct mm_struct *mm, unsigned long pfn, bool writable,
+	TP_PROTO(struct mm_struct *mm, struct page *page, bool writable,
 		 bool referenced, int none_or_zero, int status, int unmapped),
 
-	TP_ARGS(mm, pfn, writable, referenced, none_or_zero, status, unmapped),
+	TP_ARGS(mm, page, writable, referenced, none_or_zero, status, unmapped),
+
+	TP_CONDITION(page),
 
 	TP_STRUCT__entry(
 		__field(struct mm_struct *, mm)
@@ -64,7 +66,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
 
 	TP_fast_assign(
 		__entry->mm = mm;
-		__entry->pfn = pfn;
+		__entry->pfn = page_to_pfn(page);
 		__entry->writable = writable;
 		__entry->referenced = referenced;
 		__entry->none_or_zero = none_or_zero;
@@ -106,12 +108,14 @@ TRACE_EVENT(mm_collapse_huge_page,
 		__print_symbolic(__entry->status, SCAN_STATUS))
 );
 
-TRACE_EVENT(mm_collapse_huge_page_isolate,
+TRACE_EVENT_CONDITION(mm_collapse_huge_page_isolate,
 
-	TP_PROTO(unsigned long pfn, int none_or_zero,
+	TP_PROTO(struct page *page, int none_or_zero,
 		 bool referenced, bool  writable, int status),
 
-	TP_ARGS(pfn, none_or_zero, referenced, writable, status),
+	TP_ARGS(page, none_or_zero, referenced, writable, status),
+
+	TP_CONDITION(page),
 
 	TP_STRUCT__entry(
 		__field(unsigned long, pfn)
@@ -122,7 +126,7 @@ TRACE_EVENT(mm_collapse_huge_page_isolate,
 	),
 
 	TP_fast_assign(
-		__entry->pfn = pfn;
+		__entry->pfn = page_to_pfn(page);
 		__entry->none_or_zero = none_or_zero;
 		__entry->referenced = referenced;
 		__entry->writable = writable;
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 67b00a1..fb3c4f8 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1977,7 +1977,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 	if (likely(writable)) {
 		if (likely(referenced)) {
 			result = SCAN_SUCCEED;
-			trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
+			trace_mm_collapse_huge_page_isolate(page, none_or_zero,
 							    referenced, writable, result);
 			return 1;
 		}
@@ -1987,7 +1987,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 
 out:
 	release_pte_pages(pte, _pte);
-	trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
+	trace_mm_collapse_huge_page_isolate(page, none_or_zero,
 					    referenced, writable, result);
 	return 0;
 }
@@ -2530,7 +2530,7 @@ out_unmap:
 		collapse_huge_page(mm, address, hpage, vma, node);
 	}
 out:
-	trace_mm_khugepaged_scan_pmd(mm, page_to_pfn(page), writable, referenced,
+	trace_mm_khugepaged_scan_pmd(mm, page, writable, referenced,
 				     none_or_zero, result, unmapped);
 	return ret;
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-12  8:21 ` yalin wang
  0 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-12  8:21 UTC (permalink / raw)
  To: rostedt, mingo, akpm, ebru.akagunduz, riel, kirill.shutemov,
	vbabka, yalin.wang2010, jmarchan, mgorman, willy, linux-kernel,
	linux-mm

This crash is caused by NULL pointer deference, in page_to_pfn() marco,
when page == NULL :

[  182.639154 ] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[  182.639491 ] pgd = ffffffc00077a000
[  182.639761 ] [00000000] *pgd=00000000b9422003, *pud=00000000b9422003, *pmd=00000000b9423003, *pte=0060000008000707
[  182.640749 ] Internal error: Oops: 94000006 [#1] SMP
[  182.641197 ] Modules linked in:
[  182.641580 ] CPU: 1 PID: 26 Comm: khugepaged Tainted: G        W       4.3.0-rc6-next-20151022ajb-00001-g32f3386-dirty #3
[  182.642077 ] Hardware name: linux,dummy-virt (DT)
[  182.642227 ] task: ffffffc07957c080 ti: ffffffc079638000 task.ti: ffffffc079638000
[  182.642598 ] PC is at khugepaged+0x378/0x1af8
[  182.642826 ] LR is at khugepaged+0x418/0x1af8
[  182.643047 ] pc : [<ffffffc0001980ac>] lr : [<ffffffc00019814c>] pstate: 60000145
[  182.643490 ] sp : ffffffc07963bca0
[  182.643650 ] x29: ffffffc07963bca0 x28: ffffffc00075c000
[  182.644024 ] x27: ffffffc00f275040 x26: ffffffc0006c7000
[  182.644334 ] x25: 00e8000048800f51 x24: 0000000006400000
[  182.644687 ] x23: 0000000000000002 x22: 0000000000000000
[  182.644972 ] x21: 0000000000000000 x20: 0000000000000000
[  182.645446 ] x19: 0000000000000000 x18: 0000007ff86d0990
[  182.645931 ] x17: 00000000007ef9c8 x16: ffffffc000098390
[  182.646236 ] x15: ffffffffffffffff x14: 00000000ffffffff
[  182.646649 ] x13: 000000000000016a x12: 0000000000000000
[  182.647046 ] x11: ffffffc07f025020 x10: 0000000000000000
[  182.647395 ] x9 : 0000000000000048 x8 : ffffffc000721e28
[  182.647872 ] x7 : 0000000000000000 x6 : ffffffc07f02d000
[  182.648261 ] x5 : fffffffffffffe00 x4 : ffffffc00f275040
[  182.648611 ] x3 : 0000000000000000 x2 : ffffffc00f2ad000
[  182.648908 ] x1 : 0000000000000000 x0 : ffffffc000727000
[  182.649147 ]
[  182.649252 ] Process khugepaged (pid: 26, stack limit = 0xffffffc079638020)
[  182.649724 ] Stack: (0xffffffc07963bca0 to 0xffffffc07963c000)
[  182.650141 ] bca0: ffffffc07963be30 ffffffc0000b5044 ffffffc07961fb80 ffffffc00072e630
[  182.650587 ] bcc0: ffffffc0005d5090 0000000000000000 ffffffc000197d34 0000000000000000
[  182.651009 ] bce0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.651446 ] bd00: ffffffc07963bd90 ffffffc07f1cbf80 000000004f3be003 ffffffc00f2750a4
[  182.651956 ] bd20: ffffffc00f3bf000 ffffffc000000001 0000000000000001 ffffffc07f085740
[  182.652520 ] bd40: ffffffc00f2ad188 ffffffc000000000 0000000006200000 ffffffc00f275040
[  182.652972 ] bd60: ffffffc0006b1a90 ffffffc079638000 ffffffc07963be20 ffffffc00f0144d0
[  182.653357 ] bd80: ffffffc000000000 0000000006400000 ffffffc00f0144d0 00000a0800000001
[  182.653793 ] bda0: 0000100000000001 ffffffc000000001 ffffffc07f025000 ffffffc00f2750a8
[  182.654226 ] bdc0: 00000001000005f8 ffffffc00075a000 0000000006a00000 ffffffc000727000
[  182.654522 ] bde0: ffffffc0006e8478 ffffffc000000000 0000000100000000 ffffffc078fb9000
[  182.654869 ] be00: ffffffc07963be30 ffffffc000000000 ffffffc07957c080 ffffffc0000cfc4c
[  182.655225 ] be20: ffffffc07963be20 ffffffc07963be20 0000000000000000 ffffffc000085c50
[  182.655588 ] be40: ffffffc0000b4f64 ffffffc07961fb80 0000000000000000 0000000000000000
[  182.656138 ] be60: 0000000000000000 ffffffc0000bee2c ffffffc0000b4f64 0000000000000000
[  182.656609 ] be80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.657145 ] bea0: ffffffc07963bea0 ffffffc07963bea0 0000000000000000 ffffffc000000000
[  182.657475 ] bec0: ffffffc07963bec0 ffffffc07963bec0 0000000000000000 0000000000000000
[  182.657922 ] bee0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.658558 ] bf00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.658972 ] bf20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.659291 ] bf40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.659722 ] bf60: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.660122 ] bf80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.660654 ] bfa0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.661064 ] bfc0: 0000000000000000 0000000000000000 0000000000000000 0000000000000005
[  182.661466 ] bfe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  182.661848 ] Call trace:
[  182.662050 ] [<ffffffc0001980ac>] khugepaged+0x378/0x1af8
[  182.662294 ] [<ffffffc0000b5040>] kthread+0xdc/0xf4
[  182.662605 ] [<ffffffc000085c4c>] ret_from_fork+0xc/0x40
[  182.663046 ] Code: 35001700 f0002c60 aa0703e3 f9009fa0 (f94000e0)
[  182.663901 ] ---[ end trace 637503d8e28ae69e  ]---
[  182.664160 ] Kernel panic - not syncing: Fatal exception
[  182.664571 ] CPU2: stopping
[  182.664794 ] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G      D W       4.3.0-rc6-next-20151022ajb-00001-g32f3386-dirty #3
[  182.665248 ] Hardware name: linux,dummy-virt (DT)

add the trace point with TP_CONDITION(page),
avoid trace NULL page.

Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
---
 include/trace/events/huge_memory.h | 20 ++++++++++++--------
 mm/huge_memory.c                   |  6 +++---
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index 11c59ca..727647b 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -45,12 +45,14 @@ SCAN_STATUS
 #define EM(a, b)	{a, b},
 #define EMe(a, b)	{a, b}
 
-TRACE_EVENT(mm_khugepaged_scan_pmd,
+TRACE_EVENT_CONDITION(mm_khugepaged_scan_pmd,
 
-	TP_PROTO(struct mm_struct *mm, unsigned long pfn, bool writable,
+	TP_PROTO(struct mm_struct *mm, struct page *page, bool writable,
 		 bool referenced, int none_or_zero, int status, int unmapped),
 
-	TP_ARGS(mm, pfn, writable, referenced, none_or_zero, status, unmapped),
+	TP_ARGS(mm, page, writable, referenced, none_or_zero, status, unmapped),
+
+	TP_CONDITION(page),
 
 	TP_STRUCT__entry(
 		__field(struct mm_struct *, mm)
@@ -64,7 +66,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
 
 	TP_fast_assign(
 		__entry->mm = mm;
-		__entry->pfn = pfn;
+		__entry->pfn = page_to_pfn(page);
 		__entry->writable = writable;
 		__entry->referenced = referenced;
 		__entry->none_or_zero = none_or_zero;
@@ -106,12 +108,14 @@ TRACE_EVENT(mm_collapse_huge_page,
 		__print_symbolic(__entry->status, SCAN_STATUS))
 );
 
-TRACE_EVENT(mm_collapse_huge_page_isolate,
+TRACE_EVENT_CONDITION(mm_collapse_huge_page_isolate,
 
-	TP_PROTO(unsigned long pfn, int none_or_zero,
+	TP_PROTO(struct page *page, int none_or_zero,
 		 bool referenced, bool  writable, int status),
 
-	TP_ARGS(pfn, none_or_zero, referenced, writable, status),
+	TP_ARGS(page, none_or_zero, referenced, writable, status),
+
+	TP_CONDITION(page),
 
 	TP_STRUCT__entry(
 		__field(unsigned long, pfn)
@@ -122,7 +126,7 @@ TRACE_EVENT(mm_collapse_huge_page_isolate,
 	),
 
 	TP_fast_assign(
-		__entry->pfn = pfn;
+		__entry->pfn = page_to_pfn(page);
 		__entry->none_or_zero = none_or_zero;
 		__entry->referenced = referenced;
 		__entry->writable = writable;
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 67b00a1..fb3c4f8 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1977,7 +1977,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 	if (likely(writable)) {
 		if (likely(referenced)) {
 			result = SCAN_SUCCEED;
-			trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
+			trace_mm_collapse_huge_page_isolate(page, none_or_zero,
 							    referenced, writable, result);
 			return 1;
 		}
@@ -1987,7 +1987,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 
 out:
 	release_pte_pages(pte, _pte);
-	trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
+	trace_mm_collapse_huge_page_isolate(page, none_or_zero,
 					    referenced, writable, result);
 	return 0;
 }
@@ -2530,7 +2530,7 @@ out_unmap:
 		collapse_huge_page(mm, address, hpage, vma, node);
 	}
 out:
-	trace_mm_khugepaged_scan_pmd(mm, page_to_pfn(page), writable, referenced,
+	trace_mm_khugepaged_scan_pmd(mm, page, writable, referenced,
 				     none_or_zero, result, unmapped);
 	return ret;
 }
-- 
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-12  8:21 ` yalin wang
@ 2015-11-12 14:29   ` Steven Rostedt
  -1 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-12 14:29 UTC (permalink / raw)
  To: yalin wang
  Cc: mingo, akpm, ebru.akagunduz, riel, kirill.shutemov, vbabka,
	jmarchan, mgorman, willy, linux-kernel, linux-mm

On Thu, 12 Nov 2015 16:21:02 +0800
yalin wang <yalin.wang2010@gmail.com> wrote:

> This crash is caused by NULL pointer deference, in page_to_pfn() marco,
> when page == NULL :
> 
> [  182.639154 ] Unable to handle kernel NULL pointer dereference at virtual address 00000000


> add the trace point with TP_CONDITION(page),

I wonder if we still want to trace even if page is NULL?

> avoid trace NULL page.
> 
> Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
> ---
>  include/trace/events/huge_memory.h | 20 ++++++++++++--------
>  mm/huge_memory.c                   |  6 +++---
>  2 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index 11c59ca..727647b 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -45,12 +45,14 @@ SCAN_STATUS
>  #define EM(a, b)	{a, b},
>  #define EMe(a, b)	{a, b}
>  
> -TRACE_EVENT(mm_khugepaged_scan_pmd,
> +TRACE_EVENT_CONDITION(mm_khugepaged_scan_pmd,
>  
> -	TP_PROTO(struct mm_struct *mm, unsigned long pfn, bool writable,
> +	TP_PROTO(struct mm_struct *mm, struct page *page, bool writable,
>  		 bool referenced, int none_or_zero, int status, int unmapped),
>  
> -	TP_ARGS(mm, pfn, writable, referenced, none_or_zero, status, unmapped),
> +	TP_ARGS(mm, page, writable, referenced, none_or_zero, status, unmapped),
> +
> +	TP_CONDITION(page),
>  
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
> @@ -64,7 +66,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>  
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->pfn = pfn;
> +		__entry->pfn = page_to_pfn(page);

Instead of the condition, we could have:

	__entry->pfn = page ? page_to_pfn(page) : -1;


But if there's no reason to do the tracepoint if page is NULL, then
this patch is fine. I'm just throwing out this idea.

-- Steve

>  		__entry->writable = writable;
>  		__entry->referenced = referenced;
>  		__entry->none_or_zero = none_or_zero;
> @@ -106,12 +108,14 @@ TRACE_EVENT(mm_collapse_huge_page,
>  		__print_symbolic(__entry->status, SCAN_STATUS))
>  );
>  
> -TRACE_EVENT(mm_collapse_huge_page_isolate,
> +TRACE_EVENT_CONDITION(mm_collapse_huge_page_isolate,
>  
> -	TP_PROTO(unsigned long pfn, int none_or_zero,
> +	TP_PROTO(struct page *page, int none_or_zero,
>  		 bool referenced, bool  writable, int status),
>  
> -	TP_ARGS(pfn, none_or_zero, referenced, writable, status),
> +	TP_ARGS(page, none_or_zero, referenced, writable, status),
> +
> +	TP_CONDITION(page),
>  
>  	TP_STRUCT__entry(
>  		__field(unsigned long, pfn)
\

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-12 14:29   ` Steven Rostedt
  0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-12 14:29 UTC (permalink / raw)
  To: yalin wang
  Cc: mingo, akpm, ebru.akagunduz, riel, kirill.shutemov, vbabka,
	jmarchan, mgorman, willy, linux-kernel, linux-mm

On Thu, 12 Nov 2015 16:21:02 +0800
yalin wang <yalin.wang2010@gmail.com> wrote:

> This crash is caused by NULL pointer deference, in page_to_pfn() marco,
> when page == NULL :
> 
> [  182.639154 ] Unable to handle kernel NULL pointer dereference at virtual address 00000000


> add the trace point with TP_CONDITION(page),

I wonder if we still want to trace even if page is NULL?

> avoid trace NULL page.
> 
> Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
> ---
>  include/trace/events/huge_memory.h | 20 ++++++++++++--------
>  mm/huge_memory.c                   |  6 +++---
>  2 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index 11c59ca..727647b 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -45,12 +45,14 @@ SCAN_STATUS
>  #define EM(a, b)	{a, b},
>  #define EMe(a, b)	{a, b}
>  
> -TRACE_EVENT(mm_khugepaged_scan_pmd,
> +TRACE_EVENT_CONDITION(mm_khugepaged_scan_pmd,
>  
> -	TP_PROTO(struct mm_struct *mm, unsigned long pfn, bool writable,
> +	TP_PROTO(struct mm_struct *mm, struct page *page, bool writable,
>  		 bool referenced, int none_or_zero, int status, int unmapped),
>  
> -	TP_ARGS(mm, pfn, writable, referenced, none_or_zero, status, unmapped),
> +	TP_ARGS(mm, page, writable, referenced, none_or_zero, status, unmapped),
> +
> +	TP_CONDITION(page),
>  
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
> @@ -64,7 +66,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>  
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->pfn = pfn;
> +		__entry->pfn = page_to_pfn(page);

Instead of the condition, we could have:

	__entry->pfn = page ? page_to_pfn(page) : -1;


But if there's no reason to do the tracepoint if page is NULL, then
this patch is fine. I'm just throwing out this idea.

-- Steve

>  		__entry->writable = writable;
>  		__entry->referenced = referenced;
>  		__entry->none_or_zero = none_or_zero;
> @@ -106,12 +108,14 @@ TRACE_EVENT(mm_collapse_huge_page,
>  		__print_symbolic(__entry->status, SCAN_STATUS))
>  );
>  
> -TRACE_EVENT(mm_collapse_huge_page_isolate,
> +TRACE_EVENT_CONDITION(mm_collapse_huge_page_isolate,
>  
> -	TP_PROTO(unsigned long pfn, int none_or_zero,
> +	TP_PROTO(struct page *page, int none_or_zero,
>  		 bool referenced, bool  writable, int status),
>  
> -	TP_ARGS(pfn, none_or_zero, referenced, writable, status),
> +	TP_ARGS(page, none_or_zero, referenced, writable, status),
> +
> +	TP_CONDITION(page),
>  
>  	TP_STRUCT__entry(
>  		__field(unsigned long, pfn)
\

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-12 14:29   ` Steven Rostedt
@ 2015-11-13 10:47     ` Vlastimil Babka
  -1 siblings, 0 replies; 30+ messages in thread
From: Vlastimil Babka @ 2015-11-13 10:47 UTC (permalink / raw)
  To: Steven Rostedt, yalin wang
  Cc: mingo, akpm, ebru.akagunduz, riel, kirill.shutemov, jmarchan,
	mgorman, willy, linux-kernel, linux-mm

On 11/12/2015 03:29 PM, Steven Rostedt wrote:
> On Thu, 12 Nov 2015 16:21:02 +0800
> yalin wang <yalin.wang2010@gmail.com> wrote:
>
>> This crash is caused by NULL pointer deference, in page_to_pfn() marco,
>> when page == NULL :
>>
>> [  182.639154 ] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>
>
>> add the trace point with TP_CONDITION(page),
>
> I wonder if we still want to trace even if page is NULL?

I'd say we want to. There's even a "SCAN_PAGE_NULL" result defined for 
that case, and otherwise we would only have to guess why collapsing 
failed, which is the thing that the tracepoint should help us find out 
in the first place :)

>> avoid trace NULL page.
>>
>> Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
>> ---
>>   include/trace/events/huge_memory.h | 20 ++++++++++++--------
>>   mm/huge_memory.c                   |  6 +++---
>>   2 files changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
>> index 11c59ca..727647b 100644
>> --- a/include/trace/events/huge_memory.h
>> +++ b/include/trace/events/huge_memory.h
>> @@ -45,12 +45,14 @@ SCAN_STATUS
>>   #define EM(a, b)	{a, b},
>>   #define EMe(a, b)	{a, b}
>>
>> -TRACE_EVENT(mm_khugepaged_scan_pmd,
>> +TRACE_EVENT_CONDITION(mm_khugepaged_scan_pmd,
>>
>> -	TP_PROTO(struct mm_struct *mm, unsigned long pfn, bool writable,
>> +	TP_PROTO(struct mm_struct *mm, struct page *page, bool writable,
>>   		 bool referenced, int none_or_zero, int status, int unmapped),
>>
>> -	TP_ARGS(mm, pfn, writable, referenced, none_or_zero, status, unmapped),
>> +	TP_ARGS(mm, page, writable, referenced, none_or_zero, status, unmapped),
>> +
>> +	TP_CONDITION(page),
>>
>>   	TP_STRUCT__entry(
>>   		__field(struct mm_struct *, mm)
>> @@ -64,7 +66,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>>
>>   	TP_fast_assign(
>>   		__entry->mm = mm;
>> -		__entry->pfn = pfn;
>> +		__entry->pfn = page_to_pfn(page);
>
> Instead of the condition, we could have:
>
> 	__entry->pfn = page ? page_to_pfn(page) : -1;

I agree. Please do it like this.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-13 10:47     ` Vlastimil Babka
  0 siblings, 0 replies; 30+ messages in thread
From: Vlastimil Babka @ 2015-11-13 10:47 UTC (permalink / raw)
  To: Steven Rostedt, yalin wang
  Cc: mingo, akpm, ebru.akagunduz, riel, kirill.shutemov, jmarchan,
	mgorman, willy, linux-kernel, linux-mm

On 11/12/2015 03:29 PM, Steven Rostedt wrote:
> On Thu, 12 Nov 2015 16:21:02 +0800
> yalin wang <yalin.wang2010@gmail.com> wrote:
>
>> This crash is caused by NULL pointer deference, in page_to_pfn() marco,
>> when page == NULL :
>>
>> [  182.639154 ] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>
>
>> add the trace point with TP_CONDITION(page),
>
> I wonder if we still want to trace even if page is NULL?

I'd say we want to. There's even a "SCAN_PAGE_NULL" result defined for 
that case, and otherwise we would only have to guess why collapsing 
failed, which is the thing that the tracepoint should help us find out 
in the first place :)

>> avoid trace NULL page.
>>
>> Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
>> ---
>>   include/trace/events/huge_memory.h | 20 ++++++++++++--------
>>   mm/huge_memory.c                   |  6 +++---
>>   2 files changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
>> index 11c59ca..727647b 100644
>> --- a/include/trace/events/huge_memory.h
>> +++ b/include/trace/events/huge_memory.h
>> @@ -45,12 +45,14 @@ SCAN_STATUS
>>   #define EM(a, b)	{a, b},
>>   #define EMe(a, b)	{a, b}
>>
>> -TRACE_EVENT(mm_khugepaged_scan_pmd,
>> +TRACE_EVENT_CONDITION(mm_khugepaged_scan_pmd,
>>
>> -	TP_PROTO(struct mm_struct *mm, unsigned long pfn, bool writable,
>> +	TP_PROTO(struct mm_struct *mm, struct page *page, bool writable,
>>   		 bool referenced, int none_or_zero, int status, int unmapped),
>>
>> -	TP_ARGS(mm, pfn, writable, referenced, none_or_zero, status, unmapped),
>> +	TP_ARGS(mm, page, writable, referenced, none_or_zero, status, unmapped),
>> +
>> +	TP_CONDITION(page),
>>
>>   	TP_STRUCT__entry(
>>   		__field(struct mm_struct *, mm)
>> @@ -64,7 +66,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>>
>>   	TP_fast_assign(
>>   		__entry->mm = mm;
>> -		__entry->pfn = pfn;
>> +		__entry->pfn = page_to_pfn(page);
>
> Instead of the condition, we could have:
>
> 	__entry->pfn = page ? page_to_pfn(page) : -1;

I agree. Please do it like this.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-13 10:47     ` Vlastimil Babka
@ 2015-11-13 11:54       ` yalin wang
  -1 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-13 11:54 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Steven Rostedt, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm


> On Nov 13, 2015, at 18:47, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> On 11/12/2015 03:29 PM, Steven Rostedt wrote:
>> On Thu, 12 Nov 2015 16:21:02 +0800
>> yalin wang <yalin.wang2010@gmail.com> wrote:
>> 
>>> This crash is caused by NULL pointer deference, in page_to_pfn() marco,
>>> when page == NULL :
>>> 
>>> [  182.639154 ] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>> 
>> 
>>> add the trace point with TP_CONDITION(page),
>> 
>> I wonder if we still want to trace even if page is NULL?
> 
> I'd say we want to. There's even a "SCAN_PAGE_NULL" result defined for that case, and otherwise we would only have to guess why collapsing failed, which is the thing that the tracepoint should help us find out in the first place :)
> 
>>> avoid trace NULL page.
>>> 
>>> Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
>>> ---
>>>  include/trace/events/huge_memory.h | 20 ++++++++++++--------
>>>  mm/huge_memory.c                   |  6 +++---
>>>  2 files changed, 15 insertions(+), 11 deletions(-)
>>> 
>>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
>>> index 11c59ca..727647b 100644
>>> --- a/include/trace/events/huge_memory.h
>>> +++ b/include/trace/events/huge_memory.h
>>> @@ -45,12 +45,14 @@ SCAN_STATUS
>>>  #define EM(a, b)	{a, b},
>>>  #define EMe(a, b)	{a, b}
>>> 
>>> -TRACE_EVENT(mm_khugepaged_scan_pmd,
>>> +TRACE_EVENT_CONDITION(mm_khugepaged_scan_pmd,
>>> 
>>> -	TP_PROTO(struct mm_struct *mm, unsigned long pfn, bool writable,
>>> +	TP_PROTO(struct mm_struct *mm, struct page *page, bool writable,
>>>  		 bool referenced, int none_or_zero, int status, int unmapped),
>>> 
>>> -	TP_ARGS(mm, pfn, writable, referenced, none_or_zero, status, unmapped),
>>> +	TP_ARGS(mm, page, writable, referenced, none_or_zero, status, unmapped),
>>> +
>>> +	TP_CONDITION(page),
>>> 
>>>  	TP_STRUCT__entry(
>>>  		__field(struct mm_struct *, mm)
>>> @@ -64,7 +66,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>>> 
>>>  	TP_fast_assign(
>>>  		__entry->mm = mm;
>>> -		__entry->pfn = pfn;
>>> +		__entry->pfn = page_to_pfn(page);
>> 
>> Instead of the condition, we could have:
>> 
>> 	__entry->pfn = page ? page_to_pfn(page) : -1;
> 
> I agree. Please do it like this.
ok ,  i will send V5 patch .

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-13 11:54       ` yalin wang
  0 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-13 11:54 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Steven Rostedt, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm


> On Nov 13, 2015, at 18:47, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> On 11/12/2015 03:29 PM, Steven Rostedt wrote:
>> On Thu, 12 Nov 2015 16:21:02 +0800
>> yalin wang <yalin.wang2010@gmail.com> wrote:
>> 
>>> This crash is caused by NULL pointer deference, in page_to_pfn() marco,
>>> when page == NULL :
>>> 
>>> [  182.639154 ] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>> 
>> 
>>> add the trace point with TP_CONDITION(page),
>> 
>> I wonder if we still want to trace even if page is NULL?
> 
> I'd say we want to. There's even a "SCAN_PAGE_NULL" result defined for that case, and otherwise we would only have to guess why collapsing failed, which is the thing that the tracepoint should help us find out in the first place :)
> 
>>> avoid trace NULL page.
>>> 
>>> Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
>>> ---
>>>  include/trace/events/huge_memory.h | 20 ++++++++++++--------
>>>  mm/huge_memory.c                   |  6 +++---
>>>  2 files changed, 15 insertions(+), 11 deletions(-)
>>> 
>>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
>>> index 11c59ca..727647b 100644
>>> --- a/include/trace/events/huge_memory.h
>>> +++ b/include/trace/events/huge_memory.h
>>> @@ -45,12 +45,14 @@ SCAN_STATUS
>>>  #define EM(a, b)	{a, b},
>>>  #define EMe(a, b)	{a, b}
>>> 
>>> -TRACE_EVENT(mm_khugepaged_scan_pmd,
>>> +TRACE_EVENT_CONDITION(mm_khugepaged_scan_pmd,
>>> 
>>> -	TP_PROTO(struct mm_struct *mm, unsigned long pfn, bool writable,
>>> +	TP_PROTO(struct mm_struct *mm, struct page *page, bool writable,
>>>  		 bool referenced, int none_or_zero, int status, int unmapped),
>>> 
>>> -	TP_ARGS(mm, pfn, writable, referenced, none_or_zero, status, unmapped),
>>> +	TP_ARGS(mm, page, writable, referenced, none_or_zero, status, unmapped),
>>> +
>>> +	TP_CONDITION(page),
>>> 
>>>  	TP_STRUCT__entry(
>>>  		__field(struct mm_struct *, mm)
>>> @@ -64,7 +66,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>>> 
>>>  	TP_fast_assign(
>>>  		__entry->mm = mm;
>>> -		__entry->pfn = pfn;
>>> +		__entry->pfn = page_to_pfn(page);
>> 
>> Instead of the condition, we could have:
>> 
>> 	__entry->pfn = page ? page_to_pfn(page) : -1;
> 
> I agree. Please do it like this.
ok ,  i will send V5 patch .
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-13 11:54       ` yalin wang
@ 2015-11-13 14:01         ` Steven Rostedt
  -1 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-13 14:01 UTC (permalink / raw)
  To: yalin wang
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm

On Fri, 13 Nov 2015 19:54:11 +0800
yalin wang <yalin.wang2010@gmail.com> wrote:

> >>>  	TP_fast_assign(
> >>>  		__entry->mm = mm;
> >>> -		__entry->pfn = pfn;
> >>> +		__entry->pfn = page_to_pfn(page);  
> >> 
> >> Instead of the condition, we could have:
> >> 
> >> 	__entry->pfn = page ? page_to_pfn(page) : -1;  
> > 
> > I agree. Please do it like this.  

hmm, pfn is defined as an unsigned long, would -1 be the best.
Or should it be (-1UL).

Then we could also have:

        TP_printk("mm=%p, scan_pfn=0x%lx%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
                __entry->mm,
                __entry->pfn == (-1UL) ? 0 : __entry->pfn,
		__entry->pfn == (-1UL) ? "(null)" : "",

Note the added %s after %lx I have in the print format.

-- Steve



> ok ,  i will send V5 patch .

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-13 14:01         ` Steven Rostedt
  0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-13 14:01 UTC (permalink / raw)
  To: yalin wang
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm

On Fri, 13 Nov 2015 19:54:11 +0800
yalin wang <yalin.wang2010@gmail.com> wrote:

> >>>  	TP_fast_assign(
> >>>  		__entry->mm = mm;
> >>> -		__entry->pfn = pfn;
> >>> +		__entry->pfn = page_to_pfn(page);  
> >> 
> >> Instead of the condition, we could have:
> >> 
> >> 	__entry->pfn = page ? page_to_pfn(page) : -1;  
> > 
> > I agree. Please do it like this.  

hmm, pfn is defined as an unsigned long, would -1 be the best.
Or should it be (-1UL).

Then we could also have:

        TP_printk("mm=%p, scan_pfn=0x%lx%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
                __entry->mm,
                __entry->pfn == (-1UL) ? 0 : __entry->pfn,
		__entry->pfn == (-1UL) ? "(null)" : "",

Note the added %s after %lx I have in the print format.

-- Steve



> ok ,  i will send V5 patch .

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-13 14:01         ` Steven Rostedt
@ 2015-11-16  1:35           ` yalin wang
  -1 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-16  1:35 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm


> On Nov 13, 2015, at 22:01, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Fri, 13 Nov 2015 19:54:11 +0800
> yalin wang <yalin.wang2010@gmail.com> wrote:
> 
>>>>> 	TP_fast_assign(
>>>>> 		__entry->mm = mm;
>>>>> -		__entry->pfn = pfn;
>>>>> +		__entry->pfn = page_to_pfn(page);  
>>>> 
>>>> Instead of the condition, we could have:
>>>> 
>>>> 	__entry->pfn = page ? page_to_pfn(page) : -1;  
>>> 
>>> I agree. Please do it like this.  
> 
> hmm, pfn is defined as an unsigned long, would -1 be the best.
> Or should it be (-1UL).
> 
> Then we could also have:
> 
>        TP_printk("mm=%p, scan_pfn=0x%lx%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
>                __entry->mm,
>                __entry->pfn == (-1UL) ? 0 : __entry->pfn,
> 		__entry->pfn == (-1UL) ? "(null)" : "",
> 
> Note the added %s after %lx I have in the print format.
> 
> -- Steve
it is not easy to print for perf tools in userspace ,
if you use this format ,
for user space perf tool, it print the entry by look up the member in entry struct by offset ,
you print a dynamic string which user space perf tool don’t know how to print this string .

Thanks 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-16  1:35           ` yalin wang
  0 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-16  1:35 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm


> On Nov 13, 2015, at 22:01, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Fri, 13 Nov 2015 19:54:11 +0800
> yalin wang <yalin.wang2010@gmail.com> wrote:
> 
>>>>> 	TP_fast_assign(
>>>>> 		__entry->mm = mm;
>>>>> -		__entry->pfn = pfn;
>>>>> +		__entry->pfn = page_to_pfn(page);  
>>>> 
>>>> Instead of the condition, we could have:
>>>> 
>>>> 	__entry->pfn = page ? page_to_pfn(page) : -1;  
>>> 
>>> I agree. Please do it like this.  
> 
> hmm, pfn is defined as an unsigned long, would -1 be the best.
> Or should it be (-1UL).
> 
> Then we could also have:
> 
>        TP_printk("mm=%p, scan_pfn=0x%lx%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
>                __entry->mm,
>                __entry->pfn == (-1UL) ? 0 : __entry->pfn,
> 		__entry->pfn == (-1UL) ? "(null)" : "",
> 
> Note the added %s after %lx I have in the print format.
> 
> -- Steve
it is not easy to print for perf tools in userspace ,
if you use this format ,
for user space perf tool, it print the entry by look up the member in entry struct by offset ,
you print a dynamic string which user space perf tool don’t know how to print this string .

Thanks 
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-16  1:35           ` yalin wang
@ 2015-11-16 10:16             ` Vlastimil Babka
  -1 siblings, 0 replies; 30+ messages in thread
From: Vlastimil Babka @ 2015-11-16 10:16 UTC (permalink / raw)
  To: yalin wang, Steven Rostedt
  Cc: Ingo Molnar, Andrew Morton, Ebru Akagunduz, Rik van Riel,
	Kirill A. Shutemov, jmarchan, mgorman, willy, linux-kernel,
	linux-mm

On 11/16/2015 02:35 AM, yalin wang wrote:
>
>> On Nov 13, 2015, at 22:01, Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>> On Fri, 13 Nov 2015 19:54:11 +0800
>> yalin wang <yalin.wang2010@gmail.com> wrote:
>>
>>>>>> 	TP_fast_assign(
>>>>>> 		__entry->mm = mm;
>>>>>> -		__entry->pfn = pfn;
>>>>>> +		__entry->pfn = page_to_pfn(page);
>>>>>
>>>>> Instead of the condition, we could have:
>>>>>
>>>>> 	__entry->pfn = page ? page_to_pfn(page) : -1;
>>>>
>>>> I agree. Please do it like this.
>>
>> hmm, pfn is defined as an unsigned long, would -1 be the best.
>> Or should it be (-1UL).
>>
>> Then we could also have:
>>
>>         TP_printk("mm=%p, scan_pfn=0x%lx%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
>>                 __entry->mm,
>>                 __entry->pfn == (-1UL) ? 0 : __entry->pfn,
>> 		__entry->pfn == (-1UL) ? "(null)" : "",
>>
>> Note the added %s after %lx I have in the print format.
>>
>> -- Steve
> it is not easy to print for perf tools in userspace ,
> if you use this format ,
> for user space perf tool, it print the entry by look up the member in entry struct by offset ,
> you print a dynamic string which user space perf tool don’t know how to print this string .

Does it work through trace-cmd?

> Thanks
>


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-16 10:16             ` Vlastimil Babka
  0 siblings, 0 replies; 30+ messages in thread
From: Vlastimil Babka @ 2015-11-16 10:16 UTC (permalink / raw)
  To: yalin wang, Steven Rostedt
  Cc: Ingo Molnar, Andrew Morton, Ebru Akagunduz, Rik van Riel,
	Kirill A. Shutemov, jmarchan, mgorman, willy, linux-kernel,
	linux-mm

On 11/16/2015 02:35 AM, yalin wang wrote:
>
>> On Nov 13, 2015, at 22:01, Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>> On Fri, 13 Nov 2015 19:54:11 +0800
>> yalin wang <yalin.wang2010@gmail.com> wrote:
>>
>>>>>> 	TP_fast_assign(
>>>>>> 		__entry->mm = mm;
>>>>>> -		__entry->pfn = pfn;
>>>>>> +		__entry->pfn = page_to_pfn(page);
>>>>>
>>>>> Instead of the condition, we could have:
>>>>>
>>>>> 	__entry->pfn = page ? page_to_pfn(page) : -1;
>>>>
>>>> I agree. Please do it like this.
>>
>> hmm, pfn is defined as an unsigned long, would -1 be the best.
>> Or should it be (-1UL).
>>
>> Then we could also have:
>>
>>         TP_printk("mm=%p, scan_pfn=0x%lx%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
>>                 __entry->mm,
>>                 __entry->pfn == (-1UL) ? 0 : __entry->pfn,
>> 		__entry->pfn == (-1UL) ? "(null)" : "",
>>
>> Note the added %s after %lx I have in the print format.
>>
>> -- Steve
> it is not easy to print for perf tools in userspace ,
> if you use this format ,
> for user space perf tool, it print the entry by look up the member in entry struct by offset ,
> you print a dynamic string which user space perf tool dona??t know how to print this string .

Does it work through trace-cmd?

> Thanks
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-16  1:35           ` yalin wang
@ 2015-11-16 14:22             ` Steven Rostedt
  -1 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-16 14:22 UTC (permalink / raw)
  To: yalin wang
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm

On Mon, 16 Nov 2015 09:35:53 +0800
yalin wang <yalin.wang2010@gmail.com> wrote:

> > On Nov 13, 2015, at 22:01, Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > On Fri, 13 Nov 2015 19:54:11 +0800
> > yalin wang <yalin.wang2010@gmail.com> wrote:
> >   
> >>>>> 	TP_fast_assign(
> >>>>> 		__entry->mm = mm;
> >>>>> -		__entry->pfn = pfn;
> >>>>> +		__entry->pfn = page_to_pfn(page);    
> >>>> 
> >>>> Instead of the condition, we could have:
> >>>> 
> >>>> 	__entry->pfn = page ? page_to_pfn(page) : -1;    
> >>> 
> >>> I agree. Please do it like this.    
> > 
> > hmm, pfn is defined as an unsigned long, would -1 be the best.
> > Or should it be (-1UL).
> > 
> > Then we could also have:
> > 
> >        TP_printk("mm=%p, scan_pfn=0x%lx%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
> >                __entry->mm,
> >                __entry->pfn == (-1UL) ? 0 : __entry->pfn,
> > 		__entry->pfn == (-1UL) ? "(null)" : "",
> > 
> > Note the added %s after %lx I have in the print format.
> > 
> > -- Steve  
> it is not easy to print for perf tools in userspace ,
> if you use this format ,
> for user space perf tool, it print the entry by look up the member in entry struct by offset ,
> you print a dynamic string which user space perf tool don’t know how to print this string .

Have you tried it? It should work. If not, I'll fix it. The string
"null" is exported in the trace output file, and perf should have
enough information to know how to handle that. If it fails to parse, I
can easily fix it.

Remember, I'm the author of the parsing of events in userspace.

-- Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-16 14:22             ` Steven Rostedt
  0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-16 14:22 UTC (permalink / raw)
  To: yalin wang
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm

On Mon, 16 Nov 2015 09:35:53 +0800
yalin wang <yalin.wang2010@gmail.com> wrote:

> > On Nov 13, 2015, at 22:01, Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > On Fri, 13 Nov 2015 19:54:11 +0800
> > yalin wang <yalin.wang2010@gmail.com> wrote:
> >   
> >>>>> 	TP_fast_assign(
> >>>>> 		__entry->mm = mm;
> >>>>> -		__entry->pfn = pfn;
> >>>>> +		__entry->pfn = page_to_pfn(page);    
> >>>> 
> >>>> Instead of the condition, we could have:
> >>>> 
> >>>> 	__entry->pfn = page ? page_to_pfn(page) : -1;    
> >>> 
> >>> I agree. Please do it like this.    
> > 
> > hmm, pfn is defined as an unsigned long, would -1 be the best.
> > Or should it be (-1UL).
> > 
> > Then we could also have:
> > 
> >        TP_printk("mm=%p, scan_pfn=0x%lx%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
> >                __entry->mm,
> >                __entry->pfn == (-1UL) ? 0 : __entry->pfn,
> > 		__entry->pfn == (-1UL) ? "(null)" : "",
> > 
> > Note the added %s after %lx I have in the print format.
> > 
> > -- Steve  
> it is not easy to print for perf tools in userspace ,
> if you use this format ,
> for user space perf tool, it print the entry by look up the member in entry struct by offset ,
> you print a dynamic string which user space perf tool don’t know how to print this string .

Have you tried it? It should work. If not, I'll fix it. The string
"null" is exported in the trace output file, and perf should have
enough information to know how to handle that. If it fails to parse, I
can easily fix it.

Remember, I'm the author of the parsing of events in userspace.

-- Steve

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-16 10:16             ` Vlastimil Babka
@ 2015-11-16 14:25               ` Steven Rostedt
  -1 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-16 14:25 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: yalin wang, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm

On Mon, 16 Nov 2015 11:16:22 +0100
Vlastimil Babka <vbabka@suse.cz> wrote:
>
> >> -- Steve  
> > it is not easy to print for perf tools in userspace ,
> > if you use this format ,
> > for user space perf tool, it print the entry by look up the member in entry struct by offset ,
> > you print a dynamic string which user space perf tool don’t know how to print this string .  
> 
> Does it work through trace-cmd?

The two use the same code. If it works in one, it will work in the
other.

-- Steve


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-16 14:25               ` Steven Rostedt
  0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-16 14:25 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: yalin wang, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm

On Mon, 16 Nov 2015 11:16:22 +0100
Vlastimil Babka <vbabka@suse.cz> wrote:
>
> >> -- Steve  
> > it is not easy to print for perf tools in userspace ,
> > if you use this format ,
> > for user space perf tool, it print the entry by look up the member in entry struct by offset ,
> > you print a dynamic string which user space perf tool don’t know how to print this string .  
> 
> Does it work through trace-cmd?

The two use the same code. If it works in one, it will work in the
other.

-- Steve

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-16 14:25               ` Steven Rostedt
@ 2015-11-17  2:21                 ` yalin wang
  -1 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-17  2:21 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm


> On Nov 16, 2015, at 22:25, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Mon, 16 Nov 2015 11:16:22 +0100
> Vlastimil Babka <vbabka@suse.cz> wrote:
>> 
>>>> -- Steve  
>>> it is not easy to print for perf tools in userspace ,
>>> if you use this format ,
>>> for user space perf tool, it print the entry by look up the member in entry struct by offset ,
>>> you print a dynamic string which user space perf tool don’t know how to print this string .  
>> 
>> Does it work through trace-cmd?
> 
> The two use the same code. If it works in one, it will work in the
> other.
> 
> -- Steve
> 
i have not tried ,
just a question,
if you print a %s , but don’t call trace_define_field() do define this string in
__entry ,  how does user space perf tool to get this string info and print it ?
i am curious ..
i can try this when i have time.  and report to you .

Thanks

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-17  2:21                 ` yalin wang
  0 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-17  2:21 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm


> On Nov 16, 2015, at 22:25, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Mon, 16 Nov 2015 11:16:22 +0100
> Vlastimil Babka <vbabka@suse.cz> wrote:
>> 
>>>> -- Steve  
>>> it is not easy to print for perf tools in userspace ,
>>> if you use this format ,
>>> for user space perf tool, it print the entry by look up the member in entry struct by offset ,
>>> you print a dynamic string which user space perf tool don’t know how to print this string .  
>> 
>> Does it work through trace-cmd?
> 
> The two use the same code. If it works in one, it will work in the
> other.
> 
> -- Steve
> 
i have not tried ,
just a question,
if you print a %s , but don’t call trace_define_field() do define this string in
__entry ,  how does user space perf tool to get this string info and print it ?
i am curious ..
i can try this when i have time.  and report to you .

Thanks
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-17  2:21                 ` yalin wang
@ 2015-11-17  2:43                   ` Steven Rostedt
  -1 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-17  2:43 UTC (permalink / raw)
  To: yalin wang
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm

On Tue, 17 Nov 2015 10:21:47 +0800
yalin wang <yalin.wang2010@gmail.com> wrote:

  
> i have not tried ,
> just a question,
> if you print a %s , but don’t call trace_define_field() do define this string in
> __entry ,  how does user space perf tool to get this string info and print it ?
> i am curious ..
> i can try this when i have time.  and report to you .

Because the print_fmt has nothing to do with the fields. You can have
as your print_fmt as:

	TP_printk("Message = %s", "hello dolly!")

And both userspace and the kernel with process that correctly (if I got
string processing working in userspace, which I believe I do). The
string is processed, it's not dependent on TP_STRUCT__entry() unless it
references a field there. Which can also be used too:

	TP_printk("Message = %s", __entry->musical ? "Hello dolly!" :
			"Death Trap!")

userspace will see in the entry:

 print_fmt: "Message = %s", REC->musical ? "Hello dolly!" : "Death Trap!"

as long as the field "musical" exists, all is well.

-- Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-17  2:43                   ` Steven Rostedt
  0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2015-11-17  2:43 UTC (permalink / raw)
  To: yalin wang
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm

On Tue, 17 Nov 2015 10:21:47 +0800
yalin wang <yalin.wang2010@gmail.com> wrote:

  
> i have not tried ,
> just a question,
> if you print a %s , but don’t call trace_define_field() do define this string in
> __entry ,  how does user space perf tool to get this string info and print it ?
> i am curious ..
> i can try this when i have time.  and report to you .

Because the print_fmt has nothing to do with the fields. You can have
as your print_fmt as:

	TP_printk("Message = %s", "hello dolly!")

And both userspace and the kernel with process that correctly (if I got
string processing working in userspace, which I believe I do). The
string is processed, it's not dependent on TP_STRUCT__entry() unless it
references a field there. Which can also be used too:

	TP_printk("Message = %s", __entry->musical ? "Hello dolly!" :
			"Death Trap!")

userspace will see in the entry:

 print_fmt: "Message = %s", REC->musical ? "Hello dolly!" : "Death Trap!"

as long as the field "musical" exists, all is well.

-- Steve

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-17  2:43                   ` Steven Rostedt
@ 2015-11-17  3:58                     ` yalin wang
  -1 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-17  3:58 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm


> On Nov 17, 2015, at 10:43, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Tue, 17 Nov 2015 10:21:47 +0800
> yalin wang <yalin.wang2010@gmail.com> wrote:
> 
> 
>> i have not tried ,
>> just a question,
>> if you print a %s , but don’t call trace_define_field() do define this string in
>> __entry ,  how does user space perf tool to get this string info and print it ?
>> i am curious ..
>> i can try this when i have time.  and report to you .
> 
> Because the print_fmt has nothing to do with the fields. You can have
> as your print_fmt as:
> 
> 	TP_printk("Message = %s", "hello dolly!")
> 
> And both userspace and the kernel with process that correctly (if I got
> string processing working in userspace, which I believe I do). The
> string is processed, it's not dependent on TP_STRUCT__entry() unless it
> references a field there. Which can also be used too:
> 
> 	TP_printk("Message = %s", __entry->musical ? "Hello dolly!" :
> 			"Death Trap!")
> 
> userspace will see in the entry:
> 
> print_fmt: "Message = %s", REC->musical ? "Hello dolly!" : "Death Trap!"
> 
> as long as the field "musical" exists, all is well.
> 
> -- Steve
Aha,  i see.
Thanks very much for your explanation.
Better print fat is :   
TP_printk("mm=%p, scan_pfn=%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
               __entry->mm,
		__entry->pfn == (-1UL) ? "(null)" :  itoa(buff,  __entry->pin, 10), …..)

is this possible ?

Thanks









^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-17  3:58                     ` yalin wang
  0 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-17  3:58 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Vlastimil Babka, Ingo Molnar, Andrew Morton, Ebru Akagunduz,
	Rik van Riel, Kirill A. Shutemov, jmarchan, mgorman, willy,
	linux-kernel, linux-mm


> On Nov 17, 2015, at 10:43, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Tue, 17 Nov 2015 10:21:47 +0800
> yalin wang <yalin.wang2010@gmail.com> wrote:
> 
> 
>> i have not tried ,
>> just a question,
>> if you print a %s , but don’t call trace_define_field() do define this string in
>> __entry ,  how does user space perf tool to get this string info and print it ?
>> i am curious ..
>> i can try this when i have time.  and report to you .
> 
> Because the print_fmt has nothing to do with the fields. You can have
> as your print_fmt as:
> 
> 	TP_printk("Message = %s", "hello dolly!")
> 
> And both userspace and the kernel with process that correctly (if I got
> string processing working in userspace, which I believe I do). The
> string is processed, it's not dependent on TP_STRUCT__entry() unless it
> references a field there. Which can also be used too:
> 
> 	TP_printk("Message = %s", __entry->musical ? "Hello dolly!" :
> 			"Death Trap!")
> 
> userspace will see in the entry:
> 
> print_fmt: "Message = %s", REC->musical ? "Hello dolly!" : "Death Trap!"
> 
> as long as the field "musical" exists, all is well.
> 
> -- Steve
Aha,  i see.
Thanks very much for your explanation.
Better print fat is :   
TP_printk("mm=%p, scan_pfn=%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
               __entry->mm,
		__entry->pfn == (-1UL) ? "(null)" :  itoa(buff,  __entry->pin, 10), …..)

is this possible ?

Thanks








--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-17  3:58                     ` yalin wang
@ 2015-11-17  7:43                       ` Vlastimil Babka
  -1 siblings, 0 replies; 30+ messages in thread
From: Vlastimil Babka @ 2015-11-17  7:43 UTC (permalink / raw)
  To: yalin wang, Steven Rostedt
  Cc: Ingo Molnar, Andrew Morton, Ebru Akagunduz, Rik van Riel,
	Kirill A. Shutemov, jmarchan, mgorman, willy, linux-kernel,
	linux-mm

On 17.11.2015 4:58, yalin wang wrote:
> 
>> On Nov 17, 2015, at 10:43, Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>> On Tue, 17 Nov 2015 10:21:47 +0800
>> yalin wang <yalin.wang2010@gmail.com> wrote:
>>
>>
>>
>> Because the print_fmt has nothing to do with the fields. You can have
>> as your print_fmt as:
>>
>> 	TP_printk("Message = %s", "hello dolly!")
>>
>> And both userspace and the kernel with process that correctly (if I got
>> string processing working in userspace, which I believe I do). The
>> string is processed, it's not dependent on TP_STRUCT__entry() unless it
>> references a field there. Which can also be used too:
>>
>> 	TP_printk("Message = %s", __entry->musical ? "Hello dolly!" :
>> 			"Death Trap!")
>>
>> userspace will see in the entry:
>>
>> print_fmt: "Message = %s", REC->musical ? "Hello dolly!" : "Death Trap!"
>>
>> as long as the field "musical" exists, all is well.
>>
>> -- Steve
> Aha,  i see.
> Thanks very much for your explanation.
> Better print fat is :   
> TP_printk("mm=%p, scan_pfn=%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
>                __entry->mm,
> 		__entry->pfn == (-1UL) ? "(null)" :  itoa(buff,  __entry->pin, 10), …..)
> 
> is this possible ?

I doubt so.

Why don't we just do (with %lx):
 __entry->pfn != -1UL ? __entry->pfn : 0,

Status already tells us that it's not a real pfn 0 (which I doubt would be
userspace-mapped and thus reachable by khugepaged anyway?).
Also it's what some other tracepoints do, see e.g. mm_page class in
include/trace/events/kmem.h.

> Thanks
> 
> 
> 
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-17  7:43                       ` Vlastimil Babka
  0 siblings, 0 replies; 30+ messages in thread
From: Vlastimil Babka @ 2015-11-17  7:43 UTC (permalink / raw)
  To: yalin wang, Steven Rostedt
  Cc: Ingo Molnar, Andrew Morton, Ebru Akagunduz, Rik van Riel,
	Kirill A. Shutemov, jmarchan, mgorman, willy, linux-kernel,
	linux-mm

On 17.11.2015 4:58, yalin wang wrote:
> 
>> On Nov 17, 2015, at 10:43, Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>> On Tue, 17 Nov 2015 10:21:47 +0800
>> yalin wang <yalin.wang2010@gmail.com> wrote:
>>
>>
>>
>> Because the print_fmt has nothing to do with the fields. You can have
>> as your print_fmt as:
>>
>> 	TP_printk("Message = %s", "hello dolly!")
>>
>> And both userspace and the kernel with process that correctly (if I got
>> string processing working in userspace, which I believe I do). The
>> string is processed, it's not dependent on TP_STRUCT__entry() unless it
>> references a field there. Which can also be used too:
>>
>> 	TP_printk("Message = %s", __entry->musical ? "Hello dolly!" :
>> 			"Death Trap!")
>>
>> userspace will see in the entry:
>>
>> print_fmt: "Message = %s", REC->musical ? "Hello dolly!" : "Death Trap!"
>>
>> as long as the field "musical" exists, all is well.
>>
>> -- Steve
> Aha,  i see.
> Thanks very much for your explanation.
> Better print fat is :   
> TP_printk("mm=%p, scan_pfn=%s, writable=%d, referenced=%d, none_or_zero=%d, status=%s, unmapped=%d",
>                __entry->mm,
> 		__entry->pfn == (-1UL) ? "(null)" :  itoa(buff,  __entry->pin, 10), a?|..)
> 
> is this possible ?

I doubt so.

Why don't we just do (with %lx):
 __entry->pfn != -1UL ? __entry->pfn : 0,

Status already tells us that it's not a real pfn 0 (which I doubt would be
userspace-mapped and thus reachable by khugepaged anyway?).
Also it's what some other tracepoints do, see e.g. mm_page class in
include/trace/events/kmem.h.

> Thanks
> 
> 
> 
> 
> 
> 
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
  2015-11-13  8:41 ` Hillf Danton
@ 2015-11-13 10:24   ` yalin wang
  -1 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-13 10:24 UTC (permalink / raw)
  To: Hillf Danton
  Cc: Steven Rostedt, Ingo Molnar, Andrew Morton, Rik van Riel,
	Kirill A. Shutemov, Vlastimil Babka, Mel Gorman, linux-kernel,
	linux-mm


> On Nov 13, 2015, at 16:41, Hillf Danton <hillf.zj@alibaba-inc.com> wrote:
> 
>> 
>> Instead of the condition, we could have:
>> 
>> 	__entry->pfn = page ? page_to_pfn(page) : -1;
>> 
>> 
>> But if there's no reason to do the tracepoint if page is NULL, then
>> this patch is fine. I'm just throwing out this idea.
>> 
> we trace only if page is valid
> 
> --- linux-next/mm/huge_memory.c	Fri Nov 13 16:00:22 2015
> +++ b/mm/huge_memory.c	Fri Nov 13 16:26:19 2015
> @@ -1987,7 +1987,8 @@ static int __collapse_huge_page_isolate(
> 
> out:
> 	release_pte_pages(pte, _pte);
> -	trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
> +	if (page)
> +		trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
> 					    referenced, writable, result);
> 	return 0;
> }
> —
> 
my V4  patch move  if (!page)  into trace function,
so that we don’t need call page_to_fn()  if the trace if disabled .
more efficient  .
Thanks



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-13 10:24   ` yalin wang
  0 siblings, 0 replies; 30+ messages in thread
From: yalin wang @ 2015-11-13 10:24 UTC (permalink / raw)
  To: Hillf Danton
  Cc: Steven Rostedt, Ingo Molnar, Andrew Morton, Rik van Riel,
	Kirill A. Shutemov, Vlastimil Babka, Mel Gorman, linux-kernel,
	linux-mm


> On Nov 13, 2015, at 16:41, Hillf Danton <hillf.zj@alibaba-inc.com> wrote:
> 
>> 
>> Instead of the condition, we could have:
>> 
>> 	__entry->pfn = page ? page_to_pfn(page) : -1;
>> 
>> 
>> But if there's no reason to do the tracepoint if page is NULL, then
>> this patch is fine. I'm just throwing out this idea.
>> 
> we trace only if page is valid
> 
> --- linux-next/mm/huge_memory.c	Fri Nov 13 16:00:22 2015
> +++ b/mm/huge_memory.c	Fri Nov 13 16:26:19 2015
> @@ -1987,7 +1987,8 @@ static int __collapse_huge_page_isolate(
> 
> out:
> 	release_pte_pages(pte, _pte);
> -	trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
> +	if (page)
> +		trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
> 					    referenced, writable, result);
> 	return 0;
> }
> —
> 
my V4  patch move  if (!page)  into trace function,
so that we don’t need call page_to_fn()  if the trace if disabled .
more efficient  .
Thanks


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-13  8:41 ` Hillf Danton
  0 siblings, 0 replies; 30+ messages in thread
From: Hillf Danton @ 2015-11-13  8:41 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: 'yalin wang',
	Ingo Molnar, Andrew Morton, Rik van Riel, Kirill A. Shutemov,
	Vlastimil Babka, Mel Gorman, linux-kernel, linux-mm

> 
> Instead of the condition, we could have:
> 
> 	__entry->pfn = page ? page_to_pfn(page) : -1;
> 
> 
> But if there's no reason to do the tracepoint if page is NULL, then
> this patch is fine. I'm just throwing out this idea.
> 
we trace only if page is valid

--- linux-next/mm/huge_memory.c	Fri Nov 13 16:00:22 2015
+++ b/mm/huge_memory.c	Fri Nov 13 16:26:19 2015
@@ -1987,7 +1987,8 @@ static int __collapse_huge_page_isolate(
 
 out:
 	release_pte_pages(pte, _pte);
-	trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
+	if (page)
+		trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
 					    referenced, writable, result);
 	return 0;
 }
--




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH V4] mm: fix kernel crash in khugepaged thread
@ 2015-11-13  8:41 ` Hillf Danton
  0 siblings, 0 replies; 30+ messages in thread
From: Hillf Danton @ 2015-11-13  8:41 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: 'yalin wang',
	Ingo Molnar, Andrew Morton, Rik van Riel, Kirill A. Shutemov,
	Vlastimil Babka, Mel Gorman, linux-kernel, linux-mm

> 
> Instead of the condition, we could have:
> 
> 	__entry->pfn = page ? page_to_pfn(page) : -1;
> 
> 
> But if there's no reason to do the tracepoint if page is NULL, then
> this patch is fine. I'm just throwing out this idea.
> 
we trace only if page is valid

--- linux-next/mm/huge_memory.c	Fri Nov 13 16:00:22 2015
+++ b/mm/huge_memory.c	Fri Nov 13 16:26:19 2015
@@ -1987,7 +1987,8 @@ static int __collapse_huge_page_isolate(
 
 out:
 	release_pte_pages(pte, _pte);
-	trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
+	if (page)
+		trace_mm_collapse_huge_page_isolate(page_to_pfn(page), none_or_zero,
 					    referenced, writable, result);
 	return 0;
 }
--



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2015-11-17  7:43 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-12  8:21 [PATCH V4] mm: fix kernel crash in khugepaged thread yalin wang
2015-11-12  8:21 ` yalin wang
2015-11-12 14:29 ` Steven Rostedt
2015-11-12 14:29   ` Steven Rostedt
2015-11-13 10:47   ` Vlastimil Babka
2015-11-13 10:47     ` Vlastimil Babka
2015-11-13 11:54     ` yalin wang
2015-11-13 11:54       ` yalin wang
2015-11-13 14:01       ` Steven Rostedt
2015-11-13 14:01         ` Steven Rostedt
2015-11-16  1:35         ` yalin wang
2015-11-16  1:35           ` yalin wang
2015-11-16 10:16           ` Vlastimil Babka
2015-11-16 10:16             ` Vlastimil Babka
2015-11-16 14:25             ` Steven Rostedt
2015-11-16 14:25               ` Steven Rostedt
2015-11-17  2:21               ` yalin wang
2015-11-17  2:21                 ` yalin wang
2015-11-17  2:43                 ` Steven Rostedt
2015-11-17  2:43                   ` Steven Rostedt
2015-11-17  3:58                   ` yalin wang
2015-11-17  3:58                     ` yalin wang
2015-11-17  7:43                     ` Vlastimil Babka
2015-11-17  7:43                       ` Vlastimil Babka
2015-11-16 14:22           ` Steven Rostedt
2015-11-16 14:22             ` Steven Rostedt
2015-11-13  8:41 Hillf Danton
2015-11-13  8:41 ` Hillf Danton
2015-11-13 10:24 ` yalin wang
2015-11-13 10:24   ` yalin wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.