Re: [PATCH] mm: madvise: fix uneven accounting of psi

[Charan Teja Kalla <quic_charante@quicinc.com>]

Thanks Suren & Johannes,

On 6/7/2023 1:18 AM, Suren Baghdasaryan wrote:
> Hi Folks. Sorry for being late to the party.
> Yeah, userspace does not have a crystal ball to predict future user
> behavior, so there will always be pathological cases when usual
> assumptions and resulting madvise() would make things worse.
> 
> I think this discussion can be split into several questions/issues:
> 1. Inconsistency in how madvise(MADV_PAGEOUT) would affect PSI
> calculation when the page is refaulted, based on the path it took
> before being evicted by madvise(). In your initial description case
> (a) is inconsistent with (b) and (c) and it's probably worth fixing.
> IMHO (a) should be made consistent with others, not the other way
> around. My reasoning is that page was expelled from the active list,
> so it was part of the active workingset.
> 
That means we should be setting Workingset on the page while it is on
the active list and when it is being pageout through madvising. Right? I
see, this makes it consistent.

On the same note, discussing with Suren offline, Should the refaulted
madvise pages start always at the inactive list? If they are really
active, they get promoted anyway..

> 2. Whether refaults caused by incorrect madvise(MADV_PAGEOUT) should
> be counted as workingset refault and affect PSI.
> This one I think is trickier. IMHO it should be counted as workingset
> refault simply because it was refaulted and it was part of the
> workingset. Whether it should affect PSI, which is supposed to be an
> indicator of "pressure" is, I think, debatable. With madvise() in the
> mix, refault might happen without any real memory pressure... So, the
> answer is not obvious to me.
> 
> 3. Should refaults caused by incorrect madvise(MADV_PAGEOUT) be
> distinguished from the ones which were evicted by kernel reclaim
> mechanisms.
> I can see use for that from userspace to detect incorrect madvise()
> and adjust its aggressiveness. I think the API might get a bit complex
> because of the need to associate refaults with specific madvise()/VMAs
> to understand which hint was incorrect and adjust the behavior.
> Instead what is the opinion about giving an MADV_PAGEOUT_INACTIVE
interface which does operate on a page only If it is on the inactive
list and !PageWorkingset ?

> Hope my feedback is useful and if we can improve Android's userspace
> behavior, I'm happy to help make that happen.
Thanks...