lkml.org 
[lkml]   [2019]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] slab: fix a crash by reading /proc/slab_allocators
From
Date


On 4/7/19 9:59 PM, Tobin C. Harding wrote:
> On Sat, Apr 06, 2019 at 06:59:01PM -0400, Qian Cai wrote:
>> The commit 510ded33e075 ("slab: implement slab_root_caches list")
>> changes the name of the list node within "struct kmem_cache" from
>> "list" to "root_caches_node"
>
> Are you sure? It looks to me like it adds a member to the memcg_cache_array
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index a0cc7a77cda2..af1a5bef80f4 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -556,6 +556,8 @@ struct memcg_cache_array {
> * used to index child cachces during allocation and cleared
> * early during shutdown.
> *
> + * @root_caches_node: List node for slab_root_caches list.
> + *
> * @children: List of all child caches. While the child caches are also
> * reachable through @memcg_caches, a child cache remains on
> * this list until it is actually destroyed.
> @@ -573,6 +575,7 @@ struct memcg_cache_params {
> union {
> struct {
> struct memcg_cache_array __rcu *memcg_caches;
> + struct list_head __root_caches_node;
> struct list_head children;
> };
>
> And then defines 'root_caches_node' to be 'memcg_params.__root_caches_node'
> if we have CONFIG_MEMCG otherwise defines 'root_caches_node' to be 'list'
>
>
>> but leaks_show() still use the "list"
>
> I believe it should since 'list' is used to add to slab_caches list.

See the offensive commit 510ded33e075 which changed those.

@@ -1136,12 +1146,12 @@ static void print_slabinfo_header(struct seq_file *m)
void *slab_start(struct seq_file *m, loff_t *pos)
{
mutex_lock(&slab_mutex);
- return seq_list_start(&slab_caches, *pos);
+ return seq_list_start(&slab_root_caches, *pos);
}

void *slab_next(struct seq_file *m, void *p, loff_t *pos)
{
- return seq_list_next(p, &slab_caches, pos);
+ return seq_list_next(p, &slab_root_caches, pos);
}

and then memcg_link_cache() does,

if (is_root_cache(s)) {
list_add(&s->root_caches_node, &slab_root_caches);

memcg_unlink_cache() does,

if (is_root_cache(s)) {
list_del(&s->root_caches_node);

It also changed /proc/slabinfo but forgot to change /proc/slab_allocators.

@@ -1193,12 +1203,11 @@ static void cache_show(struct kmem_cache *s, struct
seq_file *m)

static int slab_show(struct seq_file *m, void *p)
{
- struct kmem_cache *s = list_entry(p, struct kmem_cache, list);
+ struct kmem_cache *s = list_entry(p, struct kmem_cache, root_caches_node);

>
>> which causes a crash when reading /proc/slab_allocators.
>
> I was unable to reproduce this crash, I built with
>
> # CONFIG_MEMCG is not set
> CONFIG_SLAB=y
> CONFIG_SLAB_MERGE_DEFAULT=y
> CONFIG_SLAB_FREELIST_RANDOM=y
> CONFIG_DEBUG_SLAB=y
> CONFIG_DEBUG_SLAB_LEAK=y
>
> I then booted in Qemu and successfully ran
> $ cat slab_allocators
>
> Perhaps you could post your config?

Yes, it won't be reproducible without CONFIG_MEMCG=y, because it has,

/* If !memcg, all caches are root. */
#define slab_root_caches slab_caches
#define root_caches_node list

Anyway,

https://git.sr.ht/~cai/linux-debug/blob/master/config

\
 
 \ /
  Last update: 2019-04-08 04:19    [W:0.090 / U:2.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site