Re: slab quirks in DEBUG, ctor, and initialization
Hi John, On Mon, 17 Dec 2007, John Reiser wrote: > idr_pre_get calls kmem_cache_alloc, which constructs 'struct idr_layer' > via the cachep->ctor() call from cache_alloc_debugcheck_after to > idr_cache_ctor, and not via cache_init_objs. So if DEBUG is off, > then idr_cache_ctor does not get its chance to call memset, > which could leave the struct idr_layer potentially undefined. No, init_cache_objs() will call the constructor, if the cache has one when DEBUG is not set so the struct idr_layer can never be undefined. However, struct idr_layer can contain non-zero elements if someone does a kmem_cache_free() on an object that hasn't been zeroed. If that matters here, idr_pre_get should call kmem_cache_zalloc() and drop the constructor. Pekka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: slab quirks in DEBUG, ctor, and initialization
Hi Pekka, >>In mm/slab.c, the DEBUG variant of cache_alloc_debugcheck_after >>might call cachep->ctor(objp, cachep, 0); but the non-DEBUG >>variant does absolutely nothing. idr_pre_get is a routine >>which notices the difference. > > > How does ipr_pre_get notice this? idr_pre_get calls kmem_cache_alloc, which constructs 'struct idr_layer' via the cachep->ctor() call from cache_alloc_debugcheck_after to idr_cache_ctor, and not via cache_init_objs. So if DEBUG is off, then idr_cache_ctor does not get its chance to call memset, which could leave the struct idr_layer potentially undefined. >>Even when cache_alloc_debugcheck_after does invoke the ctor, >>then it is conditional upon cachep->flags & SLAB_POISON. This >>assumes that the only two states are poisoned and all-zero >>(from .bss static, or via a cleared new page frame.) >>So if SLAB_POISON is not specified, then a ctor which >>does anything other than memset(,0,) is out of luck. >>Instead: if a ctor is specified then it should be called >>for each successful allocation. > > > Sorry, I don't understand at all what's the problem is here. For the > common (non-poison) case, we initialize all objects *once* whenever a > cache is grown (see cache_grow calling cache_init_objs) which is the > whole point of having constructors. When poisoning is enabled, we > obviously cannot do this which is why we call the constructor for > every allocation. There is a comment near the beginning of mm/slab.c: * This means, that your constructor is used only for newly allocated * slabs and you must pass objects with the same initializations to * kmem_cache_free. Note that the comment should be updated to account for the constructor also being called if SLAB_POISON is specified. A significant implication is that calling the constructor always establishes good state, even though the call might not be necessary, and although it might take more time than not calling it. In order to avoid the allocator having to call the constructor, then the caller of kmem_cache_free must call the constructor just before freeing, or establish the same values. In contrast to calling the constructor every time, the existing convention increases complexity and risk of bugs, in exchange for faster performance when actions equivalent to the ctor are performed in the dtor instead, or when state that is equivalent to the ctor occurs as a natural byproduct of being done with the object. The real gripe is that the current code makes it cumbersome for a dynamic checking program such as valgrind(memcheck) to record correctly the state of the object. Is the object initialized (and which parts of it, and were the initializations performed into allocated space), or is it merely allocated and uninitialized? -- John Reiser, [EMAIL PROTECTED] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: slab quirks in DEBUG, ctor, and initialization
Hi John, On Dec 17, 2007 5:47 PM, John Reiser <[EMAIL PROTECTED]> wrote: > In mm/slab.c, the DEBUG variant of cache_alloc_debugcheck_after > might call cachep->ctor(objp, cachep, 0); but the non-DEBUG > variant does absolutely nothing. idr_pre_get is a routine > which notices the difference. How does ipr_pre_get notice this? On Dec 17, 2007 5:47 PM, John Reiser <[EMAIL PROTECTED]> wrote: > Even when cache_alloc_debugcheck_after does invoke the ctor, > then it is conditional upon cachep->flags & SLAB_POISON. This > assumes that the only two states are poisoned and all-zero > (from .bss static, or via a cleared new page frame.) > So if SLAB_POISON is not specified, then a ctor which > does anything other than memset(,0,) is out of luck. > Instead: if a ctor is specified then it should be called > for each successful allocation. Sorry, I don't understand at all what's the problem is here. For the common (non-poison) case, we initialize all objects *once* whenever a cache is grown (see cache_grow calling cache_init_objs) which is the whole point of having constructors. When poisoning is enabled, we obviously cannot do this which is why we call the constructor for every allocation. Pekka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
slab quirks in DEBUG, ctor, and initialization
In mm/slab.c, the DEBUG variant of cache_alloc_debugcheck_after might call cachep->ctor(objp, cachep, 0); but the non-DEBUG variant does absolutely nothing. idr_pre_get is a routine which notices the difference. Even when cache_alloc_debugcheck_after does invoke the ctor, then it is conditional upon cachep->flags & SLAB_POISON . This assumes that the only two states are poisoned and all-zero (from .bss static, or via a cleared new page frame.) So if SLAB_POISON is not specified, then a ctor which does anything other than memset(,0,) is out of luck. Instead: if a ctor is specified then it should be called for each successful allocation. Invoking the ctor from within cache_alloc_debugcheck_after makes it awkward for a dynamic checker of allocations and initializations. Valgrind would be happier if allocation and initialization were invoked at the same subroutine nesting level, such as in __cache_alloc: cache_alloc_debugcheck_before(cachep, flags); local_irq_save(save_flags); objp = __do_cache_alloc(cachep, flags); /* checker marks the space as allocated */ local_irq_restore(save_flags); objp = cache_alloc_debugcheck_after(cachep, flags, objp, caller); if (objp && cachep->ctor) cachep->ctor(objp, cachep, 0); /* checker notes initializations during ctor [above] */ -- John Reiser, [EMAIL PROTECTED] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
slab quirks in DEBUG, ctor, and initialization
In mm/slab.c, the DEBUG variant of cache_alloc_debugcheck_after might call cachep-ctor(objp, cachep, 0); but the non-DEBUG variant does absolutely nothing. idr_pre_get is a routine which notices the difference. Even when cache_alloc_debugcheck_after does invoke the ctor, then it is conditional upon cachep-flags SLAB_POISON . This assumes that the only two states are poisoned and all-zero (from .bss static, or via a cleared new page frame.) So if SLAB_POISON is not specified, then a ctor which does anything other than memset(,0,) is out of luck. Instead: if a ctor is specified then it should be called for each successful allocation. Invoking the ctor from within cache_alloc_debugcheck_after makes it awkward for a dynamic checker of allocations and initializations. Valgrind would be happier if allocation and initialization were invoked at the same subroutine nesting level, such as in __cache_alloc: cache_alloc_debugcheck_before(cachep, flags); local_irq_save(save_flags); objp = __do_cache_alloc(cachep, flags); /* checker marks the space as allocated */ local_irq_restore(save_flags); objp = cache_alloc_debugcheck_after(cachep, flags, objp, caller); if (objp cachep-ctor) cachep-ctor(objp, cachep, 0); /* checker notes initializations during ctor [above] */ -- John Reiser, [EMAIL PROTECTED] -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: slab quirks in DEBUG, ctor, and initialization
Hi John, On Dec 17, 2007 5:47 PM, John Reiser [EMAIL PROTECTED] wrote: In mm/slab.c, the DEBUG variant of cache_alloc_debugcheck_after might call cachep-ctor(objp, cachep, 0); but the non-DEBUG variant does absolutely nothing. idr_pre_get is a routine which notices the difference. How does ipr_pre_get notice this? On Dec 17, 2007 5:47 PM, John Reiser [EMAIL PROTECTED] wrote: Even when cache_alloc_debugcheck_after does invoke the ctor, then it is conditional upon cachep-flags SLAB_POISON. This assumes that the only two states are poisoned and all-zero (from .bss static, or via a cleared new page frame.) So if SLAB_POISON is not specified, then a ctor which does anything other than memset(,0,) is out of luck. Instead: if a ctor is specified then it should be called for each successful allocation. Sorry, I don't understand at all what's the problem is here. For the common (non-poison) case, we initialize all objects *once* whenever a cache is grown (see cache_grow calling cache_init_objs) which is the whole point of having constructors. When poisoning is enabled, we obviously cannot do this which is why we call the constructor for every allocation. Pekka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: slab quirks in DEBUG, ctor, and initialization
Hi Pekka, In mm/slab.c, the DEBUG variant of cache_alloc_debugcheck_after might call cachep-ctor(objp, cachep, 0); but the non-DEBUG variant does absolutely nothing. idr_pre_get is a routine which notices the difference. How does ipr_pre_get notice this? idr_pre_get calls kmem_cache_alloc, which constructs 'struct idr_layer' via the cachep-ctor() call from cache_alloc_debugcheck_after to idr_cache_ctor, and not via cache_init_objs. So if DEBUG is off, then idr_cache_ctor does not get its chance to call memset, which could leave the struct idr_layer potentially undefined. Even when cache_alloc_debugcheck_after does invoke the ctor, then it is conditional upon cachep-flags SLAB_POISON. This assumes that the only two states are poisoned and all-zero (from .bss static, or via a cleared new page frame.) So if SLAB_POISON is not specified, then a ctor which does anything other than memset(,0,) is out of luck. Instead: if a ctor is specified then it should be called for each successful allocation. Sorry, I don't understand at all what's the problem is here. For the common (non-poison) case, we initialize all objects *once* whenever a cache is grown (see cache_grow calling cache_init_objs) which is the whole point of having constructors. When poisoning is enabled, we obviously cannot do this which is why we call the constructor for every allocation. There is a comment near the beginning of mm/slab.c: * This means, that your constructor is used only for newly allocated * slabs and you must pass objects with the same initializations to * kmem_cache_free. Note that the comment should be updated to account for the constructor also being called if SLAB_POISON is specified. A significant implication is that calling the constructor always establishes good state, even though the call might not be necessary, and although it might take more time than not calling it. In order to avoid the allocator having to call the constructor, then the caller of kmem_cache_free must call the constructor just before freeing, or establish the same values. In contrast to calling the constructor every time, the existing convention increases complexity and risk of bugs, in exchange for faster performance when actions equivalent to the ctor are performed in the dtor instead, or when state that is equivalent to the ctor occurs as a natural byproduct of being done with the object. The real gripe is that the current code makes it cumbersome for a dynamic checking program such as valgrind(memcheck) to record correctly the state of the object. Is the object initialized (and which parts of it, and were the initializations performed into allocated space), or is it merely allocated and uninitialized? -- John Reiser, [EMAIL PROTECTED] -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: slab quirks in DEBUG, ctor, and initialization
Hi John, On Mon, 17 Dec 2007, John Reiser wrote: idr_pre_get calls kmem_cache_alloc, which constructs 'struct idr_layer' via the cachep-ctor() call from cache_alloc_debugcheck_after to idr_cache_ctor, and not via cache_init_objs. So if DEBUG is off, then idr_cache_ctor does not get its chance to call memset, which could leave the struct idr_layer potentially undefined. No, init_cache_objs() will call the constructor, if the cache has one when DEBUG is not set so the struct idr_layer can never be undefined. However, struct idr_layer can contain non-zero elements if someone does a kmem_cache_free() on an object that hasn't been zeroed. If that matters here, idr_pre_get should call kmem_cache_zalloc() and drop the constructor. Pekka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/