From: Linus Torvalds <torvalds@linux-foundation.org> To: Andrew Morton <akpm@linux-foundation.org> Cc: a.sahrawat@samsung.com, Linux-MM <linux-mm@kvack.org>, maninder1.s@samsung.com, Mel Gorman <mgorman@suse.de>, Michal Hocko <mhocko@suse.com>, mm-commits@vger.kernel.org, Nick Piggin <npiggin@gmail.com>, stable <stable@vger.kernel.org>, v.narang@samsung.com, Vlastimil Babka <vbabka@suse.cz> Subject: Re: [patch 03/14] mm/vmscan: fix NR_ISOLATED_FILE corruption on 64-bit Date: Sat, 14 Nov 2020 13:39:47 -0800 [thread overview] Message-ID: <CAHk-=wj847SudR-kt+46fT3+xFFgiwpgThvm7DJWGdi4cVrbnQ@mail.gmail.com> (raw) In-Reply-To: <20201114065146.3H6OX7gIF%akpm@linux-foundation.org> 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@linux-foundation.org> wrote: > > From: Nicholas Piggin <npiggin@gmail.com> > 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);
next prev parent reply other threads:[~2020-11-14 21:40 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-14 6:51 incoming Andrew Morton 2020-11-14 6:51 ` [patch 01/14] mm/compaction: count pages and stop correctly during page isolation Andrew Morton 2020-11-14 6:51 ` [patch 02/14] mm/compaction: stop isolation if too many pages are isolated and we have pages to migrate Andrew Morton 2020-11-14 6:51 ` [patch 03/14] mm/vmscan: fix NR_ISOLATED_FILE corruption on 64-bit Andrew Morton 2020-11-14 21:39 ` Linus Torvalds [this message] 2020-11-14 22:14 ` Matthew Wilcox 2020-11-14 6:51 ` [patch 04/14] mailmap: fix entry for Dmitry Baryshkov/Eremin-Solenikov Andrew Morton 2020-11-14 6:51 ` [patch 05/14] mm/slub: fix panic in slab_alloc_node() Andrew Morton 2020-11-14 6:51 ` [patch 06/14] mm/gup: use unpin_user_pages() in __gup_longterm_locked() Andrew Morton 2020-11-14 6:51 ` [patch 07/14] compiler.h: fix barrier_data() on clang Andrew Morton 2020-11-14 6:52 ` [patch 08/14] Revert "kernel/reboot.c: convert simple_strtoul to kstrtoint" Andrew Morton 2020-11-14 6:52 ` [patch 09/14] reboot: fix overflow parsing reboot cpu number Andrew Morton 2020-11-14 6:52 ` [patch 10/14] kernel/watchdog: fix watchdog_allowed_mask not used warning Andrew Morton 2020-11-14 6:52 ` [patch 11/14] mm: memcontrol: fix missing wakeup polling thread Andrew Morton 2020-11-14 6:52 ` [patch 12/14] hugetlbfs: fix anon huge page migration race Andrew Morton 2020-11-14 6:52 ` [patch 13/14] panic: don't dump stack twice on warn Andrew Morton 2020-11-14 6:52 ` [patch 14/14] ocfs2: initialize ip_next_orphan Andrew Morton
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHk-=wj847SudR-kt+46fT3+xFFgiwpgThvm7DJWGdi4cVrbnQ@mail.gmail.com' \ --to=torvalds@linux-foundation.org \ --cc=a.sahrawat@samsung.com \ --cc=akpm@linux-foundation.org \ --cc=linux-mm@kvack.org \ --cc=maninder1.s@samsung.com \ --cc=mgorman@suse.de \ --cc=mhocko@suse.com \ --cc=mm-commits@vger.kernel.org \ --cc=npiggin@gmail.com \ --cc=stable@vger.kernel.org \ --cc=v.narang@samsung.com \ --cc=vbabka@suse.cz \ --subject='Re: [patch 03/14] mm/vmscan: fix NR_ISOLATED_FILE corruption on 64-bit' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).