Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 5 Jul 2023 09:45:43 -0400
From:   Johannes Weiner <hannes@cmpxchg.org>
To:     Charan Teja Kalla <quic_charante@quicinc.com>
Cc:     akpm@linux-foundation.org, surenb@google.com, minchan@kernel.org,
        quic_pkondeti@quicinc.com, quic_smanapra@quicinc.com,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH V3] mm: madvise: fix uneven accounting of psi
Message-ID: <20230705134543.GB156754@cmpxchg.org>
References: <1688393201-11135-1-git-send-email-quic_charante@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1688393201-11135-1-git-send-email-quic_charante@quicinc.com>
Precedence: bulk

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