[PATCH] slab: Ignore internal flags in cache creation

[PATCH] slab: Ignore internal flags in cache creation

[Glauber Costa]

Some flags are used internally by the allocators for management
purposes. One example of that is the CFLGS_OFF_SLAB flag that slab uses
to mark that the metadata for that cache is stored outside of the slab.

No cache should ever pass those as a creation flags. We can just ignore
this bit if it happens to be passed (such as when duplicating a cache in
the kmem memcg patches)

Signed-off-by: Glauber Costa <glommer@parallels.com>
CC: Christoph Lameter <cl@linux.com>
CC: Pekka Enberg <penberg@cs.helsinki.fi>
CC: David Rientjes <rientjes@google.com>
---
 include/linux/slab.h | 4 ++++
 mm/slab_common.c     | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 0dd2dfa..437c07e 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -79,6 +79,10 @@
 /* The following flags affect the page allocator grouping pages by mobility */
 #define SLAB_RECLAIM_ACCOUNT	0x00020000UL		/* Objects are reclaimable */
 #define SLAB_TEMPORARY		SLAB_RECLAIM_ACCOUNT	/* Objects are short-lived */
+
+/* The last flags are reserved for specific internal flags of the allocators */
+#define SLAB_INTERNAL 0xF0000000UL
+
 /*
  * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
  *
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 9c21725..359ef36 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -107,6 +107,11 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size, size_t align
 	if (!kmem_cache_sanity_check(name, size) == 0)
 		goto out_locked;
 
+	/*
+	 * Clean any possible internal flags the caller may have passed.
+	 * We'll make those decisions ourselves.
+	 */
+	flags &= ~SLAB_INTERNAL;
 
 	s = __kmem_cache_alias(name, size, align, flags, ctor);
 	if (s)
-- 
1.7.11.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Tue, 25 Sep 2012, Glauber Costa wrote:

> No cache should ever pass those as a creation flags. We can just ignore
> this bit if it happens to be passed (such as when duplicating a cache in
> the kmem memcg patches)

Acked-by: Christoph Lameter <cl@linux.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Tue, 25 Sep 2012, Christoph Lameter wrote:

> > No cache should ever pass those as a creation flags. We can just ignore
> > this bit if it happens to be passed (such as when duplicating a cache in
> > the kmem memcg patches)
> 
> Acked-by: Christoph Lameter <cl@linux.com>
> 

Nack, this is already handled by CREATE_MASK in the mm/slab.c allocator; 
the flag extensions beyond those defined in the generic slab.h header are 
implementation defined.  It may be true that SLAB uses a bit only 
internally (and already protects it with a BUG_ON() in 
__kmem_cache_create()) but that doesn't mean other implementations can't 
use such a flag that would be a no-op on another allocator.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Glauber Costa]

On 09/26/2012 04:46 AM, David Rientjes wrote:
> On Tue, 25 Sep 2012, Christoph Lameter wrote:
> 
>>> No cache should ever pass those as a creation flags. We can just ignore
>>> this bit if it happens to be passed (such as when duplicating a cache in
>>> the kmem memcg patches)
>>
>> Acked-by: Christoph Lameter <cl@linux.com>
>>
> 
> Nack, this is already handled by CREATE_MASK in the mm/slab.c allocator; 
> the flag extensions beyond those defined in the generic slab.h header are 
> implementation defined.  It may be true that SLAB uses a bit only 
> internally (and already protects it with a BUG_ON() in 
> __kmem_cache_create()) but that doesn't mean other implementations can't 
> use such a flag that would be a no-op on another allocator.
> 

So the problem I am facing here is that when I am creating caches from
memcg, I would very much like to reuse their flags fields. They are
stored in the cache itself, so this is not a problem. But slab also
stores that flag, leading to the precise BUG_ON() on CREATE_MASK that
you quoted.

In this context, passing this flag becomes completely valid, I just need
that to be explicitly masked out.

What is your suggestion to handle this ?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Wed, 26 Sep 2012, Glauber Costa wrote:

> So the problem I am facing here is that when I am creating caches from
> memcg, I would very much like to reuse their flags fields. They are
> stored in the cache itself, so this is not a problem. But slab also
> stores that flag, leading to the precise BUG_ON() on CREATE_MASK that
> you quoted.
> 
> In this context, passing this flag becomes completely valid, I just need
> that to be explicitly masked out.
> 
> What is your suggestion to handle this ?
> 

I would suggest cachep->flags being used solely for the flags passed to 
kmem_cache_create() and seperating out all "internal flags" based on the 
individual slab allocator's implementation into a different field.  There 
should be no problem with moving CFLGS_OFF_SLAB elsewhere, in fact, I just 
removed a "dflags" field from mm/slab.c's kmem_cache that turned out never 
to be used.  You could simply reintroduce a new "internal_flags" field and 
use it at your discretion.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Glauber Costa]

On 09/27/2012 05:16 AM, David Rientjes wrote:
> On Wed, 26 Sep 2012, Glauber Costa wrote:
> 
>> So the problem I am facing here is that when I am creating caches from
>> memcg, I would very much like to reuse their flags fields. They are
>> stored in the cache itself, so this is not a problem. But slab also
>> stores that flag, leading to the precise BUG_ON() on CREATE_MASK that
>> you quoted.
>>
>> In this context, passing this flag becomes completely valid, I just need
>> that to be explicitly masked out.
>>
>> What is your suggestion to handle this ?
>>
> 
> I would suggest cachep->flags being used solely for the flags passed to 
> kmem_cache_create() and seperating out all "internal flags" based on the 
> individual slab allocator's implementation into a different field.  There 
> should be no problem with moving CFLGS_OFF_SLAB elsewhere, in fact, I just 
> removed a "dflags" field from mm/slab.c's kmem_cache that turned out never 
> to be used.  You could simply reintroduce a new "internal_flags" field and 
> use it at your discretion.
> 
I can do it with you both agree with the approach.

But I still don't see the big reason for your objection. If other
allocator start using those bits, they would not be passed to
kmem_cache_alloc anyway, right? So what would be the big problem in
masking them out before it?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Thu, 27 Sep 2012, Glauber Costa wrote:

> But I still don't see the big reason for your objection. If other
> allocator start using those bits, they would not be passed to
> kmem_cache_alloc anyway, right? So what would be the big problem in
> masking them out before it?
> 

A slab allocator implementation may allow for additional bits that are 
currently not used or used for internal purposes by the current set of 
slab allocators to be passed in the unsigned long to kmem_cache_create() 
that would be a no-op on other allocators.  It's implementation defined, 
so this masking should be done in the implementation, i.e. 
__kmem_cache_create().

For context, as many people who attended the kernel summit and LinuxCon 
are aware, a new slab allocator is going to be proposed soon that actually 
uses additional bits that aren't defined for all slab allocators.  My 
opinion is that leaving unused bits and reserved bits to the 
implementation is the best software engineering practice.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Glauber Costa]

On 09/28/2012 02:56 AM, David Rientjes wrote:
> On Thu, 27 Sep 2012, Glauber Costa wrote:
> 
>> But I still don't see the big reason for your objection. If other
>> allocator start using those bits, they would not be passed to
>> kmem_cache_alloc anyway, right? So what would be the big problem in
>> masking them out before it?
>>
> 
> A slab allocator implementation may allow for additional bits that are 
> currently not used or used for internal purposes by the current set of 
> slab allocators to be passed in the unsigned long to kmem_cache_create() 
> that would be a no-op on other allocators.  It's implementation defined, 
> so this masking should be done in the implementation, i.e. 
> __kmem_cache_create().
> 
> For context, as many people who attended the kernel summit and LinuxCon 
> are aware, a new slab allocator is going to be proposed soon that actually 
> uses additional bits that aren't defined for all slab allocators.  My 
> opinion is that leaving unused bits and reserved bits to the 
> implementation is the best software engineering practice.
> 

I am happy as long as we don't BUG and can mask out that feature.
If Christoph is happy with me masking it in the SLAB only, I'm also fine.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Fri, 28 Sep 2012, Glauber Costa wrote:

> I am happy as long as we don't BUG and can mask out that feature.
> If Christoph is happy with me masking it in the SLAB only, I'm also fine.
> 

Absolutely, I agree that the implementation-defined __kmem_cache_create() 
can mask out bits that are not useful on the particular allocator.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Thu, 27 Sep 2012, David Rientjes wrote:

> On Thu, 27 Sep 2012, Glauber Costa wrote:
>
> > But I still don't see the big reason for your objection. If other
> > allocator start using those bits, they would not be passed to
> > kmem_cache_alloc anyway, right? So what would be the big problem in
> > masking them out before it?
> >
>
> A slab allocator implementation may allow for additional bits that are
> currently not used or used for internal purposes by the current set of
> slab allocators to be passed in the unsigned long to kmem_cache_create()
> that would be a no-op on other allocators.  It's implementation defined,
> so this masking should be done in the implementation, i.e.
> __kmem_cache_create().

Ok we can do that in the future. There is nothing in this patch that
prevents that from happening. It would affect the memcg implementation
because they can no longer simply grab the flags and pass them in for
creating another slab.

