Re: [Xen-devel] [PATCH 10/31] libxl: Crash (more sensibly) on malloc failure

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-04-11 at 12:04 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 10/31] libxl: Crash (more 
> sensibly) on malloc failure"):
> > On Wed, 2012-04-11 at 11:24 +0100, Ian Jackson wrote:
> > > Things left to do:
> > >  - Provide versions of malloc, realloc and free which do the
> > >    checking but which do not enroll the pointer in the gc.
> ...
> > We got that in the next patch?
> 
> Yes.  I don't think this bullet point is actually helpful in that
> commit message as it's not so much a deficiency in the code at that
> point as a thing I was intending to fix when I wrote it, so I have
> removed it.
> 
> > > Well, zero isn't adequate :-).  So yes, it's arbitrary.  25 is 100
> > > bytes (i386) or 200 bytes (amd64) which seems a reasonable initial
> > > overhead and will probably avoid triggering a realloc too often.
> > 
> > Why isn't just doubling each time adequate?
> 
> alloc_maxsize is initialised to 0.  Doubling zero gives zero.

Oh, for some reason I expected it would be initialised to >=1.

> NB that libxl__ptr_add needs to be rewritten not to be quadratic in
> the number of pointrs added (!)

Isn't it O(N) in numbers of pointers?

I also just noticed that the initial:
   /* fast case: we have space in the array for storing the pointer */
    for (i = 0; i < gc->alloc_maxsize; i++) {
        if (!gc->alloc_ptrs[i]) {
            gc->alloc_ptrs[i] = ptr;
            return 0;
        }
    }
is a bit suboptimal since we never remove a ptr from the array, so we
may as well track the max actually used. Then the fast path might
actually be fast...

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel