Re: [PATCH 1/4] shallow.c: make paint_alloc slightly more robust

From: Jeff King <peff@peff.net>
Date: Dec 3, 2016, 05:14 UTC
Message-ID: <20161203051454.vp772xtto5ddxe7g@sigill.intra.peff.net>
In-Reply-To: <1480710664-26290-1-git-send-email-rv@rasmusvillemoes.dk>

On Fri, Dec 02, 2016 at 09:31:01PM +0100, Rasmus Villemoes wrote:

Show 5 quoted lines

> I have no idea if this is a real issue, but it's not obvious to me that
> paint_alloc cannot be called with info->nr_bits greater than about
> 4M (\approx 8*COMMIT_SLAB_SIZE). In that case the new slab would be too
> small. So just round up the allocation to the maximum of
> COMMIT_SLAB_SIZE and size.

I had trouble understanding what the problem is from this description, but I think i figured it out from the code.

Let me try to restate it to make sure I understand.

The paint_alloc() may be asked to allocate a certain number of bits, which it does across a series of independently allocated slabs. Each slab holds a fixed size, but we only allocate a single slab. If the number we need to allocate is larger than fits in a single slab, then at the end we'll have under-allocated.

Your solution is to make the slab we allocate bigger. But that seems odd to me. Usually when we are using COMMIT_SLAB_SIZE, we are allocating a series of slabs that make up a virtual array, and we know that each slab has the same size. So if you need to find the k-th item, and each slab has length n, then you'd look at slab (k / n), and then at item (k % n) within that slab.

In other words, I think the solution isn't to make the one slab bigger, but to allocate slabs until we have enough of them to meet the request.

But I don't really know how this code is used, or why it is using COMMIT_SLAB_SIZE in the first place. That's generally supposed to be an internal detail of the commit-slab.h infrastructure. Why is it being used directly, instead of just using the functions that commit-slab defines?

-Peff