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: <4ccb8af9-3e01-2bf7-b680-abe0781d9e9d@alibaba-inc.com>
Date:   Fri, 22 Sep 2017 01:51:01 +0800
From:   "Yang Shi" <yang.s@...baba-inc.com>
To:     David Rientjes <rientjes@...gle.com>
Cc:     cl@...ux.com, penberg@...nel.org, iamjoonsoo.kim@....com,
        akpm@...ux-foundation.org, mhocko@...nel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] mm: oom: show unreclaimable slab info when kernel
 panic



On 9/21/17 1:23 AM, David Rientjes wrote:
> On Thu, 21 Sep 2017, Yang Shi wrote:
> 
>> Kernel may panic when oom happens without killable process sometimes it
>> is caused by huge unreclaimable slabs used by kernel.
>>
>> Although kdump could help debug such problem, however, kdump is not
>> available on all architectures and it might be malfunction sometime.
>> And, since kernel already panic it is worthy capturing such information
>> in dmesg to aid touble shooting.
>>
>> Print out unreclaimable slab info (used size and total size) which
>> actual memory usage is not zero (num_objs * size != 0) when panic_on_oom is set
>> or no killable process. Since such information is just showed when kernel
>> panic, so it will not lead too verbose message for normal oom.
>>
>> The output looks like:
>>
>> Unreclaimable slab info:
>> Name                      Used          Total
>> rpc_buffers               31KB         31KB
>> rpc_tasks                  7KB          7KB
>> ebitmap_node            1964KB       1964KB
>> avtab_node              5024KB       5024KB
>> xfs_buf                 1402KB       1402KB
>> xfs_ili                  134KB        134KB
>> xfs_efi_item             115KB        115KB
>> xfs_efd_item             115KB        115KB
>> xfs_buf_item             134KB        134KB
>> xfs_log_item_desc        342KB        342KB
>> xfs_trans               1412KB       1412KB
>> xfs_ifork                212KB        212KB
>>
>> Signed-off-by: Yang Shi <yang.s@...baba-inc.com>
>> ---
>>   mm/oom_kill.c    |  3 +++
>>   mm/slab.h        |  8 ++++++++
>>   mm/slab_common.c | 26 ++++++++++++++++++++++++++
>>   3 files changed, 37 insertions(+)
>>
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 99736e0..bd48d34 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -43,6 +43,7 @@
>>   
>>   #include <asm/tlb.h>
>>   #include "internal.h"
>> +#include "slab.h"
>>   
>>   #define CREATE_TRACE_POINTS
>>   #include <trace/events/oom.h>
>> @@ -960,6 +961,7 @@ static void check_panic_on_oom(struct oom_control *oc,
>>   	if (is_sysrq_oom(oc))
>>   		return;
>>   	dump_header(oc, NULL);
>> +	dump_unreclaimable_slab();
>>   	panic("Out of memory: %s panic_on_oom is enabled\n",
>>   		sysctl_panic_on_oom == 2 ? "compulsory" : "system-wide");
>>   }
>> @@ -1044,6 +1046,7 @@ bool out_of_memory(struct oom_control *oc)
>>   	/* Found nothing?!?! Either we hang forever, or we panic. */
>>   	if (!oc->chosen && !is_sysrq_oom(oc) && !is_memcg_oom(oc)) {
>>   		dump_header(oc, NULL);
>> +		dump_unreclaimable_slab();
>>   		panic("Out of memory and no killable processes...\n");
>>   	}
>>   	if (oc->chosen && oc->chosen != (void *)-1UL) {
>> diff --git a/mm/slab.h b/mm/slab.h
>> index 0733628..734a92d 100644
>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -505,6 +505,14 @@ static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
>>   void memcg_slab_stop(struct seq_file *m, void *p);
>>   int memcg_slab_show(struct seq_file *m, void *p);
>>   
>> +#ifdef CONFIG_SLABINFO
>> +void dump_unreclaimable_slab(void);
>> +#else
>> +void dump_unreclaimable_slab(void);
> 
> This won't compile when CONFIG_SLABINFO is disabled.
> 
> static inline void dump_unreclaimable_slab(void)
> {
> }
> 
> when CONFIG_SLABINFO=n.

Thanks for pointing this. Just tested CONFIG_SLANINFO = n case. It can't 
be disabled in menuconfig, just manually modified init/Kconfig to test it.

> 
>> +{
>> +}
>> +#endif
>> +
>>   void ___cache_free(struct kmem_cache *cache, void *x, unsigned long addr);
>>   
>>   #ifdef CONFIG_SLAB_FREELIST_RANDOM
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 904a83b..90d9de3 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -1272,6 +1272,32 @@ static int slab_show(struct seq_file *m, void *p)
>>   	return 0;
>>   }
>>   
>> +void dump_unreclaimable_slab(void)
>> +{
>> +	struct kmem_cache *s;
>> +	struct slabinfo sinfo;
>> +
>> +	pr_info("Unreclaimable slab info:\n");
>> +	pr_info("Name                      Used          Total\n");
>> +
>> +	/*
>> +	 * Here acquiring slab_mutex is unnecessary since we don't prefer to
>> +	 * get sleep in oom path right before kernel panic, and avoid race condition.
>> +	 * Since it is already oom, so there should be not any big allocation
>> +	 * which could change the statistics significantly.
> 
> The statistics themselves aren't protected by slab_mutex, it protects the
> iteration of the list.  I would suggest still taking the mutex here unless
> there's a reason to avoid it.

I don't think we prefer to sleep in oom path. Instead of acquiring the 
mutex, I think we can use list_for_each_entry_safe() to avoid the 
removal of kmem cache when printing the statistics.

> 
>> +	 */
>> +	list_for_each_entry(s, &slab_caches, list) {
>> +		if (!is_root_cache(s))
>> +			continue;
> 
> if (!(s->flags & SLAB_RECLAIM_ACCOUNT))
> 	continue;
> 
> No need to do the memset or get_slabinfo() if it's reclaimable, so just
> short-circuit it early in that case.
> 
>> +
>> +		memset(&sinfo, 0, sizeof(sinfo));
>> +		get_slabinfo(s, &sinfo);
>> +
>> +		if (!(s->flags & SLAB_RECLAIM_ACCOUNT) && sinfo.num_objs > 0)
>> +			pr_info("%-17s %10luKB %10luKB\n", cache_name(s), (sinfo.active_objs * s->size) / 1024, (sinfo.num_objs * s->size) / 1024);
>> +	}
>> +}
>> +
>>   #if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
>>   void *memcg_slab_start(struct seq_file *m, loff_t *pos)
>>   {
> 
> Please run scripts/checkpatch.pl on your patch since there's some
> stylistic problems.  Otherwise, I think we need one more revision and
> we'll be good to go!

Thanks, will prepare v5 soon.

Yang

> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