From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6C9FE81F28 for ; Tue, 28 Feb 2017 01:32:44 -0800 (PST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Feb 2017 01:32:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,218,1484035200"; d="scan'208";a="70425033" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga005.fm.intel.com with ESMTP; 28 Feb 2017 01:32:44 -0800 Received: from fmsmsx101.amr.corp.intel.com (10.18.124.199) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 28 Feb 2017 01:32:44 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx101.amr.corp.intel.com (10.18.124.199) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 28 Feb 2017 01:32:43 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Tue, 28 Feb 2017 17:32:40 +0800 From: "Gao, Liming" To: Ard Biesheuvel , "edk2-devel@lists.01.org" , "afish@apple.com" , "leif.lindholm@linaro.org" , "Kinney, Michael D" , "Yao, Jiewen" CC: "lersek@redhat.com" , "Tian, Feng" , "Zeng, Star" Thread-Topic: [PATCH v4 4/7] MdeModulePkg/DxeCore: use separate lock for pool allocations Thread-Index: AQHSkQcuW8se8xeoJUG8N59uKGe05KF+KRFA Date: Tue, 28 Feb 2017 09:32:39 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14D6E5AF2@shsmsx102.ccr.corp.intel.com> References: <1488206291-25768-1-git-send-email-ard.biesheuvel@linaro.org> <1488206291-25768-5-git-send-email-ard.biesheuvel@linaro.org> In-Reply-To: <1488206291-25768-5-git-send-email-ard.biesheuvel@linaro.org> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v4 4/7] MdeModulePkg/DxeCore: use separate lock for pool allocations X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Feb 2017 09:32:44 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Liming Gao -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]=20 Sent: Monday, February 27, 2017 10:38 PM To: edk2-devel@lists.01.org; afish@apple.com; leif.lindholm@linaro.org; Kin= ney, Michael D ; Gao, Liming ; Yao, Jiewen Cc: lersek@redhat.com; Tian, Feng ; Zeng, Star ; Ard Biesheuvel Subject: [PATCH v4 4/7] MdeModulePkg/DxeCore: use separate lock for pool al= locations In preparation of adding memory permission attribute management to the pool allocator, split off the locking of the pool metadata into a separate lock. This is an improvement in itself, given that pool allocation can only interfere with the page allocation bookkeeping if pool pages are allocated or released. But it is also required to ensure that the permission attribute management does not deadlock, given that it may trigger page table splits leading to additional page tables being allocated. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- MdeModulePkg/Core/Dxe/Mem/Pool.c | 58 ++++++++++++++++---- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c b/MdeModulePkg/Core/Dxe/Mem/P= ool.c index 7afd2d312c1d..ebb2fceedd80 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c @@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER= EXPRESS OR IMPLIED. #include "DxeMain.h" #include "Imem.h" =20 +STATIC EFI_LOCK mPoolMemoryLock =3D EFI_INITIALIZE_LOCK_VARIABLE (TPL_NOTI= FY); + #define POOL_FREE_SIGNATURE SIGNATURE_32('p','f','r','0') typedef struct { UINT32 Signature; @@ -239,13 +241,13 @@ CoreInternalAllocatePool ( // // Acquire the memory lock and make the allocation // - Status =3D CoreAcquireLockOrFail (&gMemoryLock); + Status =3D CoreAcquireLockOrFail (&mPoolMemoryLock); if (EFI_ERROR (Status)) { return EFI_OUT_OF_RESOURCES; } =20 *Buffer =3D CoreAllocatePoolI (PoolType, Size); - CoreReleaseMemoryLock (); + CoreReleaseLock (&mPoolMemoryLock); return (*Buffer !=3D NULL) ? EFI_SUCCESS : EFI_OUT_OF_RESOURCES; } =20 @@ -289,6 +291,28 @@ CoreAllocatePool ( return Status; } =20 +STATIC +VOID * +CoreAllocatePoolPagesI ( + IN EFI_MEMORY_TYPE PoolType, + IN UINTN NoPages, + IN UINTN Granularity + ) +{ + VOID *Buffer; + EFI_STATUS Status; + + Status =3D CoreAcquireLockOrFail (&gMemoryLock); + if (EFI_ERROR (Status)) { + return NULL; + } + + Buffer =3D CoreAllocatePoolPages (PoolType, NoPages, Granularity); + CoreReleaseMemoryLock (); + + return Buffer; +} + /** Internal function to allocate pool of a particular type. Caller must have the memory lock held @@ -317,7 +341,7 @@ CoreAllocatePoolI ( UINTN NoPages; UINTN Granularity; =20 - ASSERT_LOCKED (&gMemoryLock); + ASSERT_LOCKED (&mPoolMemoryLock); =20 if (PoolType =3D=3D EfiACPIReclaimMemory || PoolType =3D=3D EfiACPIMemoryNVS || @@ -355,7 +379,7 @@ CoreAllocatePoolI ( if (Index >=3D SIZE_TO_LIST (Granularity)) { NoPages =3D EFI_SIZE_TO_PAGES(Size) + EFI_SIZE_TO_PAGES (Granularity) = - 1; NoPages &=3D ~(UINTN)(EFI_SIZE_TO_PAGES (Granularity) - 1); - Head =3D CoreAllocatePoolPages (PoolType, NoPages, Granularity); + Head =3D CoreAllocatePoolPagesI (PoolType, NoPages, Granularity); goto Done; } =20 @@ -383,7 +407,7 @@ CoreAllocatePoolI ( // // Get another page // - NewPage =3D CoreAllocatePoolPages(PoolType, EFI_SIZE_TO_PAGES (Granula= rity), Granularity); + NewPage =3D CoreAllocatePoolPagesI (PoolType, EFI_SIZE_TO_PAGES (Granu= larity), Granularity); if (NewPage =3D=3D NULL) { goto Done; } @@ -486,9 +510,9 @@ CoreInternalFreePool ( return EFI_INVALID_PARAMETER; } =20 - CoreAcquireMemoryLock (); + CoreAcquireLock (&mPoolMemoryLock); Status =3D CoreFreePoolI (Buffer, PoolType); - CoreReleaseMemoryLock (); + CoreReleaseLock (&mPoolMemoryLock); return Status; } =20 @@ -525,6 +549,19 @@ CoreFreePool ( return Status; } =20 +STATIC +VOID +CoreFreePoolPagesI ( + IN EFI_MEMORY_TYPE PoolType, + IN EFI_PHYSICAL_ADDRESS Memory, + IN UINTN NoPages + ) +{ + CoreAcquireMemoryLock (); + CoreFreePoolPages (Memory, NoPages); + CoreReleaseMemoryLock (); +} + /** Internal function to free a pool entry. Caller must have the memory lock held @@ -573,7 +610,7 @@ CoreFreePoolI ( // ASSERT (Tail->Signature =3D=3D POOL_TAIL_SIGNATURE); ASSERT (Head->Size =3D=3D Tail->Size); - ASSERT_LOCKED (&gMemoryLock); + ASSERT_LOCKED (&mPoolMemoryLock); =20 if (Tail->Signature !=3D POOL_TAIL_SIGNATURE) { return EFI_INVALID_PARAMETER; @@ -624,7 +661,7 @@ CoreFreePoolI ( // NoPages =3D EFI_SIZE_TO_PAGES(Size) + EFI_SIZE_TO_PAGES (Granularity) = - 1; NoPages &=3D ~(UINTN)(EFI_SIZE_TO_PAGES (Granularity) - 1); - CoreFreePoolPages ((EFI_PHYSICAL_ADDRESS) (UINTN) Head, NoPages); + CoreFreePoolPagesI (Pool->MemoryType, (EFI_PHYSICAL_ADDRESS) (UINTN) H= ead, NoPages); =20 } else { =20 @@ -680,7 +717,8 @@ CoreFreePoolI ( // // Free the page // - CoreFreePoolPages ((EFI_PHYSICAL_ADDRESS) (UINTN)NewPage, EFI_SIZE= _TO_PAGES (Granularity)); + CoreFreePoolPagesI (Pool->MemoryType, (EFI_PHYSICAL_ADDRESS) (UINT= N)NewPage, + EFI_SIZE_TO_PAGES (Granularity)); } } } --=20 2.7.4