[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [PATCH 2/3] Add Lazy slab initialization


  • To: "paul@xxxxxxx" <paul@xxxxxxx>, "win-pv-devel@xxxxxxxxxxxxxxxxxxxx" <win-pv-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Owen Smith <owen.smith@xxxxxxxxxx>
  • Date: Mon, 19 Jul 2021 11:09:00 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=3Aq+eLMzBqsCQjS9AAVCIVi4COLRUZcVX4XS52AyWFc=; b=bomNg0yBcxK2LiKMk437Qgp7Kdh3IuF62YUY4Tx0AG5YE82oxWuWDIgv4oTe67yFI4PzlXxF2IUGM91m7awFgcgZBGOMUKo2PimxdarbNFMDkVotPau2VhEmWt4qSfKs7z5yGMYyDvJk3wa+7yu0ugI8Ro81xOfz8ccMugPlRByiI8NWDMormXsXjyCUa91179D6HC0ByypVPD4drw23mAb7XaZI5YarYoGkvLSnXGByVczAhYnczAT79OlssmpbHO+alDbAoLt1iWUxOOQzj24lR0yENmaTh77eTwRckO0LnWqgAUAVpPN2gFaFgrUQmiqeOjK5ggPnWZc5mGOnSQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JtZNojqGfYQa6p8hgWCRyMvF8aEqE+1+ZUWs+vo1v4Fo2NkPeu0EI+oDLskDacpq2OuUJjDPKelT5NwPy9DqFpDqCYyAOsNK/Aq6wxzX62EIRKj7JMKCe4kOinkdbbxjjTMst8EFo81/sEGW1j6568xt4YoIFMdOdF25jni3F61XNNIhVDxPPD1FoLcQWky6EQ5QjqSMI97+FIaVxpMV86s3ni56amjCQcPyvLCvtsyGuKbimX9BvfffZDLNZClzsfO0Ei9lKeT3jX9FIQN2sq4QyWjr5Jc3MIxbNiifIYUKBjeDcTa55hKblZZkT41ugs1bZTTsO4JMP4HCMgmzYw==
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Delivery-date: Mon, 19 Jul 2021 11:09:08 +0000
  • Ironport-hdrordr: A9a23:hvMt6q4Q3gE3S+vYygPXwX+BI+orL9Y04lQ7vn2ZFiY6TiXIra +TdaoguSMc0AxhJU3I6urwRJVoJkmsu6KdgLNhcotKOTOGhILGFvAa0WKP+UyDJ8S6zJ8m6U 4CSdk+NDSTNykDsS+S2mDReLxMoKjlzEnrv5ak854Ed3AwV0gK1XYcNu/vKDwReOAwP+tfKH Pz3LsjmxOQPVAsKuirDHgMWObO4/fRkoj9XBIADxk7rCGTkDKB8tfBYlil9yZbdwkK7aYp8G DDnQC8zL6kqeuHxhjV0HKWx4hKmeHm1sBICKW3+4sow3TX+0SVjbZaKvm/VQMO0aaSAZER4Z /xSiIbToFOArXqDziISFXWqlHdOX0VmgDfIBej8AXeSIrCNWgH4oN69PNkWwqc5Ew6sN5m1q VXm2qfqppMFBvF2D/w/t7SSnhR5wGJSFcZ4KcuZkZkIMMjgX5q3Pki1VIQFI1FEDPx6YghHu UrBMbA5OxOeVffa3zCpGFgzNGlQ3x2R369MwU/k93Q1yITkGFyzkMeysBalnAc9IglQ50B4+ jfKKxnmLxHU8dTZ6NgA+UKR9exFwX2MFjxGXPXJU6iGLAMOnrLpZKy6LIp5PuycJhN15c2kI SpaiIviYfzQTOYNSSq5uwGzvngehTPYd3d8LAr23EigMyNeFPCC1zwdGwT
  • Ironport-sdr: 7b7jTGe2J+Y4TV9z61iryAT5R57BlN2+j9r6os7rgLltmz7Qm+LgjnvbhFdZd8iB8GKJiKwQ/m VoScyowk8NGBabT1+eMpAqUlRYbVnyftZI0iaGMo6RoD3f8A0goUBCvvjG1n8sIyN45pud52i4 72k7W+9IkimGNDLdlm1qLspJ57tOaSClwF4Lf65HDxepDfu3cZAOJAPSMDRAV50hjaLVrow4zy a57Ys5tKukeLPOQmTX8Pf09eHnb48IN6NlZbyVMn8XzX6nOgIeQub6f5zqz3J4iPkNJzoW2Wjs 79UVYtiWFZuH6+zM71Z55xQr
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>
  • Thread-index: AQHXfIWhE2XS4Xt9zUuvyhBx6dMF3KtKH7wAgAABq9A=
  • Thread-topic: [PATCH 2/3] Add Lazy slab initialization

-----Original Message-----
From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
Paul Durrant
Sent: Monday, July 19, 2021 11:54 AM
To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 2/3] Add Lazy slab initialization

[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/07/2021 11:03, Owen Smith wrote:
> Only initialize 1/4 of a slab initially, unless a reservation is defined.
> Allocate and initialize each slab object once this initial allocation 
> has been exceeded.
> 
> This is to decrease the overall resource usage when each cache 
> object's constructor allocates additional memory.
> 

Whilst I understand the motivation, I don't think this is the right solution. 
If the resource consumption is too great to justify the performance benefit of 
using the slab constructor to allocate additional memory then the owner of the 
cache should simply refrain from doing so and defer that work until the point 
it actually allocates the object from the cache. There should be no need for a 
global change such as this.

   Paul


Its possible that this is not need when the 3rd patch (Remove 
MINIMUM_OBJECT_SIZE) is used. 
In XenVbd, the bounce buffers had a reservation of 32 objects (each object will 
allocate a page of memory). This unfortunately currently results in 2 slabs of 
31 objects, which is far more than used in a relatively unused VM 
(experimentally, 16 bounce buffers are used at any one time).
This does leave the possibility of consuming significant memory when a new slab 
is required. At worse this can be (PAGE_SIZE - sizeof(XENBUS_CACHE_SLAB)) / 
sizeof(PVOID) objects allocating a single page, or less objects allocating 
multiple pages.

I will look at making users of the cache interface attempt to avoid 
preallocation of pages during the constructor call.

Owen

> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>   src/xenbus/cache.c | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/src/xenbus/cache.c b/src/xenbus/cache.c index 
> 247f244..d695ee3 100644
> --- a/src/xenbus/cache.c
> +++ b/src/xenbus/cache.c
> @@ -61,6 +61,7 @@ typedef struct _XENBUS_CACHE_SLAB {
>       LIST_ENTRY      ListEntry;
>       USHORT          MaximumOccupancy;
>       USHORT          CurrentOccupancy;
> +    USHORT          CurrentAllocated;
>       ULONG           *Mask;
>       UCHAR           Buffer[1];
>   } XENBUS_CACHE_SLAB, *PXENBUS_CACHE_SLAB; @@ -301,6 +302,7 @@ 
> CacheCreateSlab(
>       ULONG               Size;
>       LONG                Index;
>       LONG                SlabCount;
> +    LONG                ObjectsToAllocate;
>       NTSTATUS            status;
>   
>       NumberOfBytes = P2ROUNDUP(FIELD_OFFSET(XENBUS_CACHE_SLAB, 
> Buffer) + @@ -326,6 +328,7 @@ CacheCreateSlab(
>       Slab->Magic = XENBUS_CACHE_SLAB_MAGIC;
>       Slab->Cache = Cache;
>       Slab->MaximumOccupancy = (USHORT)Count;
> +    Slab->CurrentAllocated = 0;
>   
>       Size = P2ROUNDUP(Count, BITS_PER_ULONG);
>       Size /= 8;
> @@ -334,12 +337,18 @@ CacheCreateSlab(
>       if (Slab->Mask == NULL)
>           goto fail3;
>   
> -    for (Index = 0; Index < (LONG)Slab->MaximumOccupancy; Index++) {
> +    ObjectsToAllocate = (Cache->Reservation == 0) ?
> +                (LONG)Count / 4 :
> +                min((LONG)Cache->Reservation, (LONG)Count);
> +
> +    for (Index = 0; Index < ObjectsToAllocate; Index++) {
>           PVOID Object = (PVOID)&Slab->Buffer[Index * Cache->Size];
>   
>           status = __CacheCtor(Cache, Object);
>           if (!NT_SUCCESS(status))
>               goto fail4;
> +
> +        Slab->CurrentAllocated++;
>       }
>   
>       CacheInsertSlab(Cache, Slab);
> @@ -402,7 +411,7 @@ CacheDestroySlab(
>       ASSERT(Cache->Cursor != &Cache->SlabList ||
>              IsListEmpty(&Cache->SlabList));
>   
> -    Index = Slab->MaximumOccupancy;
> +    Index = Slab->CurrentAllocated;
>       while (--Index >= 0) {
>           PVOID Object = (PVOID)&Slab->Buffer[Index * Cache->Size];
>   
> @@ -493,6 +502,20 @@ CacheGetObjectFromSlab(
>       if (Slab->CurrentOccupancy == Slab->MaximumOccupancy)
>           return NULL;
>   
> +    ASSERT3U(Slab->CurrentOccupancy, <=, Slab->CurrentAllocated);
> +    ASSERT3U(Slab->CurrentAllocated, <=, Slab->MaximumOccupancy);
> +    if (Slab->CurrentOccupancy == Slab->CurrentAllocated) {
> +        NTSTATUS            status;
> +
> +        Object = (PVOID)&Slab->Buffer[Slab->CurrentAllocated * 
> + Cache->Size];
> +
> +        status = __CacheCtor(Cache, Object);
> +        if (!NT_SUCCESS(status))
> +            return NULL;
> +
> +        Slab->CurrentAllocated++;
> +    }
> +
>       Index = __CacheMaskScan(Slab->Mask, Slab->MaximumOccupancy);
>       BUG_ON(Index >= Slab->MaximumOccupancy);
>   
> 



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.