> For context, as many people who attended the kernel summit and LinuxCon
> are aware, a new slab allocator is going to be proposed soon that actually
> uses additional bits that aren't defined for all slab allocators.  My
> opinion is that leaving unused bits and reserved bits to the
> implementation is the best software engineering practice.

Could you please come out with the new allocator and post some patchsets?

We can extend the number of flags reserved if necessary but we really need
to see the source for that.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Fri, 28 Sep 2012, Christoph Lameter wrote:

> > For context, as many people who attended the kernel summit and LinuxCon
> > are aware, a new slab allocator is going to be proposed soon that actually
> > uses additional bits that aren't defined for all slab allocators.  My
> > opinion is that leaving unused bits and reserved bits to the
> > implementation is the best software engineering practice.
> 
> Could you please come out with the new allocator and post some patchsets?
> 
> We can extend the number of flags reserved if necessary but we really need
> to see the source for that.
> 

The first prototype, SLAM XP1, will be posted in October.  I'd simply like 
to avoid reverting this patch down the road and having all of us 
reconsider the topic again when clear alternatives exist that, in my 
opinion, make the code cleaner.

 [ There _was_ a field for internal flags for mm/slab.c, called "dflags", 
   before I removed it because it was unused; I'm regretting that now 
   because it would have been very easy to use it for this purpose. ]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Fri, 28 Sep 2012, David Rientjes wrote:

> The first prototype, SLAM XP1, will be posted in October.  I'd simply like
> to avoid reverting this patch down the road and having all of us
> reconsider the topic again when clear alternatives exist that, in my
> opinion, make the code cleaner.

If you want to make changes to the kernel then you need to justify that at
the time when we can consider your patches and the approach taken.




--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Fri, 28 Sep 2012, Christoph Lameter wrote:

> > The first prototype, SLAM XP1, will be posted in October.  I'd simply like
> > to avoid reverting this patch down the road and having all of us
> > reconsider the topic again when clear alternatives exist that, in my
> > opinion, make the code cleaner.
> 
> If you want to make changes to the kernel then you need to justify that at
> the time when we can consider your patches and the approach taken.
> 

If I cannot speak up and say where there will be conflicts in the future 
and ask that Glauber spend more of his time down the road to figure all of 
this out again, especially when a simple and clean alternative exists, 
then that seems to result in a big waste of time.  Nothing is suffering 
from taking the alternative here, so please follow the best software 
engineering practices of allowing an implementation to reserve and ignore 
bits in an API when appropriate and not do it globally in the common code.

All of the move to mm/slab_common.c has obviously slowed down posting of 
SLAM and I haven't complained about that once or asked that it not be 
done, I'm simply pointing out an instance here that will conflict later on 
if we go with this patch.  That, to me, is respectful of other people's 
time.  That said, I'll leave it to Glauber to decide how he'd like to 
handle this issue given the knowledge of what is to come.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Fri, 28 Sep 2012, David Rientjes wrote:

> All of the move to mm/slab_common.c has obviously slowed down posting of
> SLAM and I haven't complained about that once or asked that it not be
> done, I'm simply pointing out an instance here that will conflict later on
> if we go with this patch.  That, to me, is respectful of other people's
> time.  That said, I'll leave it to Glauber to decide how he'd like to
> handle this issue given the knowledge of what is to come.

I do not mind if you post a version against some older kernel. We can
then see what is going on and come to an agreement on how to move forward.
I just want to finally *see* the patches instead of just hot talk.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Pekka Enberg]

Hello,

[ Found this in my @cs.helsinki.fi inbox, grmbl. ]

On Fri, Sep 28, 2012 at 11:39 PM, David Rientjes <rientjes@google.com> wrote:
> The first prototype, SLAM XP1, will be posted in October.  I'd simply like
> to avoid reverting this patch down the road and having all of us
> reconsider the topic again when clear alternatives exist that, in my
> opinion, make the code cleaner.

David, I'm sure you know we don't work speculatively against
out-of-tree code that may or may not be include in the future...

That said, I don't like Glauber's patch because it leaves CREATE_MASK
in mm/slab.c. And I'm not totally convinced a generic SLAB_INTERNAL is
going to cut it either. Hmm.

                        Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Glauber Costa]

