From: 김재원 <jaewon31.kim@samsung.com>
To: Steven Rostedt <rostedt@goodmis.org>,
"T.J. Mercier" <tjmercier@google.com>
Cc: 김재원 <jaewon31.kim@samsung.com>,
"yuzhao@google.com" <yuzhao@google.com>,
"kaleshsingh@google.com" <kaleshsingh@google.com>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"vbabka@suse.cz" <vbabka@suse.cz>,
"hannes@cmpxchg.org" <hannes@cmpxchg.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-trace-kernel@vger.kernel.org"
<linux-trace-kernel@vger.kernel.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"jaewon31.kim@gmail.com" <jaewon31.kim@gmail.com>
Subject: RE: [PATCH v2] vmscan: add trace events for lru_gen
Date: Fri, 22 Sep 2023 11:27:46 +0900 [thread overview]
Message-ID: <20230922022746epcms1p323c50015241eb01311e7e10579fd4c6c@epcms1p3> (raw)
In-Reply-To: <20230921122244.1cbf2d57@gandalf.local.home>
>On Thu, 21 Sep 2023 09:12:30 -0700
>"T.J. Mercier" <tjmercier@google.com> wrote:
>
>> > + 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.
--- 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,
>
>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.
>
>-- Steve
next prev parent reply other threads:[~2023-09-22 2:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20230921061944epcas1p2f8a022282709da12840b33c0f5898b5f@epcas1p2.samsung.com>
2023-09-21 6:22 ` Jaewon Kim
2023-09-21 16:12 ` T.J. Mercier
2023-09-21 16:22 ` Steven Rostedt
[not found] ` <CGME20230921061944epcas1p2f8a022282709da12840b33c0f5898b5f@epcms1p3>
2023-09-22 2:27 ` 김재원 [this message]
2023-09-23 15:26 ` T.J. Mercier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230922022746epcms1p323c50015241eb01311e7e10579fd4c6c@epcms1p3 \
--to=jaewon31.kim@samsung.com \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=jaewon31.kim@gmail.com \
--cc=kaleshsingh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=tjmercier@google.com \
--cc=vbabka@suse.cz \
--cc=yuzhao@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox