On Tue, Jun 27, 2023 at 04:03:12PM +0530, Charan Teja Kalla wrote: > A folio turns into a Workingset during: > 1) shrink_active_list() placing the folio from active to inactive list. > 2) When a workingset transition is happening during the folio refault. > > And when Workingset is set on a folio, PSI for memory can be accounted > during a) That folio is being reclaimed and b) Refault of that folio. > Please help me understand why PSI for memory (I understood it as the time spent in psi_memstall_enter() to psi_memstall_leave()) would be accounted in (a) i.e during reclaim. I understand that when a working The (b) part is very clear. > This accounting of PSI for memory is not consistent in the cases where > clients use madvise(COLD/PAGEOUT) to deactivate or proactively reclaim a > folio: > a) A folio started at inactive and moved to active as part of accesses. > Workingset is absent on the folio thus madvise(MADV_PAGEOUT) don't > account such folios for PSI. > > b) When the same folio transition from inactive->active and then to > inactive through shrink_active_list(). Workingset is set on the folio > thus madvise(MADV_PAGEOUT) account such folios for PSI. > > c) When the same folio is part of active list directly as a result of > folio refault and this was a workingset folio prior to eviction. > Workingset is set on the folio thus both the operations of MADV_PAGEOUT > and reclaim of the MADV_COLD operated folio account for PSI. > > d) madvise(MADV_COLD) transfers the folio from active list to inactive > list. Such folios may not have the Workingset thus reclaim operation > on such folio doesn't account for PSI. > > As said above, the MADV_PAGEOUT on a folio is accounts for memory PSI in > b) and c) but not in a). Reclaim of a folio on which MADV_COLD is > performed accounts memory PSI in c) but not in d) which is an > inconsistent behaviour. Make this PSI accounting always consistent by > turning a folio into a workingset one whenever it is leaving the active > list. Also, accounting of PSI on a folio whenever it leaves the > active list as part of the MADV_COLD/PAGEOUT operation helps the users > whether they are operating on proper folios[1]. I understood the problem from V1 discussions. But the references to "madvise account such folios for PSI" is confusing. Why would madvise(PAGEOUT) be accounting anything related to PSI. I get that madvise() is messing up PSI accuracy indirectly.. > > [1] https://lore.kernel.org/all/20230605180013.GD221380@xxxxxxxxxxx/ > > Suggested-by: Suren Baghdasaryan <surenb@xxxxxxxxxx> > Reported-by: Sai Manobhiram Manapragada <quic_smanapra@xxxxxxxxxxx> > Reported-by: Pavan Kondeti <quic_pkondeti@xxxxxxxxxxx> > Signed-off-by: Charan Teja Kalla <quic_charante@xxxxxxxxxxx> > --- > V2: Made changes as per the comments from Johannes/Suren. > > V1: https://lore.kernel.org/all/1685531374-6091-1-git-send-email-quic_charante@xxxxxxxxxxx/ > > mm/madvise.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/madvise.c b/mm/madvise.c > index d9e7b42..76fb31f 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -413,6 +413,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > > folio_clear_referenced(folio); > folio_test_clear_young(folio); > + folio_set_workingset(folio); > if (pageout) { > if (folio_isolate_lru(folio)) { > if (folio_test_unevictable(folio)) > @@ -512,6 +513,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > */ > folio_clear_referenced(folio); > folio_test_clear_young(folio); > + folio_set_workingset(folio); > if (pageout) { > if (folio_isolate_lru(folio)) { > if (folio_test_unevictable(folio)) > -- > 2.7.4 > This is not limited to madvise(PAGEOUT) right, anywhere an active page is reclaimed we have the same problem. For ex: damon_pa_pageout() and __alloc_contig_migrate_range()->reclaim_clean_pages_from_list(). If that is the case, can we set mark a folio as a workingset when it is activated? That way, we don't have make madvise() as a special case? Thanks, Pavan
