Re: [v2 PATCH] mm: vmscan: correct nr_reclaimed for THP

From: Yang Shi
Date: Tue May 14 2019 - 16:46:28 EST




On 5/13/19 11:20 PM, Michal Hocko wrote:
On Mon 13-05-19 21:36:59, Yang Shi wrote:
On Mon, May 13, 2019 at 2:45 PM Michal Hocko <mhocko@xxxxxxxxxx> wrote:
On Mon 13-05-19 14:09:59, Yang Shi wrote:
[...]
I think we can just account 512 base pages for nr_scanned for
isolate_lru_pages() to make the counters sane since PGSCAN_KSWAPD/DIRECT
just use it.

And, sc->nr_scanned should be accounted as 512 base pages too otherwise we
may have nr_scanned < nr_to_reclaim all the time to result in false-negative
for priority raise and something else wrong (e.g. wrong vmpressure).
Be careful. nr_scanned is used as a pressure indicator to slab shrinking
AFAIR. Maybe this is ok but it really begs for much more explaining
I don't know why my company mailbox didn't receive this email, so I
replied with my personal email.

It is not used to double slab pressure any more since commit
9092c71bb724 ("mm: use sc->priority for slab shrink targets"). It uses
sc->priority to determine the pressure for slab shrinking now.

So, I think we can just remove that "double slab pressure" code. It is
not used actually and looks confusing now. Actually, the "double slab
pressure" does something opposite. The extra inc to sc->nr_scanned
just prevents from raising sc->priority.
I have to get in sync with the recent changes. I am aware there were
some patches floating around but I didn't get to review them. I was
trying to point out that nr_scanned used to have a side effect to be
careful about. If it doesn't have anymore then this is getting much more
easier of course. Please document everything in the changelog.

Thanks for reminding. Yes, I remembered nr_scanned would double slab pressure. But, when I inspected into the code yesterday, it turns out it is not true anymore. I will run some test to make sure it doesn't introduce regression.

BTW, I noticed the counter of memory reclaim is not correct with THP swap on vanilla kernel, please see the below:

pgsteal_kswapd 21435
pgsteal_direct 26573329
pgscan_kswapd 3514
pgscan_direct 14417775

pgsteal is always greater than pgscan, my patch could fix the problem.

Anyway, I will elaborate these in the commit log.