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;
Message-ID: <ad41ccc3-fd9a-bca3-4910-751b72c831fe@quicinc.com>
Date:   Fri, 30 Jun 2023 18:46:02 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.2
Subject: Re: [PATCH V2] mm: madvise: fix uneven accounting of psi
Content-Language: en-US
From:   Charan Teja Kalla <quic_charante@quicinc.com>
To:     Pavan Kondeti <quic_pkondeti@quicinc.com>
CC:     <akpm@linux-foundation.org>, <surenb@google.com>,
        <hannes@cmpxchg.org>, <minchan@kernel.org>,
        <quic_smanapra@quicinc.com>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>
References: <1687861992-8722-1-git-send-email-quic_charante@quicinc.com>
 <f72dc0b0-e848-4053-879d-5eccd4d00b52@quicinc.com>
 <65ce241e-8614-b669-cd20-b315c30bd794@quicinc.com>
In-Reply-To: <65ce241e-8614-b669-cd20-b315c30bd794@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Pavan,

On 6/28/2023 4:19 PM, Charan Teja Kalla wrote:
> I think marking the folio as a workingset when it sits on the active is
> not a correct thing. For the same example you mentioned, a simple CMA
> allocation will be dropping the clean pages instead of migration. PSI
> accounting on refault of those pages don't reveal anything to the user.
> 
> Where as in the madvise() cases, this PSI tells the user about the type
> of pages that he is working on.[1]
> 
> BTW, damon_pa_pageout() seems a valid case above. let me fix it in the
> next patch.
I did look a little bit more at the damon code and IIUC it: DAMON
monitors the ranges it is asked to operate as regions and
operate(reclaim) on the region that has less number of accesses, IOW,
damon won't do pageout operation on a folio if it is really under use,
CMIW.

This is unlike the case with the madvise() operation where Workingset
helps in accounting PSI that helps user the type of folios he is
operating on.

Assume that damon is operating on wrong set of regions and Workingset
helps in giving a PSI. This got no help to user and just telling the
internals of damon. No?

Having said that, theoretically it seems correct to me to set workingset
on folios as they leave the active list, but I don't have any strong
reason to say what happens if we won't.

Moreover, this patch is mostly talks about the madvise() operated folios
not inline with the usual reclaim. May be a separate change can be
raised for damon() operated folios once we agree upon the importance of
Workingset to these folios. WDYT?

Thanks,