lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241016132452epcms1p6884ae0197aa83731baa193fb5714b0d2@epcms1p6>
Date: Wed, 16 Oct 2024 22:24:52 +0900
From: 김재원 <jaewon31.kim@...sung.com>
To: Vlastimil Babka <vbabka@...e.cz>, 김재원
	<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/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.

Let me try to change as you suggested.
I think without 'after - before', we can just use sc->nr_scanned after shrink_folio_list.

Thank you

>
>>  
>>          return nr_reclaimed;
>>  }
>> @@ -2152,6 +2154,7 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list,
>>  unsigned long reclaim_pages(struct list_head *folio_list)
>>  {
>>          int nid;
>> +        unsigned int nr_scanned = 0;
>>          unsigned int nr_reclaimed = 0;
>>          LIST_HEAD(node_folio_list);
>>          unsigned int noreclaim_flag;
>> @@ -2168,15 +2171,18 @@ unsigned long reclaim_pages(struct list_head *folio_list)
>>                  if (nid == folio_nid(folio)) {
>>                          folio_clear_active(folio);
>>                          list_move(&folio->lru, &node_folio_list);
>> +                        nr_scanned += folio_nr_pages(folio);
>>                          continue;
>>                  }
>>  
>> -                nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid));
>> +                nr_reclaimed += reclaim_folio_list(&node_folio_list, nr_scanned,
>> +                                                   NODE_DATA(nid));
>> +                nr_scanned = 0;
>>                  nid = folio_nid(lru_to_folio(folio_list));
>>          } while (!list_empty(folio_list));
>>  
>> -        nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid));
>> -
>> +        nr_reclaimed += reclaim_folio_list(&node_folio_list, nr_scanned,
>> +                                           NODE_DATA(nid));
>>          memalloc_noreclaim_restore(noreclaim_flag);
>>  
>>          return nr_reclaimed;
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