diff mbox series

[V3] mm: madvise: fix uneven accounting of psi

Message ID 1688393201-11135-1-git-send-email-quic_charante@quicinc.com
State New
Headers
Series [V3] mm: madvise: fix uneven accounting of psi |

Commit Message

Charan Teja Kalla July 3, 2023, 2:06 p.m. UTC 
  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,
for usual reclaims.

This accounting of PSI for memory is not consistent for reclaim +
refault operation between usual reclaim and madvise(COLD/PAGEOUT) which
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 refault of it when reclaimed
through MADV_PAGEOUT operation doesn't account 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 refault of it when reclaimed through MADV_PAGEOUT operation
accounts 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 the refault of it when reclaimed
through MADV_PAGEOUT/MADV_COLD operation accounts for PSI.

d) MADV_COLD transfers the folio from active list to inactive
list. Such folios may not have the Workingset thus refault operation on
such folio doesn't account for PSI.

As said above, refault operation caused because of MADV_PAGEOUT on a
folio is accounts for memory PSI in b) and c) but not in a). Refault
caused by the reclaim of a folio on which MADV_COLD is performed
accounts memory PSI in c) but not in d). These behaviours are
inconsistent w.r.t usual reclaim + refault operation. 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].

[1] https://lore.kernel.org/all/20230605180013.GD221380@cmpxchg.org/

Suggested-by: Suren Baghdasaryan <surenb@google.com>
Reported-by: Sai Manobhiram Manapragada <quic_smanapra@quicinc.com>
Reported-by: Pavan Kondeti <quic_pkondeti@quicinc.com>
Signed-off-by: Charan Teja Kalla <quic_charante@quicinc.com>
---
V3: Incorporated comments from Johannes/Pavan.

V2: Made changes as per the comments from Johannes/Suren.
    https://lore.kernel.org/all/1687861992-8722-1-git-send-email-quic_charante@quicinc.com/

V1: https://lore.kernel.org/all/1685531374-6091-1-git-send-email-quic_charante@quicinc.com/

 mm/madvise.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Johannes Weiner July 5, 2023, 1:45 p.m. UTC | #1 
On Mon, Jul 03, 2023 at 07:36:41PM +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,
> for usual reclaims.
> 
> This accounting of PSI for memory is not consistent for reclaim +
> refault operation between usual reclaim and madvise(COLD/PAGEOUT) which
> 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 refault of it when reclaimed
> through MADV_PAGEOUT operation doesn't account 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 refault of it when reclaimed through MADV_PAGEOUT operation
> accounts 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 the refault of it when reclaimed
> through MADV_PAGEOUT/MADV_COLD operation accounts for PSI.
> 
> d) MADV_COLD transfers the folio from active list to inactive
> list. Such folios may not have the Workingset thus refault operation on
> such folio doesn't account for PSI.
> 
> As said above, refault operation caused because of MADV_PAGEOUT on a
> folio is accounts for memory PSI in b) and c) but not in a). Refault
> caused by the reclaim of a folio on which MADV_COLD is performed
> accounts memory PSI in c) but not in d). These behaviours are
> inconsistent w.r.t usual reclaim + refault operation. 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].
> 
> [1] https://lore.kernel.org/all/20230605180013.GD221380@cmpxchg.org/
> 
> Suggested-by: Suren Baghdasaryan <surenb@google.com>
> Reported-by: Sai Manobhiram Manapragada <quic_smanapra@quicinc.com>
> Reported-by: Pavan Kondeti <quic_pkondeti@quicinc.com>
> Signed-off-by: Charan Teja Kalla <quic_charante@quicinc.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Thanks Charan
diff mbox series

Patch

diff --git a/mm/madvise.c b/mm/madvise.c
index d9e7b42..d00a843 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -413,6 +413,8 @@  static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 
 		folio_clear_referenced(folio);
 		folio_test_clear_young(folio);
+		if (folio_test_active(folio))
+			folio_set_workingset(folio);
 		if (pageout) {
 			if (folio_isolate_lru(folio)) {
 				if (folio_test_unevictable(folio))
@@ -512,6 +514,8 @@  static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 		 */
 		folio_clear_referenced(folio);
 		folio_test_clear_young(folio);
+		if (folio_test_active(folio))
+			folio_set_workingset(folio);
 		if (pageout) {
 			if (folio_isolate_lru(folio)) {
 				if (folio_test_unevictable(folio))