[PATCHES BUG 13865] memcache tracking of talloc object sizes
Jeremy Allison
jra at samba.org
Fri Mar 29 20:00:33 UTC 2019
On Fri, Mar 29, 2019 at 12:09:53PM -0700, Christof Schmitt wrote:
> 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.
Actually I'm wrong - we can't mix talloc/non-talloc
stores here (it crashes, I tested :-). But we still
need to take the talloc'ed size into account.
> >
> > 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.
I'm nearly done with the fix and test. If I can
get it working I'll post for your review.
More information about the samba-technical
mailing list