diff mbox series

[v2,4/4] mm: Remove managed_page_count spinlock

Message ID 1541521310-28739-5-git-send-email-arunks@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series mm: convert totalram_pages, totalhigh_pages and managed pages to atomic | expand

Commit Message

Arun KS Nov. 6, 2018, 4:21 p.m. UTC 
Now totalram_pages and managed_pages are atomic varibles. No need
of managed_page_count spinlock.

Signed-off-by: Arun KS <arunks@codeaurora.org>
Reviewed-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mmzone.h | 6 ------
 mm/page_alloc.c        | 5 -----
 2 files changed, 11 deletions(-)

Comments

kernel test robot Nov. 7, 2018, 9:26 p.m. UTC | #1 
Hi Arun,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc1 next-20181107]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Arun-KS/mm-Fix-multiple-evaluvations-of-totalram_pages-and-managed_pages/20181108-025657
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> mm/kasan/quarantine.c:239:23: error: not addressable
>> mm/kasan/quarantine.c:239:23: error: not addressable
   In file included from include/asm-generic/bug.h:5:0,
                    from arch/x86/include/asm/bug.h:47,
                    from include/linux/bug.h:5,
                    from include/linux/mmdebug.h:5,
                    from include/linux/gfp.h:5,
                    from mm/kasan/quarantine.c:20:
   mm/kasan/quarantine.c: In function 'quarantine_reduce':
   include/linux/compiler.h:246:20: error: lvalue required as unary '&' operand
      __read_once_size(&(x), __u.__c, sizeof(x));  \
                       ^
   include/linux/compiler.h:252:22: note: in expansion of macro '__READ_ONCE'
    #define READ_ONCE(x) __READ_ONCE(x, 1)
                         ^~~~~~~~~~~
   mm/kasan/quarantine.c:239:16: note: in expansion of macro 'READ_ONCE'
     total_size = (READ_ONCE(totalram_pages()) << PAGE_SHIFT) /
                   ^~~~~~~~~
   include/linux/compiler.h:248:28: error: lvalue required as unary '&' operand
      __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
                               ^
   include/linux/compiler.h:252:22: note: in expansion of macro '__READ_ONCE'
    #define READ_ONCE(x) __READ_ONCE(x, 1)
                         ^~~~~~~~~~~
   mm/kasan/quarantine.c:239:16: note: in expansion of macro 'READ_ONCE'
     total_size = (READ_ONCE(totalram_pages()) << PAGE_SHIFT) /
                   ^~~~~~~~~
--
   include/linux/slab.h:332:43: warning: dubious: x & !y
   include/linux/slab.h:332:43: warning: dubious: x & !y
>> net/sctp/protocol.c:1430:13: error: undefined identifier 'totalram_pgs'
   net/sctp/protocol.c:1431:24: error: undefined identifier 'totalram_pgs'
   net/sctp/protocol.c:1433:24: error: undefined identifier 'totalram_pgs'
>> /bin/bash: line 1: 74457 Segmentation fault      sparse -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise -Wno-return-void -Wno-unknown-attribute -D__CHECK_ENDIAN__ -D__x86_64__ -mlittle-endian -m64 -Wp,-MD,net/sctp/.protocol.o.d -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/7/include -Iarch/x86/include -I./arch/x86/include/generated -Iinclude -I./include -Iarch/x86/include/uapi -I./arch/x86/include/generated/uapi -Iinclude/uapi -I./include/generated/uapi -include include/linux/kconfig.h -include include/linux/compiler_types.h -Inet/sctp -Inet/sctp -D__KERNEL__ -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -Werror-implicit-function-declaration -Wno-format-security -std=gnu89 -fno-PIE -DCC_HAVE_ASM_GOTO -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 -falign-jumps=1 -falign-loops=1 -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3 -mskip-rax-setup -mtune=generic -mno-red-zone -mcmodel=kernel -funit-at-a-time -DCONFIG_X86_X32_ABI -DCONFIG_AS_CFI=1 -DCONFIG_AS_CFI_SIGNAL_FRAME=1 -DCONFIG_AS_CFI_SECTIONS=1 -DCONFIG_AS_FXSAVEQ=1 -DCONFIG_AS_SSSE3=1 -DCONFIG_AS_AVX=1 -DCONFIG_AS_AVX2=1 -DCONFIG_AS_AVX512=1 -DCONFIG_AS_SHA1_NI=1 -DCONFIG_AS_SHA256_NI=1 -pipe -Wno-sign-compare -fno-asynchronous-unwind-tables -mindirect-branch=thunk-extern -mindirect-branch-register -DRETPOLINE -Wa,arch/x86/kernel/macros.s -Wa,- -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-int-in-bool-context -O2 --param=allow-store-data-races=0 -fplugin=./scripts/gcc-plugins/latent_entropy_plugin.so -fplugin=./scripts/gcc-plugins/structleak_plugin.so -fplugin=./scripts/gcc-plugins/randomize_layout_plugin.so -fplugin=./scripts/gcc-plugins/stackleak_plugin.so -DLATENT_ENTROPY_PLUGIN -DSTRUCTLEAK_PLUGIN -DRANDSTRUCT_PLUGIN -DSTACKLEAK_PLUGIN -fplugin-arg-stackleak_plugin-track-min-size=100 -fno-reorder-blocks -fno-ipa-cp-clone -fno-partial-inlining -Wframe-larger-than=8192 -fstack-protector-strong -Wno-unused-but-set-variable -Wno-unused-const-variable -fno-var-tracking-assignments -pg -mrecord-mcount -mfentry -DCC_USING_FENTRY -fno-inline-functions-called-once -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -fno-strict-overflow -fno-merge-all-constants -fmerge-constants -fno-stack-check -fconserve-stack -Werror=implicit-int -Werror=strict-prototypes -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -fsanitize=kernel-address -fasan-shadow-offset=0xdffffc0000000000 --param asan-globals=1 --param asan-instrumentation-with-call-threshold=0 --param asan-stack=1 -fsanitize-coverage=trace-pc -DMODULE -DKBUILD_BASENAME='"protocol"' -DKBUILD_MODNAME='"sctp"' net/sctp/protocol.c

