linux-kernel - Re: [PATCH 2/2] mm: rename global_page_state to global_zone_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170801140520.96835ef87fe41a448c05504b@linux-foundation.org>
Date:   Tue, 1 Aug 2017 14:05:20 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Johannes Weiner <hannes@...xchg.org>
Cc:     Michal Hocko <mhocko@...e.com>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Josef Bacik <josef@...icpanda.com>,
        Vladimir Davydov <vdavydov.dev@...il.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH 2/2] mm: rename global_page_state to
 global_zone_page_state

On Tue,  1 Aug 2017 09:42:56 -0400 Johannes Weiner <hannes@...xchg.org> wrote:

> global_page_state is error prone as a recent bug report pointed out [1].
> It only returns proper values for zone based counters as the enum it
> gets suggests. We already have global_node_page_state so let's rename
> global_page_state to global_zone_page_state to be more explicit here.
> All existing users seems to be correct
> $ git grep "global_page_state(NR_" | sed 's@.*(\(NR_[A-Z_]*\)).*@\1@' | sort | uniq -c
>       2 NR_BOUNCE
>       2 NR_FREE_CMA_PAGES
>      11 NR_FREE_PAGES
>       1 NR_KERNEL_STACK_KB
>       1 NR_MLOCK
>       2 NR_PAGETABLE
> 
> This patch shouldn't introduce any functional change.

Checkpatch gets a bit whiny.


WARNING: line over 80 characters
#127: FILE: mm/mmap.c:3517:
+	free_kbytes = global_zone_page_state(NR_FREE_PAGES) << (PAGE_SHIFT - 10);

WARNING: line over 80 characters
#136: FILE: mm/mmap.c:3538:
+	free_kbytes = global_zone_page_state(NR_FREE_PAGES) << (PAGE_SHIFT - 10);

WARNING: line over 80 characters
#145: FILE: mm/mmap.c:3582:
+		free_kbytes = global_zone_page_state(NR_FREE_PAGES) << (PAGE_SHIFT - 10);

WARNING: line over 80 characters
#157: FILE: mm/nommu.c:1965:
+	free_kbytes = global_zone_page_state(NR_FREE_PAGES) << (PAGE_SHIFT - 10);

WARNING: line over 80 characters
#166: FILE: mm/nommu.c:1986:
+	free_kbytes = global_zone_page_state(NR_FREE_PAGES) << (PAGE_SHIFT - 10);

WARNING: line over 80 characters
#187: FILE: mm/page-writeback.c:1408:
+ * global_zone_page_state() too often. So scale it near-sqrt to the safety margin


Liveable with, but the code would be quite a bit neater if we had a
helper function for this.  We get things like:

--- a/mm/mmap.c~mm-rename-global_page_state-to-global_zone_page_state-fix
+++ a/mm/mmap.c
@@ -3512,11 +3512,7 @@ void __init mmap_init(void)
  */
 static int init_user_reserve(void)
 {
-	unsigned long free_kbytes;
-
-	free_kbytes = global_zone_page_state(NR_FREE_PAGES) << (PAGE_SHIFT - 10);
-
-	sysctl_user_reserve_kbytes = min(free_kbytes / 32, 1UL << 17);
+	sysctl_user_reserve_kbytes = min(global_free_kbytes() / 32, 1UL << 17);
 	return 0;
 }
 subsys_initcall(init_user_reserve);