On 1/27/21 8:14 PM, Thiago Jung Bauermann wrote:
Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx> writes:
On 1/27/21 7:52 PM, Thiago Jung Bauermann wrote:
Will Deacon <will@xxxxxxxxxx> writes:
On Wed, Jan 27, 2021 at 09:59:38AM -0800, Lakshmi Ramasubramanian wrote:
On 1/27/21 8:52 AM, Will Deacon wrote:
Hi Will,
On Fri, Jan 15, 2021 at 09:30:16AM -0800, Lakshmi Ramasubramanian wrote:
create_dtb() function allocates kernel virtual memory for
the device tree blob (DTB). This is not consistent with other
architectures, such as powerpc, which calls kmalloc() for allocating
memory for the DTB.
Call kmalloc() to allocate memory for the DTB, and kfree() to free
the allocated memory.
Co-developed-by: Prakhar Srivastava <prsriva@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Prakhar Srivastava <prsriva@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx>
---
arch/arm64/kernel/machine_kexec_file.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 7de9c47dee7c..51c40143d6fa 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -29,7 +29,7 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
int arch_kimage_file_post_load_cleanup(struct kimage *image)
{
- vfree(image->arch.dtb);
+ kfree(image->arch.dtb);
image->arch.dtb = NULL;
vfree(image->arch.elf_headers);
@@ -59,19 +59,21 @@ static int create_dtb(struct kimage *image,
+ cmdline_len + DTB_EXTRA_SPACE;
for (;;) {
- buf = vmalloc(buf_size);
+ buf = kmalloc(buf_size, GFP_KERNEL);
Is there a functional need for this patch? I build the 'dtbs' target just
now and sdm845-db845c.dtb is approaching 100K, which feels quite large
for kmalloc().
Changing the allocation from vmalloc() to kmalloc() would help us further
consolidate the DTB setup code for powerpc and arm64.
Ok, but at the risk of allocation failure. Can powerpc use vmalloc()
instead?
I believe this patch stems from this suggestion by Rob Herring:
This could be taken a step further and do the allocation of the new
FDT. The difference is arm64 uses vmalloc and powerpc uses kmalloc. The
arm64 version also retries with a bigger allocation. That seems
unnecessary.
in
https://lore.kernel.org/linux-integrity/20201211221006.1052453-3-robh@xxxxxxxxxx/
The problem is that this patch implements only part of the suggestion,
which isn't useful in itself. So the patch series should either drop
this patch or consolidate the FDT allocation between the arches.
I just tested on powernv and pseries platforms and powerpc can use
vmalloc for the FDT buffer.
Thanks for verifying on powerpc platform Thiago.
I'll update the patch to do the following:
=> Use vmalloc for FDT buffer allocation on powerpc
=> Keep vmalloc for arm64, but remove the retry on allocation.
=> Also, there was a memory leak of FDT buffer in the error code path on arm64,
which I'll fix as well.
Did I miss anything?
Yes, you missed the second part of Rob's suggestion I was mentioning,
which is factoring out the code which allocates the new FDT from both
arm64 and powerpc.
Sure - I'll address that.
thanks,
-lakshmi
