[PATCHES BUG 13865] memcache tracking of talloc object sizes

[Christof Schmitt]

On Fri, Mar 29, 2019 at 11:35:30AM -0700, Jeremy Allison via samba-technical wrote:
> On Thu, Mar 28, 2019 at 11:28:47AM -0700, Christof Schmitt via samba-technical wrote:
> > On Thu, Mar 28, 2019 at 11:28:14AM -0700, Christof Schmitt wrote:
> > > This is a fix for memcache to also add the talloc object size to the
> > > amount of used space. The objects become part of the memcache, so the
> > > sizes should be tracked as well.
> > > 
> > > The third patch is a clarification to the description of the 'max stat
> > > cache size' parameter.
> > > 
> > > Pipeline is running at:
> > > https://gitlab.com/samba-team/devel/samba/pipelines/54120627
> > > 
> > > Christof
> > 
> > And here are the actual patches.
> 
> Not quite complete I think. In the original memcache_add()
> code we have a corner case where a talloc'ed key value is
> being replaced with a non-talloc'ed value (yeah I know,
> goes into the "don't do that catagory" but we currently
> cope with it :-).
> 
> The code goes:
> 
> 271         e = memcache_find(cache, n, key);
> 272 
> 273         if (e != NULL) {
> 274                 memcache_element_parse(e, &cache_key, &cache_value);
> 275 
> 276                 if (value.length <= cache_value.length) {
> 277                         if (memcache_is_talloc(e->n)) {
> 278                                 void *ptr;
> 279                                 SMB_ASSERT(cache_value.length == sizeof(ptr));
> 280                                 memcpy(&ptr, cache_value.data, sizeof(ptr));
> 
> I think you need to add:
> 
> +				    cache->size -= talloc_total_size(ptr);
> 
> here to keep the size consistent in this edge case.

Thanks for pointing this out.

> 
> 281                                 TALLOC_FREE(ptr);
> 282                         }
> 283                         /*
> 284                          * We can reuse the existing record
> 285                          */
> 286                         memcpy(cache_value.data, value.data, value.length);
> 287                         e->valuelength = value.length;
> 288                         return;
> 289                 }
> 
> Probably need to add a test for this too...

Yes, will do. I will be busy with something else this afternoon, but i
should have an update for this early next week.

Christof