[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3fba1c4f-5c22-496b-a5cb-ffa74be9fe6c@suse.cz>
Date: Thu, 17 Oct 2024 11:20:55 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: jaewon31.kim@...sung.com, "sj@...nel.org" <sj@...nel.org>,
"minchan@...nel.org" <minchan@...nel.org>,
"kaleshsingh@...gle.com" <kaleshsingh@...gle.com>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
Cc: "linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jaewon31.kim@...il.com" <jaewon31.kim@...il.com>
Subject: Re: [PATCH v2] vmscan: add a vmscan event for reclaim_pages
On 10/16/24 15:24, 김재원 wrote:
>>On 10/11/24 14:49, Jaewon Kim wrote:
>>> The reclaim_folio_list uses a dummy reclaim_stat and is not being
>>> used. To know the memory stat, add a new trace event. This is useful how
>>> how many pages are not reclaimed or why.
>>>
>>> This is an example.
>>> mm_vmscan_reclaim_pages: nid=0 nr_scanned=112 nr_reclaimed=112 nr_dirty=0 nr_writeback=0 nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=0 nr_ref_keep=0 nr_unmap_fail=0
>>>
>>> Currenlty reclaim_folio_list is only called by reclaim_pages, and
>>> reclaim_pages is used by damon and madvise. In the latest Android,
>>> reclaim_pages is also used by shmem to reclaim all pages in a
>>> address_space.
>>>
>>> Signed-off-by: Jaewon Kim <jaewon31.kim@...sung.com>
>>> ---
>>> v2: remove reclaim_stat_add function and call the trace on each node
>>> v1: introduce a new trace event
>>> ---
>>> include/trace/events/vmscan.h | 45 +++++++++++++++++++++++++++++++++++
>>> mm/vmscan.c | 16 +++++++++----
>>> 2 files changed, 56 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
>>> index 1a488c30afa5..490958fa10de 100644
>>> --- a/include/trace/events/vmscan.h
>>> +++ b/include/trace/events/vmscan.h
>>> @@ -346,6 +346,51 @@ TRACE_EVENT(mm_vmscan_write_folio,
>>> show_reclaim_flags(__entry->reclaim_flags))
>>> );
>>>
>>> +TRACE_EVENT(mm_vmscan_reclaim_pages,
>>> +
>>> + TP_PROTO(int nid,
>>> + unsigned long nr_scanned, unsigned long nr_reclaimed,
>>> + struct reclaim_stat *stat),
>>> +
>>> + TP_ARGS(nid, nr_scanned, nr_reclaimed, stat),
>>> +
>>> + TP_STRUCT__entry(
>>> + __field(int, nid)
>>> + __field(unsigned long, nr_scanned)
>>> + __field(unsigned long, nr_reclaimed)
>>> + __field(unsigned long, nr_dirty)
>>> + __field(unsigned long, nr_writeback)
>>> + __field(unsigned long, nr_congested)
>>> + __field(unsigned long, nr_immediate)
>>> + __field(unsigned int, nr_activate0)
>>> + __field(unsigned int, nr_activate1)
>>> + __field(unsigned long, nr_ref_keep)
>>> + __field(unsigned long, nr_unmap_fail)
>>> + ),
>>> +
>>> + TP_fast_assign(
>>> + __entry->nid = nid;
>>> + __entry->nr_scanned = nr_scanned;
>>> + __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;
>>> + ),
>>> +
>>> + TP_printk("nid=%d nr_scanned=%ld 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",
>>> + __entry->nid,
>>> + __entry->nr_scanned, __entry->nr_reclaimed,
>>> + __entry->nr_dirty, __entry->nr_writeback,
>>> + __entry->nr_congested, __entry->nr_immediate,
>>> + __entry->nr_activate0, __entry->nr_activate1,
>>> + __entry->nr_ref_keep, __entry->nr_unmap_fail)
>>> +);
>>> +
>>> TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
>>>
>>> TP_PROTO(int nid,
>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>> index 749cdc110c74..0c2c36bf4c5a 100644
>>> --- a/mm/vmscan.c
>>> +++ b/mm/vmscan.c
>>> @@ -2126,9 +2126,10 @@ static void shrink_active_list(unsigned long nr_to_scan,
>>> }
>>>
>>> static unsigned int reclaim_folio_list(struct list_head *folio_list,
>>> + unsigned int nr_scanned,
>>> struct pglist_data *pgdat)
>>> {
>>> - struct reclaim_stat dummy_stat;
>>> + struct reclaim_stat stat;
>>> unsigned int nr_reclaimed;
>>> struct folio *folio;
>>> struct scan_control sc = {
>>> @@ -2139,12 +2140,13 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list,
>>> .no_demotion = 1,
>>> };
>>>
>>> - nr_reclaimed = shrink_folio_list(folio_list, pgdat, &sc, &dummy_stat, true);
>>> + nr_reclaimed = shrink_folio_list(folio_list, pgdat, &sc, &stat, true);
>>> while (!list_empty(folio_list)) {
>>> folio = lru_to_folio(folio_list);
>>> list_del(&folio->lru);
>>> folio_putback_lru(folio);
>>> }
>>> + trace_mm_vmscan_reclaim_pages(pgdat->node_id, nr_scanned, nr_reclaimed, &stat);
>>
>>Why is the new calculation of nr_scanned needed? Could you just take a delta
>>of sc->nr_scanned, i.e. after - before calling shrink_folio_list() ?
>
> Oh correct.
>
> Except the case of (!folio_trylock(folio)), shrink_folio_list would count on sc->nr_scanned.
> I don't understand why we do not count this lock case though.
Guess it means we couldn't really scan it. Should be probably rare anyway.
Powered by blists - more mailing lists