From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6CBDECAAA1 for ; Tue, 30 Aug 2022 07:34:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 61078940007; Tue, 30 Aug 2022 03:34:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5C0A16B0074; Tue, 30 Aug 2022 03:34:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 487A8940007; Tue, 30 Aug 2022 03:34:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 34D3E6B0073 for ; Tue, 30 Aug 2022 03:34:09 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 090B0A0546 for ; Tue, 30 Aug 2022 07:34:09 +0000 (UTC) X-FDA: 79855445418.12.8761F6C Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf20.hostedemail.com (Postfix) with ESMTP id 88D711C0031 for ; Tue, 30 Aug 2022 07:34:08 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 529E321F3E; Tue, 30 Aug 2022 07:34:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1661844847; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=g9OX2kU4wHyvmVyOS1gFsOzCM2jeYG5ykVfYJ1gz5vM=; b=HSzDcIeDyuom9eWNz8+KkvUd2/QJ4Xih3zqa1iMfFL5THkJWFnWKlc8EavPOvEcCID9Qql I/xTEljCbc1xj/5IZo7Qkl6cPstQjKBC3le0sN68JbadPxMpVjn5BQzU2bRedpOzJDiodt 3Z1siIWz4UIbSWxQtP7Spo5k4i0Ew7E= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 329DD13ACF; Tue, 30 Aug 2022 07:34:07 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id eUueCW+9DWM4OAAAMHmgww (envelope-from ); Tue, 30 Aug 2022 07:34:07 +0000 Date: Tue, 30 Aug 2022 09:34:06 +0200 From: Michal Hocko To: linux-mm@kvack.org Cc: Christoph Hellwig , Andrew Morton , Mel Gorman , Vlastimil Babka , Johannes Weiner , Dan Carpenter , LKML Subject: Re: [RFC PATCH] mm: reduce noise in show_mem for lowmem allocations Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=HSzDcIeD; spf=pass (imf20.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661844848; a=rsa-sha256; cv=none; b=OSK3/bNC7Ram/FFA8BEmMQ9UmRWz+F1374HNiFl9TnGAAyU/RD5qazUjLg7EdT9gKWduVS DOWiYgFSJ55hz2yos80KoTzrTosUvt7eqd6Xg66aa2itdxj0bcDr0Mk9lviNulNnIIKwdw LirV+s6T0jmYdLJ+eW4yF3VQwgtFx7U= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661844848; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=g9OX2kU4wHyvmVyOS1gFsOzCM2jeYG5ykVfYJ1gz5vM=; b=u5iqleJ915WVzToOCbORcU89A+wk/A3kAh5ik54n90vHlnRZo1TnbAbc3iE3Yz4Xmr0QfS xWfkxeXRsUHRY5wjsGi0VNYz4OYLcUX1tapWU6mWsbcscFmCYSSS5Fp4cEo57xoNrK/kPR kplCFGvXiy9FVIJa0Qw4k1ZyqMj7Pas= Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=HSzDcIeD; spf=pass (imf20.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com X-Rspamd-Server: rspam06 X-Rspam-User: X-Stat-Signature: t3fb8py61z4rkd1esp4coy3ks53hzq74 X-Rspamd-Queue-Id: 88D711C0031 X-HE-Tag: 1661844848-740912 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: [Cc Dan] Dan has brought up[1] that the use of gfp mask has confused his static analyzer which assumes that GFP_HIGHUSER_MOVABLE implies a sleeping allocation and that wouldn't be a great idea from the panic path. I would add that most callers of this function would be really bad to allocate. The report itself is a false positive but it made me think a bit about this. Even if the check is too simplistic I guess it resembles how many developers are thinking (including me). If I see GFP_HIGHUSER_MOVABLE or GF_KERNEL I automatically assume a sleeping allocation down the road. And who know somebody might add one in the future even into show_mem because the gfp parameter would be too tempting to not (ab)use. My original intention was to use a natural allocation speak but this can backfire so maybe it would be better to give the argument its real meaning and that is the high_zone_idx. This is cryptic for code outside of MM but that is not all that many callers and we can hide this fact from them. In other words does the thing below looks better (incremental for illustration, I will make it a proper patch if yes)? [1] https://lore.kernel.org/all/Yw2ugrlZ8bwE5/hh@kili/?q=http%3A%2F%2Flkml.kernel.org%2Fr%2FYw2ugrlZ8bwE5%2Fhh%40kili --- diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index 19b1a94b0c00..3d9782ea3fa7 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -1086,7 +1086,7 @@ cmds(struct pt_regs *excp) memzcan(); break; case 'i': - show_mem(0, NULL, GFP_HIGHUSER_MOVABLE); + show_mem(0, NULL); break; default: termch = cmd; diff --git a/arch/sparc/kernel/setup_32.c b/arch/sparc/kernel/setup_32.c index 2a1e5641a7cc..c8e0dd99f370 100644 --- a/arch/sparc/kernel/setup_32.c +++ b/arch/sparc/kernel/setup_32.c @@ -83,7 +83,7 @@ static void prom_sync_me(void) "nop\n\t" : : "r" (&trapbase)); prom_printf("PROM SYNC COMMAND...\n"); - show_free_areas(0, NULL, GFP_HIGHUSER_MOVABLE); + show_free_areas(0, NULL); if (!is_idle_task(current)) { local_irq_enable(); ksys_sync(); diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c index c6b2b42d2367..d2b2720db6ca 100644 --- a/drivers/tty/sysrq.c +++ b/drivers/tty/sysrq.c @@ -342,7 +342,7 @@ static const struct sysrq_key_op sysrq_ftrace_dump_op = { static void sysrq_handle_showmem(int key) { - show_mem(0, NULL, GFP_HIGHUSER_MOVABLE); + show_mem(0, NULL); } static const struct sysrq_key_op sysrq_showmem_op = { .handler = sysrq_handle_showmem, diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c index 9ceeea531cf6..be8313cdbac3 100644 --- a/drivers/tty/vt/keyboard.c +++ b/drivers/tty/vt/keyboard.c @@ -606,7 +606,7 @@ static void fn_scroll_back(struct vc_data *vc) static void fn_show_mem(struct vc_data *vc) { - show_mem(0, NULL, GFP_HIGHUSER_MOVABLE); + show_mem(0, NULL); } static void fn_show_state(struct vc_data *vc) diff --git a/include/linux/mm.h b/include/linux/mm.h index e0d0ac52770c..0756798feaff 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1827,7 +1827,11 @@ extern void pagefault_out_of_memory(void); */ #define SHOW_MEM_FILTER_NODES (0x0001u) /* disallowed nodes */ -extern void show_free_areas(unsigned int flags, nodemask_t *nodemask, gfp_t gfp_mask); +extern void __show_free_areas(unsigned int flags, nodemask_t *nodemask, int max_zone_idx); +static void __maybe_unused show_free_areas(unsigned int flags, nodemask_t *nodemask) +{ + __show_free_areas(flags, nodemask, MAX_NR_ZONES - 1); +} #ifdef CONFIG_MMU extern bool can_do_mlock(void); @@ -2563,7 +2567,12 @@ extern void calculate_min_free_kbytes(void); extern int __meminit init_per_zone_wmark_min(void); extern void mem_init(void); extern void __init mmap_init(void); -extern void show_mem(unsigned int flags, nodemask_t *nodemask, gfp_t gfp_mask); + +extern void __show_mem(unsigned int flags, nodemask_t *nodemask, int max_zone_idx); +static inline void show_mem(unsigned int flags, nodemask_t *nodemask) +{ + __show_mem(flags, nodemask, MAX_NR_ZONES - 1); +} extern long si_mem_available(void); extern void si_meminfo(struct sysinfo * val); extern void si_meminfo_node(struct sysinfo *val, int nid); diff --git a/init/initramfs.c b/init/initramfs.c index ec966cb78363..18229cfe8906 100644 --- a/init/initramfs.c +++ b/init/initramfs.c @@ -63,7 +63,7 @@ static void panic_show_mem(const char *fmt, ...) { va_list args; - show_mem(0, NULL, GFP_HIGHUSER_MOVABLE); + show_mem(0, NULL); va_start(args, fmt); panic(fmt, args); va_end(args); diff --git a/kernel/panic.c b/kernel/panic.c index ba5f7691d15c..a3308af28a21 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -187,7 +187,7 @@ static void panic_print_sys_info(bool console_flush) show_state(); if (panic_print & PANIC_PRINT_MEM_INFO) - show_mem(0, NULL, GFP_HIGHUSER_MOVABLE); + show_mem(0, NULL); if (panic_print & PANIC_PRINT_TIMER_INFO) sysrq_timer_list_show(); diff --git a/lib/show_mem.c b/lib/show_mem.c index b97461a6c4bb..ade71096e241 100644 --- a/lib/show_mem.c +++ b/lib/show_mem.c @@ -8,13 +8,13 @@ #include #include -void show_mem(unsigned int filter, nodemask_t *nodemask, gfp_t gfp_mask) +void __show_mem(unsigned int filter, nodemask_t *nodemask, int max_zone_idx) { pg_data_t *pgdat; unsigned long total = 0, reserved = 0, highmem = 0; printk("Mem-Info:\n"); - show_free_areas(filter, nodemask, gfp_mask); + show_free_areas(filter, nodemask, max_zone_idx); for_each_online_pgdat(pgdat) { int zoneid; diff --git a/mm/nommu.c b/mm/nommu.c index 46cff4a51fa0..9d7afc2d959e 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -1047,7 +1047,7 @@ static int do_mmap_private(struct vm_area_struct *vma, enomem: pr_err("Allocation of length %lu from process %d (%s) failed\n", len, current->pid, current->comm); - show_free_areas(0, NULL, GFP_KERNEL); + show_free_areas(0, NULL); return -ENOMEM; } @@ -1270,13 +1270,13 @@ unsigned long do_mmap(struct file *file, kmem_cache_free(vm_region_jar, region); pr_warn("Allocation of vma for %lu byte allocation from process %d failed\n", len, current->pid); - show_free_areas(0, NULL, GFP_KERNEL); + show_free_areas(0, NULL); return -ENOMEM; error_getting_region: pr_warn("Allocation of vm region for %lu byte allocation from process %d failed\n", len, current->pid); - show_free_areas(0, NULL, GFP_KERNEL); + show_free_areas(0, NULL); return -ENOMEM; } diff --git a/mm/oom_kill.c b/mm/oom_kill.c index ffeaad3b348d..94804504be9c 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -461,7 +461,7 @@ static void dump_header(struct oom_control *oc, struct task_struct *p) if (is_memcg_oom(oc)) mem_cgroup_print_oom_meminfo(oc->memcg); else { - show_mem(SHOW_MEM_FILTER_NODES, oc->nodemask, oc->gfp_mask); + __show_mem(SHOW_MEM_FILTER_NODES, oc->nodemask, gfp_zone(oc->gfp_mask)); if (should_dump_unreclaim_slab()) dump_unreclaimable_slab(); } diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 208e3f8b38f8..110a16ea848a 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4248,7 +4248,7 @@ static void warn_alloc_show_mem(gfp_t gfp_mask, nodemask_t *nodemask) if (!in_task() || !(gfp_mask & __GFP_DIRECT_RECLAIM)) filter &= ~SHOW_MEM_FILTER_NODES; - show_mem(filter, nodemask, gfp_mask); + __show_mem(filter, nodemask, gfp_zone(gfp_mask)); } void warn_alloc(gfp_t gfp_mask, nodemask_t *nodemask, const char *fmt, ...) @@ -5944,10 +5944,9 @@ static bool node_has_managed_zones(pg_data_t *pgdat, int max_zone_idx) * SHOW_MEM_FILTER_NODES: suppress nodes that are not allowed by current's * cpuset. */ -void show_free_areas(unsigned int filter, nodemask_t *nodemask, gfp_t gfp_mask) +void __show_free_areas(unsigned int filter, nodemask_t *nodemask, int max_zone_idx) { unsigned long free_pcp = 0; - int max_zone_idx = gfp_zone(gfp_mask); int cpu; struct zone *zone; pg_data_t *pgdat; -- Michal Hocko SUSE Labs