PATCH : talloc-2.0.1 to add support for static talloc pools

Matthias Dieter Wallnöfer mdw at samba.org
Wed Jan 19 00:53:58 MST 2011


Kathick,

the final review has to be done by one of the talloc maintainers 
(MAINTAINERS.txt):
          Andrew Tridgell <tridge at samba.org>
          Rusty Russell <rusty at samba.org>

Regards,
Matthias Wallnöfer

Karthick A R wrote:
> Hi Matthias,
>
> On Tue, Jan 18, 2011 at 11:21 PM, Matthias Dieter Wallnöfer 
> <mdw at samba.org <mailto:mdw at samba.org>> wrote:
>
>     Hi Kathick,
>
>     small question to your patch:
>
>         +static inline int talloc_pool_map_get(struct talloc_chunk *tc)
>         +{
>         +    unsigned int *map = TALLOC_POOL_MAP(tc);
>         +    unsigned int count = *talloc_cache_objectcount(tc);
>         +    unsigned int map_words = TALLOC_POOL_MAP_WORDS(count);
>         +    static int ffz_map[16] = { 0, 1, 0, 2, 0, 1, 0, 3, 0, 1,
>         0, 2, 0, 1, 0, -1 };
>         +    int i;
>         +    for(i = 0; i<  map_words; ++i) {
>         +        unsigned int offset = 0;
>         +        unsigned int mask = map[i]&  0xffffffffU;
>         +        if(mask == 0xffffffffU) continue;
>
>     Why is "i" signed here? Is it since "talloc_pool_map_get" returns
>     "-1" on error conditions?
>
>
> Yes.
>
>     I would transform "i" to unsigned and return it like this: return
>     (int) i;
>
> Sure. Will do. Let me know if these changes are a candidate for talloc 
> and I would update and send probably a final patch with the review 
> feedback.
>
> Regards,
> -Karthick
>
>
>
>     Karthick A R wrote:
>
>         Hi,
>          Please find attached a patch that would apply clean on top of
>         talloc 2.0.1
>         that adds support for static talloc pools.
>          I introduced a simple talloc static pool which creates a
>         static pool of
>         fixed object sizes for a reclaimable object cache.
>          The current talloc_pool is a variable sized pool where frees
>         just decrement
>         the object count which is good if one wants to preallocate a
>         large pool for
>         faster allocations.
>          But I thought of adding another api that allows us to create
>         a fixed size
>         cache. When all allocations are granted, it would fail instead
>         of falling
>         back to malloc.(like the current pool)
>
>          This enables one to create a talloc pool cache and then just
>         use the
>         context pointer to allocate fixed size objects from the
>          pre-created static cache. Because its a fixed-size pool
>         cache, it supports
>         freeing objects to the pool as well.
>          Allocations and free from the static pool add a bitmap
>         overhead to keep
>         track of objects allocations and frees.
>          But the existing talloc_pool header overhead remains
>         unchanged. The
>         following apis were added:
>
>         +void *talloc_pool_cache(const void *context, size_t size,
>         size_t objsize);
>         +void *talloc_cache(const void *context);
>         +void *talloc_cache_zero(const void *context);
>
>         talloc_cache just passed the context and expects to grab the
>         static object
>         size for the pool. I haven't added any overhead to the
>         talloc_chunk struct
>         as well to avoid impacting the current use-case on top of the
>         talloc_pools.
>         A new TALLOC_FLAG_POOLCACHE has been added to track the
>         allocations to the
>         static pool. This adds an extra bit the flag bit and hence
>         also adjusts the
>         talloc magic base which currently has a 4 bit flag space
>         accordingly.
>
>         Apart from the api additions, I have also fixed a crash when
>         trying to use
>         nested talloc pools as currently that tries to free(tc) in
>         case the tc or
>         the pool itself was allocated from another parent pool.
>
>         Eg:
>            void *poolfoo = talloc_pool(parent, sizeof(struct foo) * 1024);
>            void *poolbar = talloc_pool(poolfoo, sizeof(struct bar) *
>         100); /*
>         subpool*/
>
>         Trying to talloc_free(poolbar) would cause a crash currently.
>         The patch also
>         addresses this by trying to trace the parent of the subpool to
>         free the
>         subpool to the parent pool context.
>
>         Alternatively I have also forked the talloc 2.0.1 code base
>         with the above
>         changes at my github repo:
>         https://github.com/karthick18/talloc
>
>         You can also trace the diff over 2.0.1 here :
>         https://github.com/karthick18/talloc/commit/d19bf7d298c276e38b988d00234ca8a8606db6e3
>
>         <https://github.com/karthick18/talloc/commit/d19bf7d298c276e38b988d00234ca8a8606db6e3>A
>         simple usage eg: for talloc in general which also includes
>         usage for the
>         newly added apis can be traced here:
>         https://github.com/karthick18/talloc/blob/master/talloc_test.c
>
>         If you guys are ok with the patch, then maybe I can patch it
>         on top of the
>         latest 2.0.5 talloc. This won't be required to get into Samba
>         talloc anyway
>         but might be a good candidate to consider for the standalone
>         LGPL super-COOL
>         talloc!
>
>         Regards,
>         -Karthick
>
>         <https://github.com/karthick18/talloc/blob/master/talloc_test.c>
>
>
>
>
>
> -- 
> -- 
> Software is like sex: it's better when it's free.
> -Linus Torvalds
>
> A.R.Karthick
> http://github.com/karthick18



More information about the samba-technical mailing list