* Re: Switch per cpu partial page support off for debugging
[not found] <alpine.DEB.2.00.1111230923260.16544@router.home>
@ 2011-12-12 7:16 ` Pekka Enberg
2011-12-12 7:23 ` Eric Dumazet
2011-12-15 3:10 ` David Rientjes
0 siblings, 2 replies; 8+ messages in thread
From: Pekka Enberg @ 2011-12-12 7:16 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Eric Dumazet, David Rientjes, linux-kernel
On Wed, 23 Nov 2011, Christoph Lameter wrote:
> Eric saw an issue with accounting of slabs during validation. Its not
> possible to determine accurately how many per cpu partial slabs exist at
> any time so this switches off per cpu partial pages during debug.
>
> Subject: Switch per cpu partial page support off for debugging
>
> Otherwise we have accounting issues.
>
> Signed-off-by: Christoph Lameter <cl@linux.com>
>
>
> ---
> mm/slub.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Index: linux-2.6/mm/slub.c
> ===================================================================
> --- linux-2.6.orig/mm/slub.c 2011-11-22 11:57:21.000000000 -0600
> +++ linux-2.6/mm/slub.c 2011-11-22 11:57:55.000000000 -0600
> @@ -3027,7 +3027,9 @@ static int kmem_cache_open(struct kmem_c
> * per node list when we run out of per cpu objects. We only fetch 50%
> * to keep some capacity around for frees.
> */
> - if (s->size >= PAGE_SIZE)
> + if (kmem_cache_debug(s))
> + s->cpu_partial = 0;
> + else if (s->size >= PAGE_SIZE)
> s->cpu_partial = 2;
> else if (s->size >= 1024)
> s->cpu_partial = 6;
Looks OK to me. CC'ing Eric and David.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Switch per cpu partial page support off for debugging
2011-12-12 7:16 ` Switch per cpu partial page support off for debugging Pekka Enberg
@ 2011-12-12 7:23 ` Eric Dumazet
2011-12-15 3:10 ` David Rientjes
1 sibling, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2011-12-12 7:23 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Christoph Lameter, David Rientjes, linux-kernel
Le lundi 12 décembre 2011 à 09:16 +0200, Pekka Enberg a écrit :
> On Wed, 23 Nov 2011, Christoph Lameter wrote:
> > Eric saw an issue with accounting of slabs during validation. Its not
> > possible to determine accurately how many per cpu partial slabs exist at
> > any time so this switches off per cpu partial pages during debug.
> >
> > Subject: Switch per cpu partial page support off for debugging
> >
> > Otherwise we have accounting issues.
> >
> > Signed-off-by: Christoph Lameter <cl@linux.com>
> >
> >
> > ---
> > mm/slub.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > Index: linux-2.6/mm/slub.c
> > ===================================================================
> > --- linux-2.6.orig/mm/slub.c 2011-11-22 11:57:21.000000000 -0600
> > +++ linux-2.6/mm/slub.c 2011-11-22 11:57:55.000000000 -0600
> > @@ -3027,7 +3027,9 @@ static int kmem_cache_open(struct kmem_c
> > * per node list when we run out of per cpu objects. We only fetch 50%
> > * to keep some capacity around for frees.
> > */
> > - if (s->size >= PAGE_SIZE)
> > + if (kmem_cache_debug(s))
> > + s->cpu_partial = 0;
> > + else if (s->size >= PAGE_SIZE)
> > s->cpu_partial = 2;
> > else if (s->size >= 1024)
> > s->cpu_partial = 6;
>
> Looks OK to me. CC'ing Eric and David.
This seems fine to me, thanks.
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Switch per cpu partial page support off for debugging
2011-12-12 7:16 ` Switch per cpu partial page support off for debugging Pekka Enberg
2011-12-12 7:23 ` Eric Dumazet
@ 2011-12-15 3:10 ` David Rientjes
2012-01-06 17:37 ` David Rientjes
1 sibling, 1 reply; 8+ messages in thread
From: David Rientjes @ 2011-12-15 3:10 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Christoph Lameter, Eric Dumazet, linux-kernel
On Mon, 12 Dec 2011, Pekka Enberg wrote:
> > Eric saw an issue with accounting of slabs during validation. Its not
> > possible to determine accurately how many per cpu partial slabs exist at
> > any time so this switches off per cpu partial pages during debug.
> >
> > Subject: Switch per cpu partial page support off for debugging
> >
> > Otherwise we have accounting issues.
> >
> > Signed-off-by: Christoph Lameter <cl@linux.com>
> >
> >
> > ---
> > mm/slub.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > Index: linux-2.6/mm/slub.c
> > ===================================================================
> > --- linux-2.6.orig/mm/slub.c 2011-11-22 11:57:21.000000000 -0600
> > +++ linux-2.6/mm/slub.c 2011-11-22 11:57:55.000000000 -0600
> > @@ -3027,7 +3027,9 @@ static int kmem_cache_open(struct kmem_c
> > * per node list when we run out of per cpu objects. We only fetch
> > 50%
> > * to keep some capacity around for frees.
> > */
> > - if (s->size >= PAGE_SIZE)
> > + if (kmem_cache_debug(s))
> > + s->cpu_partial = 0;
> > + else if (s->size >= PAGE_SIZE)
> > s->cpu_partial = 2;
> > else if (s->size >= 1024)
> > s->cpu_partial = 6;
I think this deserves a comment since it's not clear why it's being done
neither in the code or by the changelog. Also, you'd want a check for
kmem_cache_debug() when storing to /sys/kernel/slab/cache/cpu_partial as
well, otherwise this could easily be broken from userspace again. (It
also seems like Documentation/ABI/testing/sysfs-kernel-slab got left out
of an update when that was added, too.)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Switch per cpu partial page support off for debugging
2011-12-15 3:10 ` David Rientjes
@ 2012-01-06 17:37 ` David Rientjes
2012-01-08 10:54 ` Pekka Enberg
0 siblings, 1 reply; 8+ messages in thread
From: David Rientjes @ 2012-01-06 17:37 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Christoph Lameter, Eric Dumazet, linux-kernel
On Wed, 14 Dec 2011, David Rientjes wrote:
> > > Eric saw an issue with accounting of slabs during validation. Its not
> > > possible to determine accurately how many per cpu partial slabs exist at
> > > any time so this switches off per cpu partial pages during debug.
> > >
> > > Subject: Switch per cpu partial page support off for debugging
> > >
> > > Otherwise we have accounting issues.
> > >
> > > Signed-off-by: Christoph Lameter <cl@linux.com>
> > >
> > >
> > > ---
> > > mm/slub.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > Index: linux-2.6/mm/slub.c
> > > ===================================================================
> > > --- linux-2.6.orig/mm/slub.c 2011-11-22 11:57:21.000000000 -0600
> > > +++ linux-2.6/mm/slub.c 2011-11-22 11:57:55.000000000 -0600
> > > @@ -3027,7 +3027,9 @@ static int kmem_cache_open(struct kmem_c
> > > * per node list when we run out of per cpu objects. We only fetch
> > > 50%
> > > * to keep some capacity around for frees.
> > > */
> > > - if (s->size >= PAGE_SIZE)
> > > + if (kmem_cache_debug(s))
> > > + s->cpu_partial = 0;
> > > + else if (s->size >= PAGE_SIZE)
> > > s->cpu_partial = 2;
> > > else if (s->size >= 1024)
> > > s->cpu_partial = 6;
>
> I think this deserves a comment since it's not clear why it's being done
> neither in the code or by the changelog. Also, you'd want a check for
> kmem_cache_debug() when storing to /sys/kernel/slab/cache/cpu_partial as
> well, otherwise this could easily be broken from userspace again. (It
> also seems like Documentation/ABI/testing/sysfs-kernel-slab got left out
> of an update when that was added, too.)
>
This patch is merged in slab/urgent without any changes from my comment
above; since userspace can alter cpu_partial directly via
/sys/kernel/slab/cache/cpu_partial which would reintroduce the
"accounting issues", shouldn't that return -EINVAL when debug is enabled?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Switch per cpu partial page support off for debugging
2012-01-06 17:37 ` David Rientjes
@ 2012-01-08 10:54 ` Pekka Enberg
2012-01-09 14:56 ` Christoph Lameter
0 siblings, 1 reply; 8+ messages in thread
From: Pekka Enberg @ 2012-01-08 10:54 UTC (permalink / raw)
To: David Rientjes; +Cc: Christoph Lameter, Eric Dumazet, linux-kernel
On Fri, 2012-01-06 at 09:37 -0800, David Rientjes wrote:
> On Wed, 14 Dec 2011, David Rientjes wrote:
>
> > > > Eric saw an issue with accounting of slabs during validation. Its not
> > > > possible to determine accurately how many per cpu partial slabs exist at
> > > > any time so this switches off per cpu partial pages during debug.
> > > >
> > > > Subject: Switch per cpu partial page support off for debugging
> > > >
> > > > Otherwise we have accounting issues.
> > > >
> > > > Signed-off-by: Christoph Lameter <cl@linux.com>
> > > >
> > > >
> > > > ---
> > > > mm/slub.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > Index: linux-2.6/mm/slub.c
> > > > ===================================================================
> > > > --- linux-2.6.orig/mm/slub.c 2011-11-22 11:57:21.000000000 -0600
> > > > +++ linux-2.6/mm/slub.c 2011-11-22 11:57:55.000000000 -0600
> > > > @@ -3027,7 +3027,9 @@ static int kmem_cache_open(struct kmem_c
> > > > * per node list when we run out of per cpu objects. We only fetch
> > > > 50%
> > > > * to keep some capacity around for frees.
> > > > */
> > > > - if (s->size >= PAGE_SIZE)
> > > > + if (kmem_cache_debug(s))
> > > > + s->cpu_partial = 0;
> > > > + else if (s->size >= PAGE_SIZE)
> > > > s->cpu_partial = 2;
> > > > else if (s->size >= 1024)
> > > > s->cpu_partial = 6;
> >
> > I think this deserves a comment since it's not clear why it's being done
> > neither in the code or by the changelog. Also, you'd want a check for
> > kmem_cache_debug() when storing to /sys/kernel/slab/cache/cpu_partial as
> > well, otherwise this could easily be broken from userspace again. (It
> > also seems like Documentation/ABI/testing/sysfs-kernel-slab got left out
> > of an update when that was added, too.)
> >
>
> This patch is merged in slab/urgent without any changes from my comment
> above; since userspace can alter cpu_partial directly via
> /sys/kernel/slab/cache/cpu_partial which would reintroduce the
> "accounting issues", shouldn't that return -EINVAL when debug is enabled?
I think I applied the patch before your comments. EINVAL sounds
reasonable to me. Christoph?
Pekka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Switch per cpu partial page support off for debugging
2012-01-08 10:54 ` Pekka Enberg
@ 2012-01-09 14:56 ` Christoph Lameter
2012-01-09 21:19 ` [patch slab/urgent] disallow changing cpu_partial from userspace for debug caches David Rientjes
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Lameter @ 2012-01-09 14:56 UTC (permalink / raw)
To: Pekka Enberg; +Cc: David Rientjes, Eric Dumazet, linux-kernel
On Sun, 8 Jan 2012, Pekka Enberg wrote:
> I think I applied the patch before your comments. EINVAL sounds
> reasonable to me. Christoph?
Sounds all fine to me. Hope I get around to it this week.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch slab/urgent] disallow changing cpu_partial from userspace for debug caches
2012-01-09 14:56 ` Christoph Lameter
@ 2012-01-09 21:19 ` David Rientjes
2012-01-10 16:49 ` Christoph Lameter
0 siblings, 1 reply; 8+ messages in thread
From: David Rientjes @ 2012-01-09 21:19 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Christoph Lameter, Eric Dumazet, linux-kernel
For caches with debugging enabled, "slub: Switch per cpu partial page
support off for debugging" changes cpu_partial to 0. It shouldn't be
tunable from userspace for such caches, otherwise the same accounting
issues arise during validation.
This patch disallows tuning /sys/kernel/slab/cache/cpu_partial to be non-
zero for caches with debugging enabled.
Signed-off-by: David Rientjes <rientjes@google.com>
---
mm/slub.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4639,6 +4639,8 @@ static ssize_t cpu_partial_store(struct kmem_cache *s, const char *buf,
err = strict_strtoul(buf, 10, &objects);
if (err)
return err;
+ if (objects && kmem_cache_debug(s))
+ return -EINVAL;
s->cpu_partial = objects;
flush_all(s);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch slab/urgent] disallow changing cpu_partial from userspace for debug caches
2012-01-09 21:19 ` [patch slab/urgent] disallow changing cpu_partial from userspace for debug caches David Rientjes
@ 2012-01-10 16:49 ` Christoph Lameter
0 siblings, 0 replies; 8+ messages in thread
From: Christoph Lameter @ 2012-01-10 16:49 UTC (permalink / raw)
To: David Rientjes; +Cc: Pekka Enberg, Eric Dumazet, linux-kernel
On Mon, 9 Jan 2012, David Rientjes wrote:
> This patch disallows tuning /sys/kernel/slab/cache/cpu_partial to be non-
> zero for caches with debugging enabled.
Acked-by: Christoph Lameter <cl@linux.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-01-10 16:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <alpine.DEB.2.00.1111230923260.16544@router.home>
2011-12-12 7:16 ` Switch per cpu partial page support off for debugging Pekka Enberg
2011-12-12 7:23 ` Eric Dumazet
2011-12-15 3:10 ` David Rientjes
2012-01-06 17:37 ` David Rientjes
2012-01-08 10:54 ` Pekka Enberg
2012-01-09 14:56 ` Christoph Lameter
2012-01-09 21:19 ` [patch slab/urgent] disallow changing cpu_partial from userspace for debug caches David Rientjes
2012-01-10 16:49 ` Christoph Lameter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).