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: <230e45e8-8cd8-3668-bbfa-a95212b4cb99@quicinc.com>
Date:   Thu, 1 Jun 2023 18:37:50 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
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>
From:   Charan Teja Kalla <quic_charante@quicinc.com>
In-Reply-To: <20230531221955.GD102494@cmpxchg.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

Thanks Johannes for taking a look at it..

On 6/1/2023 3:49 AM, Johannes Weiner wrote:
> On Wed, May 31, 2023 at 04:39:34PM +0530, Charan Teja Kalla wrote:
>> This patch is tested on Android, Snapdragon SoC with 8Gb RAM, 4GB swap
>> mounted on zram which has 2GB of backingdev. The test case involved
>> launching some memory hungry apps in an order and do the proactive
>> reclaim for the app that went to background using madvise(MADV_PAGEOUT).
>> We are seeing ~40% less total values of psi mem some and full when this
>> patch is combined with [1].
> Does that mean those pages are thrashing, but because you clear their
> workingset it isn't detected and reported via psi?
> 

Seems I didn't mention the usecase clearly and let me correct it. Say we
have the Android apps A, B, C, ... H and launching of these apps goes
like below.

1) Launch app A.
2) Launch app B.
3) Launch app C. At this time, we consider the memory used by app A is
no more in active use thus proactively reclaim them where we do issue
MADV_PAGEOUT on anon regions only thus these pages goes to swap mounted
on zram and subsequently into the backing dev attached to the zram.
4) Launch app D.. Proactively reclaim the anon regions of App B into
swap and through to backing dev.
5) Now make the app A to foreground. This can read the pages from the
swap + backing dev (because of the step 3)) that belongs to app A and
also proactively reclaim anon regions of app C.
6) Launch E --> proactive reclaim of app D to zram swap + backing dev.
7) Make App B to foreground --> Read memory of app B from swap +
backingdev and as well reclaim the anon regions of app A.
8) Like wise launches of apps F, C, G, D, H, E .....

If we look at steps 5, 7,..., we are just making the apps foreground
which folios (if were marked as workingset) can contribute to PSI events
through swap_readpage(). But in reality, these are not the real
workingset folios (I think it is safe to say this), because it is the
user who decided the reclaim of these pages by issuing the MADV_PAGEOUT
which he knows that these are not going to be needed in the near future
thus reclaim them.

I think the other way to look at the problem is the user can write a
simple application where he can do  MADV_PAGEOUT and read them back in a
loop. If at any point, folios user working on turns out to be a
workingset( he can just be probabilistic here), the PSI events will be
triggered though there may not be real memory pressure in the system.


> I don't rally get why silencing the thrashing is an improvement.
> 
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.

>> [1]https://lore.kernel.org/all/20220214214921.419687-1-hannes@cmpxchg.org/T/#u
>>
>> Signed-off-by: Charan Teja Kalla <quic_charante@quicinc.com>