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

Groups > linux.kernel > #1267711 > unrolled thread

[PATCH V4] mm: fix kernel crash in khugepaged thread

Back to article view | Back to linux.kernel

Contents

  [PATCH V4] mm: fix kernel crash in khugepaged thread yalin wang <yalin.wang2010@gmail.com> - 2015-11-12 09:30 +0100
    Re: [PATCH V4] mm: fix kernel crash in khugepaged thread Steven Rostedt <rostedt@goodmis.org> - 2015-11-12 15:30 +0100
      Re: [PATCH V4] mm: fix kernel crash in khugepaged thread Vlastimil Babka <vbabka@suse.cz> - 2015-11-13 11:50 +0100
        Re: [PATCH V4] mm: fix kernel crash in khugepaged thread yalin wang <yalin.wang2010@gmail.com> - 2015-11-13 13:00 +0100
          Re: [PATCH V4] mm: fix kernel crash in khugepaged thread Steven Rostedt <rostedt@goodmis.org> - 2015-11-13 15:10 +0100
            Re: [PATCH V4] mm: fix kernel crash in khugepaged thread yalin wang <yalin.wang2010@gmail.com> - 2015-11-16 02:40 +0100
              Re: [PATCH V4] mm: fix kernel crash in khugepaged thread Vlastimil Babka <vbabka@suse.cz> - 2015-11-16 11:20 +0100
                Re: [PATCH V4] mm: fix kernel crash in khugepaged thread Steven Rostedt <rostedt@goodmis.org> - 2015-11-16 15:30 +0100
                  Re: [PATCH V4] mm: fix kernel crash in khugepaged thread yalin wang <yalin.wang2010@gmail.com> - 2015-11-17 03:30 +0100
                    Re: [PATCH V4] mm: fix kernel crash in khugepaged thread Steven Rostedt <rostedt@goodmis.org> - 2015-11-17 03:50 +0100
                      Re: [PATCH V4] mm: fix kernel crash in khugepaged thread yalin wang <yalin.wang2010@gmail.com> - 2015-11-17 05:00 +0100
                        Re: [PATCH V4] mm: fix kernel crash in khugepaged thread Vlastimil Babka <vbabka@suse.cz> - 2015-11-17 08:50 +0100
              Re: [PATCH V4] mm: fix kernel crash in khugepaged thread Steven Rostedt <rostedt@goodmis.org> - 2015-11-16 15:30 +0100

#1267711 — [PATCH V4] mm: fix kernel crash in khugepaged thread

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1267973

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1268840

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1268887

> 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1268964

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269799

> 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269970

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
>

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

[toc] | [prev] | [next] | [standalone]

#1270224

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1270770

> 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1270777

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1270839

> 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1270935

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

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

[toc] | [prev] | [next] | [standalone]

#1270225

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web