vim +239 mm/kasan/quarantine.c

55834c59 Alexander Potapenko 2016-05-20  211  
55834c59 Alexander Potapenko 2016-05-20  212  void quarantine_reduce(void)
55834c59 Alexander Potapenko 2016-05-20  213  {
64abdcb2 Dmitry Vyukov       2016-12-12  214  	size_t total_size, new_quarantine_size, percpu_quarantines;
55834c59 Alexander Potapenko 2016-05-20  215  	unsigned long flags;
ce5bec54 Dmitry Vyukov       2017-03-09  216  	int srcu_idx;
55834c59 Alexander Potapenko 2016-05-20  217  	struct qlist_head to_free = QLIST_INIT;
55834c59 Alexander Potapenko 2016-05-20  218  
64abdcb2 Dmitry Vyukov       2016-12-12  219  	if (likely(READ_ONCE(quarantine_size) <=
64abdcb2 Dmitry Vyukov       2016-12-12  220  		   READ_ONCE(quarantine_max_size)))
55834c59 Alexander Potapenko 2016-05-20  221  		return;
55834c59 Alexander Potapenko 2016-05-20  222  
ce5bec54 Dmitry Vyukov       2017-03-09  223  	/*
ce5bec54 Dmitry Vyukov       2017-03-09  224  	 * srcu critical section ensures that quarantine_remove_cache()
ce5bec54 Dmitry Vyukov       2017-03-09  225  	 * will not miss objects belonging to the cache while they are in our
ce5bec54 Dmitry Vyukov       2017-03-09  226  	 * local to_free list. srcu is chosen because (1) it gives us private
ce5bec54 Dmitry Vyukov       2017-03-09  227  	 * grace period domain that does not interfere with anything else,
ce5bec54 Dmitry Vyukov       2017-03-09  228  	 * and (2) it allows synchronize_srcu() to return without waiting
ce5bec54 Dmitry Vyukov       2017-03-09  229  	 * if there are no pending read critical sections (which is the
ce5bec54 Dmitry Vyukov       2017-03-09  230  	 * expected case).
ce5bec54 Dmitry Vyukov       2017-03-09  231  	 */
ce5bec54 Dmitry Vyukov       2017-03-09  232  	srcu_idx = srcu_read_lock(&remove_cache_srcu);
026d1eaf Clark Williams      2018-10-26  233  	raw_spin_lock_irqsave(&quarantine_lock, flags);
55834c59 Alexander Potapenko 2016-05-20  234  
55834c59 Alexander Potapenko 2016-05-20  235  	/*
55834c59 Alexander Potapenko 2016-05-20  236  	 * Update quarantine size in case of hotplug. Allocate a fraction of
55834c59 Alexander Potapenko 2016-05-20  237  	 * the installed memory to quarantine minus per-cpu queue limits.
55834c59 Alexander Potapenko 2016-05-20  238  	 */
a399c534 Arun KS             2018-11-06 @239  	total_size = (READ_ONCE(totalram_pages()) << PAGE_SHIFT) /
55834c59 Alexander Potapenko 2016-05-20  240  		QUARANTINE_FRACTION;
c3cee372 Alexander Potapenko 2016-08-02  241  	percpu_quarantines = QUARANTINE_PERCPU_SIZE * num_online_cpus();
64abdcb2 Dmitry Vyukov       2016-12-12  242  	new_quarantine_size = (total_size < percpu_quarantines) ?
64abdcb2 Dmitry Vyukov       2016-12-12  243  		0 : total_size - percpu_quarantines;
64abdcb2 Dmitry Vyukov       2016-12-12  244  	WRITE_ONCE(quarantine_max_size, new_quarantine_size);
64abdcb2 Dmitry Vyukov       2016-12-12  245  	/* Aim at consuming at most 1/2 of slots in quarantine. */
64abdcb2 Dmitry Vyukov       2016-12-12  246  	WRITE_ONCE(quarantine_batch_size, max((size_t)QUARANTINE_PERCPU_SIZE,
64abdcb2 Dmitry Vyukov       2016-12-12  247  		2 * total_size / QUARANTINE_BATCHES));
64abdcb2 Dmitry Vyukov       2016-12-12  248  
64abdcb2 Dmitry Vyukov       2016-12-12  249  	if (likely(quarantine_size > quarantine_max_size)) {
64abdcb2 Dmitry Vyukov       2016-12-12  250  		qlist_move_all(&global_quarantine[quarantine_head], &to_free);
64abdcb2 Dmitry Vyukov       2016-12-12  251  		WRITE_ONCE(quarantine_size, quarantine_size - to_free.bytes);
64abdcb2 Dmitry Vyukov       2016-12-12  252  		quarantine_head++;
64abdcb2 Dmitry Vyukov       2016-12-12  253  		if (quarantine_head == QUARANTINE_BATCHES)
64abdcb2 Dmitry Vyukov       2016-12-12  254  			quarantine_head = 0;
55834c59 Alexander Potapenko 2016-05-20  255  	}
55834c59 Alexander Potapenko 2016-05-20  256  
026d1eaf Clark Williams      2018-10-26  257  	raw_spin_unlock_irqrestore(&quarantine_lock, flags);
55834c59 Alexander Potapenko 2016-05-20  258  
55834c59 Alexander Potapenko 2016-05-20  259  	qlist_free_all(&to_free, NULL);
ce5bec54 Dmitry Vyukov       2017-03-09  260  	srcu_read_unlock(&remove_cache_srcu, srcu_idx);
55834c59 Alexander Potapenko 2016-05-20  261  }
55834c59 Alexander Potapenko 2016-05-20  262  

