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: <f3f15b6a-8618-f755-f21c-4193c502e65c@quicinc.com>
Date:   Tue, 6 Jun 2023 20:23:59 +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] mm: madvise: fix uneven accounting of psi
Content-Language: en-US
To:     Johannes Weiner <hannes@cmpxchg.org>
CC:     <akpm@linux-foundation.org>, <minchan@kernel.org>,
        <quic_pkondeti@quicinc.com>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>,
        Suren Baghdasaryan <surenb@google.com>
References: <1685531374-6091-1-git-send-email-quic_charante@quicinc.com>
 <20230531221955.GD102494@cmpxchg.org>
 <230e45e8-8cd8-3668-bbfa-a95212b4cb99@quicinc.com>
 <20230605180013.GD221380@cmpxchg.org>
From:   Charan Teja Kalla <quic_charante@quicinc.com>
In-Reply-To: <20230605180013.GD221380@cmpxchg.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

Thanks Johannes for the detailed review comments...

On 6/5/2023 11:30 PM, Johannes Weiner wrote:
>> Agree that we shouldn't be really silence the thrashing. My point is we
>> shouldn't be  considering the folios as thrashing If those were getting
>> reclaim by the user him self through MADV_PAGEOUT under the assumption
>> that __user knows they are not real working set__.  Please let me know
>> if I am not making sense here.
> I'm not sure I agree with this. I think it misses the point of what
> the madvise is actually for.
> 
> The workingset is defined based on access frequency and available
> memory. Thrashing is defined as having to read pages back shortly
> after their eviction.
> 
> MADV_PAGEOUT is for the application to inform the kernel that it's
> done accessing the pages, so that the kernel can accelerate their
> eviction over other pages that may still be in use. This is ultimately
> meant to REDUCE reclaim and paging.
> 
> However, in this case, the MADVISE_PAGEOUT evicts pages that are
> reused after and then refault. It INCREASED reclaim and paging.
> 
I agree here...
> Surely that's a problem? And the system would have behaved better
> without the madvise() in the first place?
> 
Yes, the system behavior could be much better without this PAGEOUT
operation...
> In fact, I would argue that the pressure spike is a great signal for
> detecting overzealous madvising. If you're redefining the workingset
> from access frequency to "whatever the user is saying", that will take
> away an important mechanism to detect advise bugs and unnecessary IO.
currently wanted to do the PAGEOUT operation but what information lacks
is if I am really operating on the workingset pages. Had the client
knows that he is operating on the workingset pages, he could have backed
off from madvising.

I now note that I shouldn't be defining the workingset from "whatever
user is saying". But then, IMO, there should be a way from the kernel to
the user that his madvise operation is being performed on the workingset
pages.

One way the user can do is monitoring the PSI events while PAGEOUT is
being performed and he may exclude those VMA's from next time.

Alternatively kernel itself can support it may be through like
MADV_PAGEOUT_INACTIVE which doesn't pageout the Workingset pages.

Please let me know your opinion about this interface.

This has the usecase on android where it just assumes that 2nd
background app will most likely to be not used in the future thus
reclaim those app pages. It works well for most of the times but such
assumption will go wrong with the usecase I had mentioned.

--Thanks.