On Tue, Apr 28, 2020 at 4:05 PM Michal Hocko <mhocko@xxxxxxxxxx> wrote:
>
> On Tue 28-04-20 09:45:27, Yafang Shao wrote:
> [...]
> > Seems we can't get an agreement on how to improve current code.
> > So I will submit a patch to revert the commit 9783aa9917f8 ("mm,
> > memcg: proportional memory.{low,min} reclaim") first.
>
> My current understanding is that the issue we are discussing here is
> mostly theoretical. Your changelog doesn't really talk about any real
> life workloads that would be suffering.
Is real life workload really important ?
If so, why an issue[1] occured in the real workload report by me in
2019 that memcg proection can't protect inactive pages (inodes) is
ignored again and again ?
So I'm questioning that what is the real life workload ? The great
FACEBOOK workload ? These issues can all be igored if they don't
impact the great FACEBOOK workload ?
BTW, you don't need to answer me as this is another issue.
[1]. https://lore.kernel.org/linux-mm/1584355198-10137-1-git-send-email-laoar.shao@xxxxxxxxx/
> While it is possible to construct
> one that misbehaves it is really important to know whether this actually
> happens in real life. My guess would be that it is not because nax/high
> limits do not tend to be close to protection usually.
>
> With that in mind I do not really believe that reverting 9783aa9917f8
> is a reasonable approach. Try to weigh pros and cons of the
> functionality. Useful functionality for reasonable setups vs. potential
> corner cases which are not really likely.
>
> Please also note that we might disagree on implementation details
> because people usually have very different taste for code. But it seems
> that we are in agreement with Johannes that your patch does not really
> improve the overall situation all that much while it adds stuff that we
> actively disagree with.
>
> So it would be really more helpful to not insist on unrelated
> implementation details and focus on two things 1) split up the effective
> values calculation from the predicate (cleanup without any functional
> changes) 2) make the calculation more robust against racing reclaimers.
>
Another thing should be considered as well, 0) don't access
memroy.emin and elow in get_scan_count().
> I plan to get to this unless you beat me to it.
> --
--
Thanks
Yafang
