From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ard Biesheuvel Subject: Re: [PATCH 10/11] efi: reduce the amount of memblock reservations for persistent allocations Date: Fri, 30 Nov 2018 09:39:34 +0100 Message-ID: References: <20181129171230.18699-1-ard.biesheuvel@linaro.org> <20181129171230.18699-11-ard.biesheuvel@linaro.org> <20181130083823.GB97998@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20181130083823.GB97998@gmail.com> Sender: linux-kernel-owner@vger.kernel.org To: Ingo Molnar Cc: linux-efi , Thomas Gleixner , Linux Kernel Mailing List , Andy Lutomirski , Arend Van Spriel , Bhupesh Sharma , Borislav Petkov , Dave Hansen , Eric Snowberg , Hans de Goede , Joe Perches , Jon Hunter , Julien Thierry , Marc Zyngier , Nathan Chancellor , Peter Zijlstra , "Prakhya, Sai Praneeth" , Sedat Dilek , YiFei Zhu List-Id: linux-efi@vger.kernel.org On Fri, 30 Nov 2018 at 09:38, Ingo Molnar wrote: > > > * Ard Biesheuvel wrote: > > > The current implementation of efi_mem_reserve_persistent() is rather > > naive, in the sense that for each invocation, it creates a separate > > linked list entry to describe the reservation. Since the linked list > > entries themselves need to persist across subsequent kexec reboots, > > every reservation created this way results in two memblock_reserve() > > calls at the next boot. > > > > On arm64 systems with 100s of CPUs, this may result in a excessive > > number of memblock reservations, and needless fragmentation. > > > > So instead, make use of the newly updated struct linux_efi_memreserve > > layout to put multiple reservations into a single linked list entry. > > This should get rid of the numerous tiny memblock reservations, and > > effectively cut the total number of reservations in half on arm64 > > systems with many CPUs. > > > > Tested-by: Marc Zyngier > > Signed-off-by: Ard Biesheuvel > > --- > > drivers/firmware/efi/efi.c | 20 +++++++++++++++++--- > > include/linux/efi.h | 3 +++ > > 2 files changed, 20 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > > index 80b11521627a..e90bc32c2670 100644 > > --- a/drivers/firmware/efi/efi.c > > +++ b/drivers/firmware/efi/efi.c > > @@ -998,7 +998,8 @@ int __ref efi_mem_reserve_persistent(phys_addr_t ad= dr, u64 size) > > { > > struct linux_efi_memreserve *rsv; > > int rsvsize =3D EFI_MEMRESERVE_SIZE(1); > > - int rc; > > + unsigned long prsv; > > + int rc, index; > > > > if (efi_memreserve_root =3D=3D (void *)ULONG_MAX) > > return -ENODEV; > > @@ -1009,11 +1010,24 @@ int __ref efi_mem_reserve_persistent(phys_addr_= t addr, u64 size) > > return rc; > > } > > > > - rsv =3D kmalloc(rsvsize, GFP_ATOMIC); > > I fixed the following build warning in this patch: > > drivers/firmware/efi/efi.c:1000:6: warning: unused variable =E2=80=98rs= vsize=E2=80=99 [-Wunused-variable] > > 'rsvsize' got entirely orphaned by the patch, so it can be removed. > Thanks, that was a rebase error on my part - apologies for not spotting it.