* [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
@ 2018-03-22 16:10 Steven Rostedt
2018-03-22 20:03 ` David Rientjes
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Steven Rostedt @ 2018-03-22 16:10 UTC (permalink / raw)
To: LKML, linux-mm
Cc: Michal Hocko, Mel Gorman, Vlastimil Babka, Andrew Morton,
Linus Torvalds, Alexei Starovoitov
The trace event trace_mm_vmscan_lru_shrink_inactive() currently has 12
parameters! Seven of them are from the reclaim_stat structure. This
structure is currently local to mm/vmscan.c. By moving it to the global
vmstat.h header, we can also reference it from the vmscan tracepoints. In
moving it, it brings down the overhead of passing so many arguments to the
trace event. In the future, we may limit the number of arguments that a
trace event may pass (ideally just 6, but more realistically it may be 8).
Before this patch, the code to call the trace event is this:
0f 83 aa fe ff ff jae ffffffff811e6261 <shrink_inactive_list+0x1e1>
48 8b 45 a0 mov -0x60(%rbp),%rax
45 8b 64 24 20 mov 0x20(%r12),%r12d
44 8b 6d d4 mov -0x2c(%rbp),%r13d
8b 4d d0 mov -0x30(%rbp),%ecx
44 8b 75 cc mov -0x34(%rbp),%r14d
44 8b 7d c8 mov -0x38(%rbp),%r15d
48 89 45 90 mov %rax,-0x70(%rbp)
8b 83 b8 fe ff ff mov -0x148(%rbx),%eax
8b 55 c0 mov -0x40(%rbp),%edx
8b 7d c4 mov -0x3c(%rbp),%edi
8b 75 b8 mov -0x48(%rbp),%esi
89 45 80 mov %eax,-0x80(%rbp)
65 ff 05 e4 f7 e2 7e incl %gs:0x7ee2f7e4(%rip) # 15bd0 <__preempt_count>
48 8b 05 75 5b 13 01 mov 0x1135b75(%rip),%rax # ffffffff8231bf68 <__tracepoint_mm_vmscan_lru_shrink_inactive+0x28>
48 85 c0 test %rax,%rax
74 72 je ffffffff811e646a <shrink_inactive_list+0x3ea>
48 89 c3 mov %rax,%rbx
4c 8b 10 mov (%rax),%r10
89 f8 mov %edi,%eax
48 89 85 68 ff ff ff mov %rax,-0x98(%rbp)
89 f0 mov %esi,%eax
48 89 85 60 ff ff ff mov %rax,-0xa0(%rbp)
89 c8 mov %ecx,%eax
48 89 85 78 ff ff ff mov %rax,-0x88(%rbp)
89 d0 mov %edx,%eax
48 89 85 70 ff ff ff mov %rax,-0x90(%rbp)
8b 45 8c mov -0x74(%rbp),%eax
48 8b 7b 08 mov 0x8(%rbx),%rdi
48 83 c3 18 add $0x18,%rbx
50 push %rax
41 54 push %r12
41 55 push %r13
ff b5 78 ff ff ff pushq -0x88(%rbp)
41 56 push %r14
41 57 push %r15
ff b5 70 ff ff ff pushq -0x90(%rbp)
4c 8b 8d 68 ff ff ff mov -0x98(%rbp),%r9
4c 8b 85 60 ff ff ff mov -0xa0(%rbp),%r8
48 8b 4d 98 mov -0x68(%rbp),%rcx
48 8b 55 90 mov -0x70(%rbp),%rdx
8b 75 80 mov -0x80(%rbp),%esi
41 ff d2 callq *%r10
After the patch:
0f 83 a8 fe ff ff jae ffffffff811e626d <shrink_inactive_list+0x1cd>
8b 9b b8 fe ff ff mov -0x148(%rbx),%ebx
45 8b 64 24 20 mov 0x20(%r12),%r12d
4c 8b 6d a0 mov -0x60(%rbp),%r13
65 ff 05 f5 f7 e2 7e incl %gs:0x7ee2f7f5(%rip) # 15bd0 <__preempt_count>
4c 8b 35 86 5b 13 01 mov 0x1135b86(%rip),%r14 # ffffffff8231bf68 <__tracepoint_mm_vmscan_lru_shrink_inactive+0x28>
4d 85 f6 test %r14,%r14
74 2a je ffffffff811e6411 <shrink_inactive_list+0x371>
49 8b 06 mov (%r14),%rax
8b 4d 8c mov -0x74(%rbp),%ecx
49 8b 7e 08 mov 0x8(%r14),%rdi
49 83 c6 18 add $0x18,%r14
4c 89 ea mov %r13,%rdx
45 89 e1 mov %r12d,%r9d
4c 8d 45 b8 lea -0x48(%rbp),%r8
89 de mov %ebx,%esi
51 push %rcx
48 8b 4d 98 mov -0x68(%rbp),%rcx
ff d0 callq *%rax
Link: http://lkml.kernel.org/r/2559d7cb-ec60-1200-2362-04fa34fd02bb@fb.com
Reported-by: Alexei Starovoitov <ast@fb.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
include/linux/vmstat.h | 11 +++++++++++
include/trace/events/vmscan.h | 24 +++++++++---------------
mm/vmscan.c | 18 +-----------------
3 files changed, 21 insertions(+), 32 deletions(-)
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index a4c2317d8b9f..f25cef84b41d 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -20,6 +20,17 @@ extern int sysctl_vm_numa_stat_handler(struct ctl_table *table,
int write, void __user *buffer, size_t *length, loff_t *ppos);
#endif
+struct reclaim_stat {
+ unsigned nr_dirty;
+ unsigned nr_unqueued_dirty;
+ unsigned nr_congested;
+ unsigned nr_writeback;
+ unsigned nr_immediate;
+ unsigned nr_activate;
+ unsigned nr_ref_keep;
+ unsigned nr_unmap_fail;
+};
+
#ifdef CONFIG_VM_EVENT_COUNTERS
/*
* Light weight per cpu counter implementation.
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index e0b8b9173e1c..5a7435296d89 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -343,15 +343,9 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
TP_PROTO(int nid,
unsigned long nr_scanned, unsigned long nr_reclaimed,
- unsigned long nr_dirty, unsigned long nr_writeback,
- unsigned long nr_congested, unsigned long nr_immediate,
- unsigned long nr_activate, unsigned long nr_ref_keep,
- unsigned long nr_unmap_fail,
- int priority, int file),
+ struct reclaim_stat *stat, int priority, int file),
- TP_ARGS(nid, nr_scanned, nr_reclaimed, nr_dirty, nr_writeback,
- nr_congested, nr_immediate, nr_activate, nr_ref_keep,
- nr_unmap_fail, priority, file),
+ TP_ARGS(nid, nr_scanned, nr_reclaimed, stat, priority, file),
TP_STRUCT__entry(
__field(int, nid)
@@ -372,13 +366,13 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
__entry->nid = nid;
__entry->nr_scanned = nr_scanned;
__entry->nr_reclaimed = nr_reclaimed;
- __entry->nr_dirty = nr_dirty;
- __entry->nr_writeback = nr_writeback;
- __entry->nr_congested = nr_congested;
- __entry->nr_immediate = nr_immediate;
- __entry->nr_activate = nr_activate;
- __entry->nr_ref_keep = nr_ref_keep;
- __entry->nr_unmap_fail = nr_unmap_fail;
+ __entry->nr_dirty = stat->nr_dirty;
+ __entry->nr_writeback = stat->nr_writeback;
+ __entry->nr_congested = stat->nr_congested;
+ __entry->nr_immediate = stat->nr_immediate;
+ __entry->nr_activate = stat->nr_activate;
+ __entry->nr_ref_keep = stat->nr_ref_keep;
+ __entry->nr_unmap_fail = stat->nr_unmap_fail;
__entry->priority = priority;
__entry->reclaim_flags = trace_shrink_flags(file);
),
diff --git a/mm/vmscan.c b/mm/vmscan.c
index bee53495a829..aaeb86642095 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -865,17 +865,6 @@ static void page_check_dirty_writeback(struct page *page,
mapping->a_ops->is_dirty_writeback(page, dirty, writeback);
}
-struct reclaim_stat {
- unsigned nr_dirty;
- unsigned nr_unqueued_dirty;
- unsigned nr_congested;
- unsigned nr_writeback;
- unsigned nr_immediate;
- unsigned nr_activate;
- unsigned nr_ref_keep;
- unsigned nr_unmap_fail;
-};
-
/*
* shrink_page_list() returns the number of reclaimed pages
*/
@@ -1828,12 +1817,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
wait_iff_congested(pgdat, BLK_RW_ASYNC, HZ/10);
trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id,
- nr_scanned, nr_reclaimed,
- stat.nr_dirty, stat.nr_writeback,
- stat.nr_congested, stat.nr_immediate,
- stat.nr_activate, stat.nr_ref_keep,
- stat.nr_unmap_fail,
- sc->priority, file);
+ nr_scanned, nr_reclaimed, &stat, sc->priority, file);
return nr_reclaimed;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
2018-03-22 16:10 [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event Steven Rostedt
@ 2018-03-22 20:03 ` David Rientjes
2018-03-22 21:10 ` Andrew Morton
2018-03-23 13:42 ` Michal Hocko
2 siblings, 0 replies; 9+ messages in thread
From: David Rientjes @ 2018-03-22 20:03 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, linux-mm, Michal Hocko, Mel Gorman, Vlastimil Babka,
Andrew Morton, Linus Torvalds, Alexei Starovoitov
On Thu, 22 Mar 2018, Steven Rostedt wrote:
>
> The trace event trace_mm_vmscan_lru_shrink_inactive() currently has 12
> parameters! Seven of them are from the reclaim_stat structure. This
> structure is currently local to mm/vmscan.c. By moving it to the global
> vmstat.h header, we can also reference it from the vmscan tracepoints. In
> moving it, it brings down the overhead of passing so many arguments to the
> trace event. In the future, we may limit the number of arguments that a
> trace event may pass (ideally just 6, but more realistically it may be 8).
>
> Before this patch, the code to call the trace event is this:
>
> 0f 83 aa fe ff ff jae ffffffff811e6261 <shrink_inactive_list+0x1e1>
> 48 8b 45 a0 mov -0x60(%rbp),%rax
> 45 8b 64 24 20 mov 0x20(%r12),%r12d
> 44 8b 6d d4 mov -0x2c(%rbp),%r13d
> 8b 4d d0 mov -0x30(%rbp),%ecx
> 44 8b 75 cc mov -0x34(%rbp),%r14d
> 44 8b 7d c8 mov -0x38(%rbp),%r15d
> 48 89 45 90 mov %rax,-0x70(%rbp)
> 8b 83 b8 fe ff ff mov -0x148(%rbx),%eax
> 8b 55 c0 mov -0x40(%rbp),%edx
> 8b 7d c4 mov -0x3c(%rbp),%edi
> 8b 75 b8 mov -0x48(%rbp),%esi
> 89 45 80 mov %eax,-0x80(%rbp)
> 65 ff 05 e4 f7 e2 7e incl %gs:0x7ee2f7e4(%rip) # 15bd0 <__preempt_count>
> 48 8b 05 75 5b 13 01 mov 0x1135b75(%rip),%rax # ffffffff8231bf68 <__tracepoint_mm_vmscan_lru_shrink_inactive+0x28>
> 48 85 c0 test %rax,%rax
> 74 72 je ffffffff811e646a <shrink_inactive_list+0x3ea>
> 48 89 c3 mov %rax,%rbx
> 4c 8b 10 mov (%rax),%r10
> 89 f8 mov %edi,%eax
> 48 89 85 68 ff ff ff mov %rax,-0x98(%rbp)
> 89 f0 mov %esi,%eax
> 48 89 85 60 ff ff ff mov %rax,-0xa0(%rbp)
> 89 c8 mov %ecx,%eax
> 48 89 85 78 ff ff ff mov %rax,-0x88(%rbp)
> 89 d0 mov %edx,%eax
> 48 89 85 70 ff ff ff mov %rax,-0x90(%rbp)
> 8b 45 8c mov -0x74(%rbp),%eax
> 48 8b 7b 08 mov 0x8(%rbx),%rdi
> 48 83 c3 18 add $0x18,%rbx
> 50 push %rax
> 41 54 push %r12
> 41 55 push %r13
> ff b5 78 ff ff ff pushq -0x88(%rbp)
> 41 56 push %r14
> 41 57 push %r15
> ff b5 70 ff ff ff pushq -0x90(%rbp)
> 4c 8b 8d 68 ff ff ff mov -0x98(%rbp),%r9
> 4c 8b 85 60 ff ff ff mov -0xa0(%rbp),%r8
> 48 8b 4d 98 mov -0x68(%rbp),%rcx
> 48 8b 55 90 mov -0x70(%rbp),%rdx
> 8b 75 80 mov -0x80(%rbp),%esi
> 41 ff d2 callq *%r10
>
> After the patch:
>
> 0f 83 a8 fe ff ff jae ffffffff811e626d <shrink_inactive_list+0x1cd>
> 8b 9b b8 fe ff ff mov -0x148(%rbx),%ebx
> 45 8b 64 24 20 mov 0x20(%r12),%r12d
> 4c 8b 6d a0 mov -0x60(%rbp),%r13
> 65 ff 05 f5 f7 e2 7e incl %gs:0x7ee2f7f5(%rip) # 15bd0 <__preempt_count>
> 4c 8b 35 86 5b 13 01 mov 0x1135b86(%rip),%r14 # ffffffff8231bf68 <__tracepoint_mm_vmscan_lru_shrink_inactive+0x28>
> 4d 85 f6 test %r14,%r14
> 74 2a je ffffffff811e6411 <shrink_inactive_list+0x371>
> 49 8b 06 mov (%r14),%rax
> 8b 4d 8c mov -0x74(%rbp),%ecx
> 49 8b 7e 08 mov 0x8(%r14),%rdi
> 49 83 c6 18 add $0x18,%r14
> 4c 89 ea mov %r13,%rdx
> 45 89 e1 mov %r12d,%r9d
> 4c 8d 45 b8 lea -0x48(%rbp),%r8
> 89 de mov %ebx,%esi
> 51 push %rcx
> 48 8b 4d 98 mov -0x68(%rbp),%rcx
> ff d0 callq *%rax
>
> Link: http://lkml.kernel.org/r/2559d7cb-ec60-1200-2362-04fa34fd02bb@fb.com
>
> Reported-by: Alexei Starovoitov <ast@fb.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: David Rientjes <rientjes@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
2018-03-22 16:10 [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event Steven Rostedt
2018-03-22 20:03 ` David Rientjes
@ 2018-03-22 21:10 ` Andrew Morton
2018-03-22 21:21 ` Steven Rostedt
2018-03-23 15:19 ` Andrey Ryabinin
2018-03-23 13:42 ` Michal Hocko
2 siblings, 2 replies; 9+ messages in thread
From: Andrew Morton @ 2018-03-22 21:10 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, linux-mm, Michal Hocko, Mel Gorman, Vlastimil Babka,
Linus Torvalds, Alexei Starovoitov, Andrey Ryabinin
On Thu, 22 Mar 2018 12:10:03 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:
>
> The trace event trace_mm_vmscan_lru_shrink_inactive() currently has 12
> parameters! Seven of them are from the reclaim_stat structure. This
> structure is currently local to mm/vmscan.c. By moving it to the global
> vmstat.h header, we can also reference it from the vmscan tracepoints. In
> moving it, it brings down the overhead of passing so many arguments to the
> trace event. In the future, we may limit the number of arguments that a
> trace event may pass (ideally just 6, but more realistically it may be 8).
Unfortunately this is not a good time. Andrey's "mm/vmscan: replace
mm_vmscan_lru_shrink_inactive with shrink_page_list tracepoint" mucks
with this code quite a lot and that patch's series is undergoing review
at present, with a few issues yet unresolved.
I'll park your patch for now and if Andrey's series doesn't converge
soon I'll merge this and will ask Andrey to redo things.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
2018-03-22 21:10 ` Andrew Morton
@ 2018-03-22 21:21 ` Steven Rostedt
2018-03-23 15:19 ` Andrey Ryabinin
1 sibling, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2018-03-22 21:21 UTC (permalink / raw)
To: Andrew Morton
Cc: LKML, linux-mm, Michal Hocko, Mel Gorman, Vlastimil Babka,
Linus Torvalds, Alexei Starovoitov, Andrey Ryabinin
On Thu, 22 Mar 2018 14:10:22 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> Unfortunately this is not a good time. Andrey's "mm/vmscan: replace
> mm_vmscan_lru_shrink_inactive with shrink_page_list tracepoint" mucks
> with this code quite a lot and that patch's series is undergoing review
> at present, with a few issues yet unresolved.
>
> I'll park your patch for now and if Andrey's series doesn't converge
> soon I'll merge this and will ask Andrey to redo things.
No problem. I can easily update that patch on top, as it didn't take
much effort to write and test it. Just let me know if you do pull in
Andrey's work and I need to do the update.
Thanks!
-- Steve
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
2018-03-22 16:10 [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event Steven Rostedt
2018-03-22 20:03 ` David Rientjes
2018-03-22 21:10 ` Andrew Morton
@ 2018-03-23 13:42 ` Michal Hocko
2018-03-23 13:47 ` Steven Rostedt
2 siblings, 1 reply; 9+ messages in thread
From: Michal Hocko @ 2018-03-23 13:42 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, linux-mm, Mel Gorman, Vlastimil Babka, Andrew Morton,
Linus Torvalds, Alexei Starovoitov
On Thu 22-03-18 12:10:03, Steven Rostedt wrote:
>
> The trace event trace_mm_vmscan_lru_shrink_inactive() currently has 12
> parameters! Seven of them are from the reclaim_stat structure. This
> structure is currently local to mm/vmscan.c. By moving it to the global
> vmstat.h header, we can also reference it from the vmscan tracepoints. In
> moving it, it brings down the overhead of passing so many arguments to the
> trace event. In the future, we may limit the number of arguments that a
> trace event may pass (ideally just 6, but more realistically it may be 8).
Yes, the number of parameter is large. struct reclaim_stat is an
internal stuff so I didn't want to export it. I do not have strong
objections to add it somewhere tracing can find it though.
> Before this patch, the code to call the trace event is this:
>
> 0f 83 aa fe ff ff jae ffffffff811e6261 <shrink_inactive_list+0x1e1>
> 48 8b 45 a0 mov -0x60(%rbp),%rax
> 45 8b 64 24 20 mov 0x20(%r12),%r12d
> 44 8b 6d d4 mov -0x2c(%rbp),%r13d
> 8b 4d d0 mov -0x30(%rbp),%ecx
> 44 8b 75 cc mov -0x34(%rbp),%r14d
> 44 8b 7d c8 mov -0x38(%rbp),%r15d
> 48 89 45 90 mov %rax,-0x70(%rbp)
> 8b 83 b8 fe ff ff mov -0x148(%rbx),%eax
> 8b 55 c0 mov -0x40(%rbp),%edx
> 8b 7d c4 mov -0x3c(%rbp),%edi
> 8b 75 b8 mov -0x48(%rbp),%esi
> 89 45 80 mov %eax,-0x80(%rbp)
> 65 ff 05 e4 f7 e2 7e incl %gs:0x7ee2f7e4(%rip) # 15bd0 <__preempt_count>
> 48 8b 05 75 5b 13 01 mov 0x1135b75(%rip),%rax # ffffffff8231bf68 <__tracepoint_mm_vmscan_lru_shrink_inactive+0x28>
> 48 85 c0 test %rax,%rax
> 74 72 je ffffffff811e646a <shrink_inactive_list+0x3ea>
> 48 89 c3 mov %rax,%rbx
> 4c 8b 10 mov (%rax),%r10
> 89 f8 mov %edi,%eax
> 48 89 85 68 ff ff ff mov %rax,-0x98(%rbp)
> 89 f0 mov %esi,%eax
> 48 89 85 60 ff ff ff mov %rax,-0xa0(%rbp)
> 89 c8 mov %ecx,%eax
> 48 89 85 78 ff ff ff mov %rax,-0x88(%rbp)
> 89 d0 mov %edx,%eax
> 48 89 85 70 ff ff ff mov %rax,-0x90(%rbp)
> 8b 45 8c mov -0x74(%rbp),%eax
> 48 8b 7b 08 mov 0x8(%rbx),%rdi
> 48 83 c3 18 add $0x18,%rbx
> 50 push %rax
> 41 54 push %r12
> 41 55 push %r13
> ff b5 78 ff ff ff pushq -0x88(%rbp)
> 41 56 push %r14
> 41 57 push %r15
> ff b5 70 ff ff ff pushq -0x90(%rbp)
> 4c 8b 8d 68 ff ff ff mov -0x98(%rbp),%r9
> 4c 8b 85 60 ff ff ff mov -0xa0(%rbp),%r8
> 48 8b 4d 98 mov -0x68(%rbp),%rcx
> 48 8b 55 90 mov -0x70(%rbp),%rdx
> 8b 75 80 mov -0x80(%rbp),%esi
> 41 ff d2 callq *%r10
>
> After the patch:
>
> 0f 83 a8 fe ff ff jae ffffffff811e626d <shrink_inactive_list+0x1cd>
> 8b 9b b8 fe ff ff mov -0x148(%rbx),%ebx
> 45 8b 64 24 20 mov 0x20(%r12),%r12d
> 4c 8b 6d a0 mov -0x60(%rbp),%r13
> 65 ff 05 f5 f7 e2 7e incl %gs:0x7ee2f7f5(%rip) # 15bd0 <__preempt_count>
> 4c 8b 35 86 5b 13 01 mov 0x1135b86(%rip),%r14 # ffffffff8231bf68 <__tracepoint_mm_vmscan_lru_shrink_inactive+0x28>
> 4d 85 f6 test %r14,%r14
> 74 2a je ffffffff811e6411 <shrink_inactive_list+0x371>
> 49 8b 06 mov (%r14),%rax
> 8b 4d 8c mov -0x74(%rbp),%ecx
> 49 8b 7e 08 mov 0x8(%r14),%rdi
> 49 83 c6 18 add $0x18,%r14
> 4c 89 ea mov %r13,%rdx
> 45 89 e1 mov %r12d,%r9d
> 4c 8d 45 b8 lea -0x48(%rbp),%r8
> 89 de mov %ebx,%esi
> 51 push %rcx
> 48 8b 4d 98 mov -0x68(%rbp),%rcx
> ff d0 callq *%rax
>
> Link: http://lkml.kernel.org/r/2559d7cb-ec60-1200-2362-04fa34fd02bb@fb.com
>
> Reported-by: Alexei Starovoitov <ast@fb.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> include/linux/vmstat.h | 11 +++++++++++
> include/trace/events/vmscan.h | 24 +++++++++---------------
> mm/vmscan.c | 18 +-----------------
> 3 files changed, 21 insertions(+), 32 deletions(-)
>
> diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> index a4c2317d8b9f..f25cef84b41d 100644
> --- a/include/linux/vmstat.h
> +++ b/include/linux/vmstat.h
> @@ -20,6 +20,17 @@ extern int sysctl_vm_numa_stat_handler(struct ctl_table *table,
> int write, void __user *buffer, size_t *length, loff_t *ppos);
> #endif
>
> +struct reclaim_stat {
> + unsigned nr_dirty;
> + unsigned nr_unqueued_dirty;
> + unsigned nr_congested;
> + unsigned nr_writeback;
> + unsigned nr_immediate;
> + unsigned nr_activate;
> + unsigned nr_ref_keep;
> + unsigned nr_unmap_fail;
> +};
> +
> #ifdef CONFIG_VM_EVENT_COUNTERS
> /*
> * Light weight per cpu counter implementation.
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index e0b8b9173e1c..5a7435296d89 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -343,15 +343,9 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
>
> TP_PROTO(int nid,
> unsigned long nr_scanned, unsigned long nr_reclaimed,
> - unsigned long nr_dirty, unsigned long nr_writeback,
> - unsigned long nr_congested, unsigned long nr_immediate,
> - unsigned long nr_activate, unsigned long nr_ref_keep,
> - unsigned long nr_unmap_fail,
> - int priority, int file),
> + struct reclaim_stat *stat, int priority, int file),
>
> - TP_ARGS(nid, nr_scanned, nr_reclaimed, nr_dirty, nr_writeback,
> - nr_congested, nr_immediate, nr_activate, nr_ref_keep,
> - nr_unmap_fail, priority, file),
> + TP_ARGS(nid, nr_scanned, nr_reclaimed, stat, priority, file),
>
> TP_STRUCT__entry(
> __field(int, nid)
> @@ -372,13 +366,13 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
> __entry->nid = nid;
> __entry->nr_scanned = nr_scanned;
> __entry->nr_reclaimed = nr_reclaimed;
> - __entry->nr_dirty = nr_dirty;
> - __entry->nr_writeback = nr_writeback;
> - __entry->nr_congested = nr_congested;
> - __entry->nr_immediate = nr_immediate;
> - __entry->nr_activate = nr_activate;
> - __entry->nr_ref_keep = nr_ref_keep;
> - __entry->nr_unmap_fail = nr_unmap_fail;
> + __entry->nr_dirty = stat->nr_dirty;
> + __entry->nr_writeback = stat->nr_writeback;
> + __entry->nr_congested = stat->nr_congested;
> + __entry->nr_immediate = stat->nr_immediate;
> + __entry->nr_activate = stat->nr_activate;
> + __entry->nr_ref_keep = stat->nr_ref_keep;
> + __entry->nr_unmap_fail = stat->nr_unmap_fail;
> __entry->priority = priority;
> __entry->reclaim_flags = trace_shrink_flags(file);
> ),
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bee53495a829..aaeb86642095 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -865,17 +865,6 @@ static void page_check_dirty_writeback(struct page *page,
> mapping->a_ops->is_dirty_writeback(page, dirty, writeback);
> }
>
> -struct reclaim_stat {
> - unsigned nr_dirty;
> - unsigned nr_unqueued_dirty;
> - unsigned nr_congested;
> - unsigned nr_writeback;
> - unsigned nr_immediate;
> - unsigned nr_activate;
> - unsigned nr_ref_keep;
> - unsigned nr_unmap_fail;
> -};
> -
> /*
> * shrink_page_list() returns the number of reclaimed pages
> */
> @@ -1828,12 +1817,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
> wait_iff_congested(pgdat, BLK_RW_ASYNC, HZ/10);
>
> trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id,
> - nr_scanned, nr_reclaimed,
> - stat.nr_dirty, stat.nr_writeback,
> - stat.nr_congested, stat.nr_immediate,
> - stat.nr_activate, stat.nr_ref_keep,
> - stat.nr_unmap_fail,
> - sc->priority, file);
> + nr_scanned, nr_reclaimed, &stat, sc->priority, file);
> return nr_reclaimed;
> }
>
> --
> 2.13.6
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
2018-03-23 13:42 ` Michal Hocko
@ 2018-03-23 13:47 ` Steven Rostedt
2018-03-23 13:52 ` Michal Hocko
0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2018-03-23 13:47 UTC (permalink / raw)
To: Michal Hocko
Cc: LKML, linux-mm, Mel Gorman, Vlastimil Babka, Andrew Morton,
Linus Torvalds, Alexei Starovoitov
On Fri, 23 Mar 2018 14:42:00 +0100
Michal Hocko <mhocko@kernel.org> wrote:
> Yes, the number of parameter is large. struct reclaim_stat is an
> internal stuff so I didn't want to export it. I do not have strong
> objections to add it somewhere tracing can find it though.
The one solution is to pull the tracing file
include/trace/events/vmscan.h into mm/ and have a local header to store
the reclaim_stat structure that both vmscan.h and vmscan.c can
reference.
I'll make a patch once I hear which way Andrey's patches are going.
That way I don't need to do it twice.
-- Steve
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
2018-03-23 13:47 ` Steven Rostedt
@ 2018-03-23 13:52 ` Michal Hocko
2018-03-23 14:15 ` Steven Rostedt
0 siblings, 1 reply; 9+ messages in thread
From: Michal Hocko @ 2018-03-23 13:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, linux-mm, Mel Gorman, Vlastimil Babka, Andrew Morton,
Linus Torvalds, Alexei Starovoitov
On Fri 23-03-18 09:47:53, Steven Rostedt wrote:
> On Fri, 23 Mar 2018 14:42:00 +0100
> Michal Hocko <mhocko@kernel.org> wrote:
>
> > Yes, the number of parameter is large. struct reclaim_stat is an
> > internal stuff so I didn't want to export it. I do not have strong
> > objections to add it somewhere tracing can find it though.
>
> The one solution is to pull the tracing file
> include/trace/events/vmscan.h into mm/ and have a local header to store
> the reclaim_stat structure that both vmscan.h and vmscan.c can
> reference.
I guess we can live with the public definition as well.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
2018-03-23 13:52 ` Michal Hocko
@ 2018-03-23 14:15 ` Steven Rostedt
0 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2018-03-23 14:15 UTC (permalink / raw)
To: Michal Hocko
Cc: LKML, linux-mm, Mel Gorman, Vlastimil Babka, Andrew Morton,
Linus Torvalds, Alexei Starovoitov
On Fri, 23 Mar 2018 14:52:25 +0100
Michal Hocko <mhocko@kernel.org> wrote:
> On Fri 23-03-18 09:47:53, Steven Rostedt wrote:
> >
> > The one solution is to pull the tracing file
> > include/trace/events/vmscan.h into mm/ and have a local header to store
> > the reclaim_stat structure that both vmscan.h and vmscan.c can
> > reference.
>
> I guess we can live with the public definition as well.
>
Yes, that would be easier. Thanks!
Then if Andrey's patches are not pulled, then this patch will be good
to go.
-- Steve
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
2018-03-22 21:10 ` Andrew Morton
2018-03-22 21:21 ` Steven Rostedt
@ 2018-03-23 15:19 ` Andrey Ryabinin
1 sibling, 0 replies; 9+ messages in thread
From: Andrey Ryabinin @ 2018-03-23 15:19 UTC (permalink / raw)
To: Andrew Morton, Steven Rostedt
Cc: LKML, linux-mm, Michal Hocko, Mel Gorman, Vlastimil Babka,
Linus Torvalds, Alexei Starovoitov
On 03/23/2018 12:10 AM, Andrew Morton wrote:
> On Thu, 22 Mar 2018 12:10:03 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:
>
>>
>> The trace event trace_mm_vmscan_lru_shrink_inactive() currently has 12
>> parameters! Seven of them are from the reclaim_stat structure. This
>> structure is currently local to mm/vmscan.c. By moving it to the global
>> vmstat.h header, we can also reference it from the vmscan tracepoints. In
>> moving it, it brings down the overhead of passing so many arguments to the
>> trace event. In the future, we may limit the number of arguments that a
>> trace event may pass (ideally just 6, but more realistically it may be 8).
>
> Unfortunately this is not a good time. Andrey's "mm/vmscan: replace
> mm_vmscan_lru_shrink_inactive with shrink_page_list tracepoint" mucks
> with this code quite a lot and that patch's series is undergoing review
> at present, with a few issues yet unresolved.
I slightly reworked my patch series, so that patch "mm/vmscan: replace
mm_vmscan_lru_shrink_inactive with shrink_page_list tracepoint"
is not needed anymore. Replacing that tracepoint with less informative shrink_page_list
probably isn't a very good idea anyway.
So Steven's patch applies *almost* cleanly on top of my v2, nothing that 3-way merge can't handle:
# git am -3 mm-vmscan-tracing-Use-pointer-to-reclaim_stat-struct.patch
Applying: mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event
Using index info to reconstruct a base tree...
M include/trace/events/vmscan.h
M mm/vmscan.c
Falling back to patching base and 3-way merge...
Auto-merging mm/vmscan.c
Auto-merging include/trace/events/vmscan.h
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-03-23 15:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 16:10 [PATCH] mm, vmscan, tracing: Use pointer to reclaim_stat struct in trace event Steven Rostedt
2018-03-22 20:03 ` David Rientjes
2018-03-22 21:10 ` Andrew Morton
2018-03-22 21:21 ` Steven Rostedt
2018-03-23 15:19 ` Andrey Ryabinin
2018-03-23 13:42 ` Michal Hocko
2018-03-23 13:47 ` Steven Rostedt
2018-03-23 13:52 ` Michal Hocko
2018-03-23 14:15 ` Steven Rostedt
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.