I've applied this patch, but I have to say that I absolutely detest it.
This is very fragile, and I think the problem is the nasty interface.
Why don't we have simple wrappers that internally do that "mod", but
actually expose "inc" and "dec" instead, and make it much harder to
have these kinds of problems?
The function name is horrible too: "mod_node_page_state()"? It's not
like that really even describes what it does. It doesn't modify any
page state what-so-ever, what it does is to update some node counters.
Did somebody mis-understand what "stat" stands for? It's not some odd
shortening of "state" (like "creat"). It's shorthand for "statistics".
Because that completely nonsensical "state" naming shows up in all of
those helper functions too.
And the "page" part of the name is misleading too, since it's not even
about page statistics, and the people who have a page need to
literally do "page_pgdat()" on that page in order to pass in the
"struct pglist_data" that the function wants.
To make matters worse, we have the double underscore versions too,
which have that magical "we know interrupts are disabled"
optimization, so we duplicate all of this horror.
So I really think this whole area would be ripe for some major
interface cleanups, both in naming and in interfaces.
A _small_ step would have been to avoid the "mod" thing.
Linus
On Fri, Nov 13, 2020 at 10:51 PM Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> From: Nicholas Piggin <npiggin@xxxxxxxxx>
> Subject: mm/vmscan: fix NR_ISOLATED_FILE corruption on 64-bit
>
> - mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE, -nr_reclaimed);
> + mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE,
> + -(long)nr_reclaimed);
...
> mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE,
> - -stat.nr_lazyfree_fail);
> + -(long)stat.nr_lazyfree_fail);
