Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
Hello, [ Found this in my @cs.helsinki.fi inbox, grmbl. ] On Fri, Sep 28, 2012 at 11:39 PM, 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. 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
Hello, [ Found this in my @cs.helsinki.fi inbox, grmbl. ] On Fri, Sep 28, 2012 at 11:39 PM, David Rientjes rient...@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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 rient...@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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 >> > > 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 c...@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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 > 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] slab: Ignore internal flags in cache creation
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 CC: Christoph Lameter CC: Pekka Enberg CC: David Rientjes --- 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 0x0002UL/* 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 0xF000UL + /* * 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] slab: Ignore internal flags in cache creation
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 glom...@parallels.com CC: Christoph Lameter c...@linux.com CC: Pekka Enberg penb...@cs.helsinki.fi CC: David Rientjes rient...@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 0x0002UL/* 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 0xF000UL + /* * 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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 c...@linux.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: Ignore internal flags in cache creation
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 c...@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 from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