:::::: The code at line 239 was first introduced by commit
:::::: a399c534492723c9d2f175bc2b66aa930abd895f mm: convert totalram_pages and totalhigh_pages variables to atomic

:::::: TO: Arun KS <arunks@codeaurora.org>
:::::: CC: 0day robot <lkp@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index e73dc31..c71b4d9 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -428,12 +428,6 @@  struct zone {
 	 * Write access to present_pages at runtime should be protected by
 	 * mem_hotplug_begin/end(). Any reader who can't tolerant drift of
 	 * present_pages should get_online_mems() to get a stable value.
-	 *
-	 * Read access to managed_pages should be safe because it's unsigned
-	 * long. Write access to zone->managed_pages and totalram_pages are
-	 * protected by managed_page_count_lock at runtime. Idealy only
-	 * adjust_managed_page_count() should be used instead of directly
-	 * touching zone->managed_pages and totalram_pages.
 	 */
 	atomic_long_t		managed_pages;
 	unsigned long		spanned_pages;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2a42c3f..4d78bde 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -122,9 +122,6 @@ 
 };
 EXPORT_SYMBOL(node_states);
 
-/* Protect totalram_pages and zone->managed_pages */
-static DEFINE_SPINLOCK(managed_page_count_lock);
-
 atomic_long_t _totalram_pages __read_mostly;
 unsigned long totalreserve_pages __read_mostly;
 unsigned long totalcma_pages __read_mostly;
@@ -7064,14 +7061,12 @@  static int __init cmdline_parse_movablecore(char *p)
 
 void adjust_managed_page_count(struct page *page, long count)
 {
-	spin_lock(&managed_page_count_lock);
 	atomic_long_add(count, &page_zone(page)->managed_pages);
 	totalram_pages_add(count);
 #ifdef CONFIG_HIGHMEM
 	if (PageHighMem(page))
 		totalhigh_pages_add(count);
 #endif
-	spin_unlock(&managed_page_count_lock);
 }
 EXPORT_SYMBOL(adjust_managed_page_count);