RE: [PATCH] Rework CACHE to use a FreeList

[Owen Smith <owen.smith@xxxxxxxxxx>]

-----Original Message-----
From: Paul Durrant <xadimgnik@xxxxxxxxx> 
Sent: 22 August 2022 09:22
To: Owen Smith <owen.smith@xxxxxxxxxx>; win-pv-devel@xxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH] Rework CACHE to use a FreeList

[CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
unless you have verified the sender and know the content is safe.

On 22/08/2022 08:24, Owen Smith wrote:
> 
> 
> -----Original Message-----
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On 
> Behalf Of Paul Durrant
> Sent: 19 August 2022 17:23
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] Rework CACHE to use a FreeList
> 
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
> unless you have verified the sender and know the content is safe.
> 
> On 19/08/2022 11:15, Owen Smith wrote:
>> The slab allocation method will allogate about a PAGE worth of 
>> objects, and every object will be initialized. If the objects 
>> initializer allocates any resources, this can result in resource 
>> starvation. A particular bad example of this is the grant table cache, where 
>> a page of gnttab objects is 253 objects.
>> This is highlighted by xenvif's queues, where the receiver requires
>> 257 grant references (1 for the ring, and 256 for the ring slots) 
>> which results in 2 slabs, or 506 gnttab objects, reserving 506 grant 
>> references.
>>
>> Use a FreeList to contain individual objects that are not in use. 
>> This trades an increase in smaller allocations for reducing the wastage of 
>> unused objects.
>>
>> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> Sorry I didn't comment on the RFC; I was on PTO and then snowed under with 
> mail etc.
> I think this is a sledgehammer to crack a nut. I agree that XENVIF is being a 
> grant ref hog... but the correct thing to do there is to re-work the grant 
> table cache, not the underlying slab allocator; which I think is fine. The 
> problem is (ab)using the slab allocator's Ctor to get the reference. So a 
> free list implementation is fine... just in the gnttab code, rather than the 
> cache code.
> 
>     Paul
> 
> 
> 
> This also hits any other use of the CACHE interface where the object's Ctor 
> allocates a resource and there are many objects in a slab. XenVbd's segment 
> cache allocates a page, which can lead to failures with large number of VBDs 
> (one of our automated test cases fails with a 0x4B NO_PAGES_AVAILABLE when 
> run with 256 VBDs - I cant remember exactly how many VBDs are required).
> 
> The alternative here would be to rework the GNTTAB interface to use a 
> free list rather than the CACHE interface, and rework any other uses 
> where the Ctor could allocate significant resources (possibly using a 
> lazy allocation of memory, but this could Fill without enough Spill to 
> still lead to resource exhaustion)
> 

Ok, so the problem is not with the use of slabs per se; it's with the eager 
calls to the Ctor. How about, rather than constructing the entire slab, we 
construct a batch and have a separate 'constructed' and 'allocated' masks in 
the control structure to track what we've done. 
When we run out of constructed objects, we construct another batch. When we 
have more than two batches free, we free up one of them. How does that sound?

   Paul


Yes, if Ctors are too eager in allocating finite resources, they tend to 
exhaust resources too quickly. I did look at lazy initialization in the Cache, 
but haven’t got too far yet, and it likely will need a new Cache interface 
version to pass the step-size

Owen