On 10/01/2012 11:28 AM, Pekka Enberg wrote:
> Hello,
> 
> [ Found this in my @cs.helsinki.fi inbox, grmbl. ]
> 
> On Fri, Sep 28, 2012 at 11:39 PM, David Rientjes <rientjes@google.com> wrote:
>> The first prototype, SLAM XP1, will be posted in October.  I'd simply like
>> to avoid reverting this patch down the road and having all of us
>> reconsider the topic again when clear alternatives exist that, in my
>> opinion, make the code cleaner.
> 
> David, I'm sure you know we don't work speculatively against
> out-of-tree code that may or may not be include in the future...
> 
> That said, I don't like Glauber's patch because it leaves CREATE_MASK
> in mm/slab.c. And I'm not totally convinced a generic SLAB_INTERNAL is
> going to cut it either. Hmm.
> 
>                         Pekka
> 

How about we require allocators to define their own CREATE_MASK, and
then in slab_common.c we mask out any flags outside that mask?

This way we can achieve masking in common code while still leaving the
decision to the allocators.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Wed, 26 Sep 2012, David Rientjes wrote:

> I would suggest cachep->flags being used solely for the flags passed to
> kmem_cache_create() and seperating out all "internal flags" based on the
> individual slab allocator's implementation into a different field.  There
> should be no problem with moving CFLGS_OFF_SLAB elsewhere, in fact, I just
> removed a "dflags" field from mm/slab.c's kmem_cache that turned out never
> to be used.  You could simply reintroduce a new "internal_flags" field and
> use it at your discretion.

This means touching another field from critical paths of the allocators.
It would increase the cache footprint and therefore reduce performance.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Thu, 27 Sep 2012, Christoph Lameter wrote:

> > I would suggest cachep->flags being used solely for the flags passed to
> > kmem_cache_create() and seperating out all "internal flags" based on the
> > individual slab allocator's implementation into a different field.  There
> > should be no problem with moving CFLGS_OFF_SLAB elsewhere, in fact, I just
> > removed a "dflags" field from mm/slab.c's kmem_cache that turned out never
> > to be used.  You could simply reintroduce a new "internal_flags" field and
> > use it at your discretion.
> 
> This means touching another field from critical paths of the allocators.
> It would increase the cache footprint and therefore reduce performance.
> 

To clarify your statement, you're referring to the mm/slab.c allocation of 
new slab pages and when debugging is enabled as "critical paths", correct?  
We would disagree on that point.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Thu, 27 Sep 2012, David Rientjes wrote:

> > This means touching another field from critical paths of the allocators.
> > It would increase the cache footprint and therefore reduce performance.
> >
>
> To clarify your statement, you're referring to the mm/slab.c allocation of
> new slab pages and when debugging is enabled as "critical paths", correct?
> We would disagree on that point.

This is not debugging specific. Flags are also consulted to do RCU
processing and other things.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Fri, 28 Sep 2012, Christoph Lameter wrote:

> > > This means touching another field from critical paths of the allocators.
> > > It would increase the cache footprint and therefore reduce performance.
> > >
> >
> > To clarify your statement, you're referring to the mm/slab.c allocation of
> > new slab pages and when debugging is enabled as "critical paths", correct?
> > We would disagree on that point.
> 
> This is not debugging specific. Flags are also consulted to do RCU
> processing and other things.
> 

There is no "critical path" in mm/slab.c that tests CFLGS_OFF_SLAB; the 
flag itself is used to determine where slab management is done and you 
certainly wouldn't want to find this for any path that is critical.

If you'd like to disagree with that, please show the code and why you 
consider increasing the cache footprint in that case to be critical to 
performance.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Tue, 25 Sep 2012, David Rientjes wrote:

> Nack, this is already handled by CREATE_MASK in the mm/slab.c allocator;

CREATE_MASK defines legal flags that can be specified. Other flags cause
and error. This is about flags that are internal that should be ignored
when specified.

I think it makes sense to reserve some top flags for internal purposes.

> the flag extensions beyond those defined in the generic slab.h header are
> implementation defined.  It may be true that SLAB uses a bit only
> internally (and already protects it with a BUG_ON() in
> __kmem_cache_create()) but that doesn't mean other implementations can't
> use such a flag that would be a no-op on another allocator.

Other implementations such as SLUB also use the bits in that high range.

Simply ignoring the internal bits on cache creation if they are set is
IMHO not a bit issue and simplifies Glaubers task.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Wed, 26 Sep 2012, Christoph Lameter wrote:

> > Nack, this is already handled by CREATE_MASK in the mm/slab.c allocator;
> 
> CREATE_MASK defines legal flags that can be specified. Other flags cause
> and error. This is about flags that are internal that should be ignored
> when specified.
> 

That should be ignored for the mm/slab.c allocator, yes.

> I think it makes sense to reserve some top flags for internal purposes.
> 

It depends on the implementation: if another slab allocator were to use 
additional bits that would be a no-op with mm/slab.c, then this patch 
would be too restrictive.  There's also no requirement that any "internal 
flags" reserved by a slab allocator implementation must be shared in the 
same kmem_cache field as the flags passed to kmem_cache_create() -- it's 
actually better if they aren't since they seldom need to be accessed in 
the same cacheline.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Wed, 26 Sep 2012, David Rientjes wrote:

> On Wed, 26 Sep 2012, Christoph Lameter wrote:
>
> > > Nack, this is already handled by CREATE_MASK in the mm/slab.c allocator;
> >
> > CREATE_MASK defines legal flags that can be specified. Other flags cause
> > and error. This is about flags that are internal that should be ignored
> > when specified.
> >
>
> That should be ignored for the mm/slab.c allocator, yes.

Then you are ok with the patch as is?


> > I think it makes sense to reserve some top flags for internal purposes.
> >
>
> It depends on the implementation: if another slab allocator were to use
> additional bits that would be a no-op with mm/slab.c, then this patch
> would be too restrictive.  There's also no requirement that any "internal
> flags" reserved by a slab allocator implementation must be shared in the
> same kmem_cache field as the flags passed to kmem_cache_create() -- it's
> actually better if they aren't since they seldom need to be accessed in
> the same cacheline.

There *are* multiple slab allocators using those bits! And this works for
them. There is nothing too restrictive here. The internal flags are
standardized by this patch to be in the highest nibble.



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Thu, 27 Sep 2012, Christoph Lameter wrote:

> > > > Nack, this is already handled by CREATE_MASK in the mm/slab.c allocator;
> > >
> > > CREATE_MASK defines legal flags that can be specified. Other flags cause
> > > and error. This is about flags that are internal that should be ignored
> > > when specified.
> > >
> >
> > That should be ignored for the mm/slab.c allocator, yes.
> 
> Then you are ok with the patch as is?
> 

No, it's implementation defined so it shouldn't be in kmem_cache_create(), 
it should be in __kmem_cache_create().

> There *are* multiple slab allocators using those bits! And this works for
> them. There is nothing too restrictive here. The internal flags are
> standardized by this patch to be in the highest nibble.
> 

I'm referring to additional slab allocators that will be proposed for 
inclusion shortly.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[Christoph Lameter]

On Thu, 27 Sep 2012, David Rientjes wrote:

> No, it's implementation defined so it shouldn't be in kmem_cache_create(),
> it should be in __kmem_cache_create()

The flags are standardized between allocators. We carve out a couple of
bits here that can be slab specific.

> > There *are* multiple slab allocators using those bits! And this works for
> > them. There is nothing too restrictive here. The internal flags are
> > standardized by this patch to be in the highest nibble.
> >
>
> I'm referring to additional slab allocators that will be proposed for
> inclusion shortly.

I am sorry but we cannot consider something that has not been discussed
and publicly reviewed on the mailing list before. We have no way to
understand your rationales at this point and it would take quite some time
to review a new allocator. I would at least have expected the design of
the allocator to be discussed on linux-mm. Nothing of that nature has
happened as far as I can tell.

I think there was a presentation at one of the conference but sadly I was
not able to attend it. I tried to find some details on what was proposed
but so far I have been unsuccessful.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: Ignore internal flags in cache creation

[David Rientjes]

On Fri, 28 Sep 2012, Christoph Lameter wrote:

> > No, it's implementation defined so it shouldn't be in kmem_cache_create(),
> > it should be in __kmem_cache_create()
> 
> The flags are standardized between allocators. We carve out a couple of
> bits here that can be slab specific.
> 

That's true today, it won't be true in a week.

> > I'm referring to additional slab allocators that will be proposed for
> > inclusion shortly.
> 
> I am sorry but we cannot consider something that has not been discussed
> and publicly reviewed on the mailing list before. We have no way to
> understand your rationales at this point and it would take quite some time
> to review a new allocator. I would at least have expected the design of
> the allocator to be discussed on linux-mm. Nothing of that nature has
> happened as far as I can tell.
> 

Nobody here is disagreeing that the patch here is fine for slab, slub, 
and slob as they are currently implemented.  I'm simply trying to avoid 
ripping it out later and asking Glauber to consider something else that 
achieves what he needs.  There is, until this patch, no requirement 
anywhere that the flags passed to kmem_cache_create() may not be extended 
for allocator-specific behavior and I'd prefer to avoid adding such a 
specification unless absolutely necessary; in this case, there is an 
alternative that I've already outlined and it seems like Glauber is 
comfortable with using.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>