From: Christopher Lameter <cl@linux.com> To: Mikulas Patocka <mpatocka@redhat.com> Cc: Matthew Wilcox <willy@infradead.org>, Pekka Enberg <penberg@kernel.org>, David Rientjes <rientjes@google.com>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, dm-devel@redhat.com, Mike Snitzer <msnitzer@redhat.com> Subject: Re: [PATCH] slab: introduce the flag SLAB_MINIMIZE_WASTE Date: Fri, 23 Mar 2018 10:10:26 -0500 (CDT) [thread overview] Message-ID: <alpine.DEB.2.20.1803230956420.4108@nuc-kabylake> (raw) In-Reply-To: <alpine.LRH.2.02.1803211613010.28365@file01.intranet.prod.int.rdu2.redhat.com> On Wed, 21 Mar 2018, Mikulas Patocka wrote: > > + s->allocflags = allocflags; > > I'd also use "WRITE_ONCE(s->allocflags, allocflags)" here and when writing > s->oo and s->min to avoid some possible compiler misoptimizations. It only matters that 0 etc is never written. > Another problem is that it updates s->oo and later it updates s->max: > s->oo = oo_make(order, size, s->reserved); > s->min = oo_make(get_order(size), size, s->reserved); > if (oo_objects(s->oo) > oo_objects(s->max)) > s->max = s->oo; > --- so, the concurrently running code could see s->oo > s->max, which > could trigger some memory corruption. Well s->max is only relevant for code that analyses the details of slab structures for diagnostics. > s->max is only used in memory allocations - > kmalloc(BITS_TO_LONGS(oo_objects(s->max)) * sizeof(unsigned long)), so > perhaps we could fix the bug by removing s->max at all and always > allocating enough memory for the maximum possible number of objects? > > - kmalloc(BITS_TO_LONGS(oo_objects(s->max)) * sizeof(unsigned long), GFP_KERNEL); > + kmalloc(BITS_TO_LONGS(MAX_OBJS_PER_PAGE) * sizeof(unsigned long), GFP_KERNEL); MAX_OBJS_PER_PAGE is 32k. So you are looking at contiguous allocations of 256kbyte. Not good. The simplest measure would be to disallow the changing of the order while the slab contains objects. Subject: slub: Disallow order changes when objects exist in a slab There seems to be a couple of races that would have to be addressed if the slab order would be changed during active use. Lets disallow this in the same way as we also do not allow other changes of slab characteristics when objects are active. Signed-off-by: Christoph Lameter <cl@linux.com> Index: linux/mm/slub.c =================================================================== --- linux.orig/mm/slub.c +++ linux/mm/slub.c @@ -4919,6 +4919,9 @@ static ssize_t order_store(struct kmem_c unsigned long order; int err; + if (any_slab_objects(s)) + return -EBUSY; + err = kstrtoul(buf, 10, &order); if (err) return err;
WARNING: multiple messages have this Message-ID (diff)
From: Christopher Lameter <cl@linux.com> To: Mikulas Patocka <mpatocka@redhat.com> Cc: Mike Snitzer <msnitzer@redhat.com>, Matthew Wilcox <willy@infradead.org>, Pekka Enberg <penberg@kernel.org>, linux-mm@kvack.org, dm-devel@redhat.com, David Rientjes <rientjes@google.com>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, Andrew Morton <akpm@linux-foundation.org> Subject: Re: [PATCH] slab: introduce the flag SLAB_MINIMIZE_WASTE Date: Fri, 23 Mar 2018 10:10:26 -0500 (CDT) [thread overview] Message-ID: <alpine.DEB.2.20.1803230956420.4108@nuc-kabylake> (raw) In-Reply-To: <alpine.LRH.2.02.1803211613010.28365@file01.intranet.prod.int.rdu2.redhat.com> On Wed, 21 Mar 2018, Mikulas Patocka wrote: > > + s->allocflags = allocflags; > > I'd also use "WRITE_ONCE(s->allocflags, allocflags)" here and when writing > s->oo and s->min to avoid some possible compiler misoptimizations. It only matters that 0 etc is never written. > Another problem is that it updates s->oo and later it updates s->max: > s->oo = oo_make(order, size, s->reserved); > s->min = oo_make(get_order(size), size, s->reserved); > if (oo_objects(s->oo) > oo_objects(s->max)) > s->max = s->oo; > --- so, the concurrently running code could see s->oo > s->max, which > could trigger some memory corruption. Well s->max is only relevant for code that analyses the details of slab structures for diagnostics. > s->max is only used in memory allocations - > kmalloc(BITS_TO_LONGS(oo_objects(s->max)) * sizeof(unsigned long)), so > perhaps we could fix the bug by removing s->max at all and always > allocating enough memory for the maximum possible number of objects? > > - kmalloc(BITS_TO_LONGS(oo_objects(s->max)) * sizeof(unsigned long), GFP_KERNEL); > + kmalloc(BITS_TO_LONGS(MAX_OBJS_PER_PAGE) * sizeof(unsigned long), GFP_KERNEL); MAX_OBJS_PER_PAGE is 32k. So you are looking at contiguous allocations of 256kbyte. Not good. The simplest measure would be to disallow the changing of the order while the slab contains objects. Subject: slub: Disallow order changes when objects exist in a slab There seems to be a couple of races that would have to be addressed if the slab order would be changed during active use. Lets disallow this in the same way as we also do not allow other changes of slab characteristics when objects are active. Signed-off-by: Christoph Lameter <cl@linux.com> Index: linux/mm/slub.c =================================================================== --- linux.orig/mm/slub.c +++ linux/mm/slub.c @@ -4919,6 +4919,9 @@ static ssize_t order_store(struct kmem_c unsigned long order; int err; + if (any_slab_objects(s)) + return -EBUSY; + err = kstrtoul(buf, 10, &order); if (err) return err;
next prev parent reply other threads:[~2018-03-23 15:10 UTC|newest] Thread overview: 109+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-20 17:25 [PATCH] slab: introduce the flag SLAB_MINIMIZE_WASTE Mikulas Patocka 2018-03-20 17:35 ` Matthew Wilcox 2018-03-20 17:35 ` Matthew Wilcox 2018-03-20 17:54 ` Christopher Lameter 2018-03-20 17:54 ` Christopher Lameter 2018-03-20 19:22 ` Mikulas Patocka 2018-03-20 19:22 ` Mikulas Patocka 2018-03-20 20:42 ` Christopher Lameter 2018-03-20 20:42 ` Christopher Lameter 2018-03-20 22:02 ` Mikulas Patocka 2018-03-20 22:02 ` Mikulas Patocka 2018-03-21 15:35 ` Christopher Lameter 2018-03-21 15:35 ` Christopher Lameter 2018-03-21 16:25 ` Mikulas Patocka 2018-03-21 16:25 ` Mikulas Patocka 2018-03-21 17:10 ` Matthew Wilcox 2018-03-21 17:10 ` Matthew Wilcox 2018-03-21 17:30 ` Christopher Lameter 2018-03-21 17:30 ` Christopher Lameter 2018-03-21 17:39 ` Christopher Lameter 2018-03-21 17:39 ` Christopher Lameter 2018-03-21 17:49 ` Matthew Wilcox 2018-03-21 17:49 ` Matthew Wilcox 2018-03-21 18:01 ` Christopher Lameter 2018-03-21 18:01 ` Christopher Lameter 2018-03-21 18:23 ` Mikulas Patocka 2018-03-21 18:23 ` Mikulas Patocka 2018-03-21 18:40 ` Christopher Lameter 2018-03-21 18:40 ` Christopher Lameter 2018-03-21 18:55 ` Mikulas Patocka 2018-03-21 18:55 ` Mikulas Patocka 2018-03-21 18:55 ` Matthew Wilcox 2018-03-21 18:55 ` Matthew Wilcox 2018-03-21 18:58 ` Christopher Lameter 2018-03-21 18:58 ` Christopher Lameter 2018-03-21 19:25 ` Mikulas Patocka 2018-03-21 19:25 ` Mikulas Patocka 2018-03-21 18:36 ` Mikulas Patocka 2018-03-21 18:36 ` Mikulas Patocka 2018-03-21 18:57 ` Christopher Lameter 2018-03-21 18:57 ` Christopher Lameter 2018-03-21 19:19 ` Mikulas Patocka 2018-03-21 19:19 ` Mikulas Patocka 2018-03-21 20:09 ` Christopher Lameter 2018-03-21 20:09 ` Christopher Lameter 2018-03-21 20:37 ` Mikulas Patocka 2018-03-21 20:37 ` Mikulas Patocka 2018-03-23 15:10 ` Christopher Lameter [this message] 2018-03-23 15:10 ` Christopher Lameter 2018-03-23 15:31 ` Mikulas Patocka 2018-03-23 15:31 ` Mikulas Patocka 2018-03-23 15:48 ` Christopher Lameter 2018-03-23 15:48 ` Christopher Lameter 2018-04-13 9:22 ` Vlastimil Babka 2018-04-13 9:22 ` Vlastimil Babka 2018-04-13 15:10 ` Mike Snitzer 2018-04-13 15:10 ` Mike Snitzer 2018-04-16 12:38 ` Vlastimil Babka 2018-04-16 12:38 ` Vlastimil Babka 2018-04-16 14:27 ` Mike Snitzer 2018-04-16 14:27 ` Mike Snitzer 2018-04-16 14:37 ` Mikulas Patocka 2018-04-16 14:37 ` Mikulas Patocka 2018-04-16 14:46 ` Mike Snitzer 2018-04-16 14:46 ` Mike Snitzer 2018-04-16 14:57 ` Mikulas Patocka 2018-04-16 14:57 ` Mikulas Patocka 2018-04-16 15:18 ` Christopher Lameter 2018-04-16 15:18 ` Christopher Lameter 2018-04-16 15:25 ` Mikulas Patocka 2018-04-16 15:25 ` Mikulas Patocka 2018-04-16 15:45 ` Christopher Lameter 2018-04-16 15:45 ` Christopher Lameter 2018-04-16 19:36 ` Mikulas Patocka 2018-04-16 19:36 ` Mikulas Patocka 2018-04-16 19:53 ` Vlastimil Babka 2018-04-16 21:01 ` Mikulas Patocka 2018-04-17 14:40 ` Christopher Lameter 2018-04-17 18:53 ` Mikulas Patocka 2018-04-17 18:53 ` Mikulas Patocka 2018-04-17 21:42 ` Christopher Lameter 2018-04-17 14:49 ` Christopher Lameter 2018-04-17 14:49 ` Christopher Lameter 2018-04-17 14:47 ` Christopher Lameter 2018-04-17 14:47 ` Christopher Lameter 2018-04-16 19:32 ` [PATCH RESEND] " Mikulas Patocka 2018-04-17 14:45 ` Christopher Lameter 2018-04-17 16:16 ` Vlastimil Babka 2018-04-17 16:38 ` Christopher Lameter 2018-04-17 19:09 ` Mikulas Patocka 2018-04-17 17:26 ` Mikulas Patocka 2018-04-17 19:13 ` Vlastimil Babka 2018-04-17 19:06 ` Mikulas Patocka 2018-04-17 19:06 ` Mikulas Patocka 2018-04-18 14:55 ` Christopher Lameter 2018-04-25 21:04 ` Mikulas Patocka 2018-04-25 23:24 ` Mikulas Patocka 2018-04-26 19:01 ` Christopher Lameter 2018-04-26 21:09 ` Mikulas Patocka 2018-04-27 16:41 ` Christopher Lameter 2018-04-27 19:19 ` Mikulas Patocka 2018-06-13 17:01 ` Mikulas Patocka 2018-06-13 18:16 ` Christoph Hellwig 2018-06-13 18:53 ` Mikulas Patocka 2018-04-26 18:51 ` Christopher Lameter 2018-04-16 19:38 ` Vlastimil Babka 2018-04-16 19:38 ` Vlastimil Babka 2018-04-16 21:04 ` Mikulas Patocka 2018-04-16 21:04 ` Mikulas Patocka
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.DEB.2.20.1803230956420.4108@nuc-kabylake \ --to=cl@linux.com \ --cc=akpm@linux-foundation.org \ --cc=dm-devel@redhat.com \ --cc=iamjoonsoo.kim@lge.com \ --cc=linux-mm@kvack.org \ --cc=mpatocka@redhat.com \ --cc=msnitzer@redhat.com \ --cc=penberg@kernel.org \ --cc=rientjes@google.com \ --cc=willy@infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.