Re: [PATCH v2] vmscan: add trace events for lru_gen
From: "T.J. Mercier" <tjmercier@google.com>
Date: 2023-09-23 15:26:18
Also in:
linux-mm, lkml
On Thu, Sep 21, 2023 at 7:27 PM 김재원 [off-list ref] wrote:
quoted
On Thu, 21 Sep 2023 09:12:30 -0700 "T.J. Mercier" [off-list ref] wrote:quoted
quoted
+ TP_fast_assign( + __entry->nid = nid; + __entry->nr_reclaimed = nr_reclaimed; + __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_activate0 = stat->nr_activate[0]; + __entry->nr_activate1 = stat->nr_activate[1]; + __entry->nr_ref_keep = stat->nr_ref_keep; + __entry->nr_unmap_fail = stat->nr_unmap_fail; + __entry->priority = priority; + __entry->reclaim_flags = trace_reclaim_flags(file); + ), + + TP_printk("nid=%d nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",Many of these values are unsigned so I think many of these format specifiers should be %lu instead of %ld.Hello T.J. Thank you for your comment As you expected I got this from the legacy lru trace. I've changed as you recommended. Actually I changed isolate_mode, too. Please let me know if this is not actually needed.
Great, looks good to me. Reviewed-by: T.J. Mercier <tjmercier@google.com>
quoted hunk ↗ jump to hunk
--- a/include/trace/events/vmscan.h +++ b/include/trace/events/vmscan.h@@ -367,7 +367,7 @@ TRACE_EVENT_CONDITION(mm_vmscan_lru_gen_scan, * classzone is previous name of the highest_zoneidx. * Reason not to change it is the ABI requirement of the tracepoint. */ - TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s", + TP_printk("isolate_mode=%u classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s", __entry->isolate_mode, __entry->highest_zoneidx, __entry->order,@@ -525,7 +525,7 @@ TRACE_EVENT(mm_vmscan_lru_gen_evict, __entry->reclaim_flags = trace_reclaim_flags(file); ), - TP_printk("nid=%d nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s", + TP_printk("nid=%d nr_reclaimed=%lu nr_dirty=%lu nr_writeback=%lu nr_congested=%lu nr_immediate=%lu nr_activate_anon=%u nr_activate_file=%u nr_ref_keep=%lu nr_unmap_fail=%lu priority=%d flags=%s", __entry->nid, __entry->nr_reclaimed, __entry->nr_dirty, __entry->nr_writeback, __entry->nr_congested, __entry->nr_immediate,quoted
Other than this, from the tracing POV: Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>Hello Steven I've appreciated your Reviewed-by, let me take this on my next v3 patch.quoted
-- Steve