Messages in this thread Patch in this message | | | Date | Fri, 26 Feb 2016 09:37:41 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v7] x86/setup: get ramdisk parameters only once |
| |
On Fri, Feb 26, 2016 at 01:31:45PM +0600, Alexander Kuleshov wrote: > arch/x86/kernel/setup.c | 109 ++++++++++++++++++++++++++---------------------- > 1 file changed, 60 insertions(+), 49 deletions(-) > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index d3d80e6..449b4da 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -169,6 +169,15 @@ static struct resource bss_resource = { > .flags = IORESOURCE_BUSY | IORESOURCE_MEM > }; > > +/* > + * ramdisk setup > + */
No need for that comment - the struct naming and members should be sufficient.
> +struct ramdisk { > + u64 start_addr; /* ramdisk load address */ > + u64 end_addr; /* ramdisk end address */ > + u64 size; /* ramdisk size */ > + bool reserve_ramdisk; /* do initrd provided by bootloader */ > +}; > > #ifdef CONFIG_X86_32 > /* cpu data as detected by the assembly code in head.S */ > @@ -318,90 +327,84 @@ static u64 __init get_ramdisk_size(void) > return ramdisk_size; > } > > -static void __init relocate_initrd(void) > +static void __init relocate_initrd(struct ramdisk *ramdisk) > { > - /* Assume only end is not page aligned */ > - u64 ramdisk_image = get_ramdisk_image(); > - u64 ramdisk_size = get_ramdisk_size(); > - u64 area_size = PAGE_ALIGN(ramdisk_size); > - > /* We need to move the initrd down into directly mapped mem */ > relocated_ramdisk = memblock_find_in_range(0, PFN_PHYS(max_pfn_mapped), > - area_size, PAGE_SIZE); > + ramdisk->size, PAGE_SIZE); > > if (!relocated_ramdisk) > panic("Cannot find place for new RAMDISK of size %lld\n", > - ramdisk_size); > + ramdisk->size); > > /* Note: this includes all the mem currently occupied by > the initrd, we rely on that fact to keep the data intact. */ > - memblock_reserve(relocated_ramdisk, area_size); > + memblock_reserve(relocated_ramdisk, ramdisk->size); > initrd_start = relocated_ramdisk + PAGE_OFFSET; > - initrd_end = initrd_start + ramdisk_size; > + initrd_end = initrd_start + ramdisk->size; > printk(KERN_INFO "Allocated new RAMDISK: [mem %#010llx-%#010llx]\n",
I think all those printks talking about ramdisk should be
printk(KERN_INFO "RAMDISK: ..."
for easier grepping of dmesg about ramdisk-specific messages. This should be another patch though.
> - relocated_ramdisk, relocated_ramdisk + ramdisk_size - 1); > + relocated_ramdisk, relocated_ramdisk + ramdisk->size - 1); > > - copy_from_early_mem((void *)initrd_start, ramdisk_image, ramdisk_size); > + copy_from_early_mem((void *)initrd_start, ramdisk->start_addr, ramdisk->size); > > printk(KERN_INFO "Move RAMDISK from [mem %#010llx-%#010llx] to" > " [mem %#010llx-%#010llx]\n", > - ramdisk_image, ramdisk_image + ramdisk_size - 1, > - relocated_ramdisk, relocated_ramdisk + ramdisk_size - 1); > + ramdisk->start_addr, ramdisk->start_addr + ramdisk->size - 1, > + relocated_ramdisk, relocated_ramdisk + ramdisk->size - 1); > } > -
That \n should not have been deleted here.
> -static void __init early_reserve_initrd(void) > +static void __init early_reserve_initrd(struct ramdisk *ramdisk) > { > - /* Assume only end is not page aligned */ > - u64 ramdisk_image = get_ramdisk_image(); > - u64 ramdisk_size = get_ramdisk_size(); > - u64 ramdisk_end = PAGE_ALIGN(ramdisk_image + ramdisk_size); > - > - if (!boot_params.hdr.type_of_loader || > - !ramdisk_image || !ramdisk_size) > - return; /* No initrd provided by bootloader */ > - > - memblock_reserve(ramdisk_image, ramdisk_end - ramdisk_image); > + if (!boot_params.hdr.type_of_loader || !ramdisk->start_addr || !ramdisk->size) > + ramdisk->reserve_ramdisk = false; /* No initrd provided by bootloader */ > + else > + memblock_reserve(ramdisk->start_addr, ramdisk->size); > }
Make this one even more readable:
static void __init early_reserve_initrd(struct ramdisk *ramdisk) { /* No initrd provided by bootloader */ if (!boot_params.hdr.type_of_loader || !ramdisk->start_addr || !ramdisk->size) ramdisk->reserve_ramdisk = false; else memblock_reserve(ramdisk->start_addr, ramdisk->size); }
It also has some whitespace damage.
> -static void __init reserve_initrd(void) > + > +static void __init reserve_initrd(struct ramdisk *ramdisk) > { > - /* Assume only end is not page aligned */ > - u64 ramdisk_image = get_ramdisk_image(); > - u64 ramdisk_size = get_ramdisk_size(); > - u64 ramdisk_end = PAGE_ALIGN(ramdisk_image + ramdisk_size); > u64 mapped_size; > > - if (!boot_params.hdr.type_of_loader || > - !ramdisk_image || !ramdisk_size) > - return; /* No initrd provided by bootloader */ > + if (!ramdisk->reserve_ramdisk) > + return; > > initrd_start = 0; > > mapped_size = memblock_mem_size(max_pfn_mapped); > - if (ramdisk_size >= (mapped_size>>1)) > + if (ramdisk->size >= (mapped_size>>1))
Space that shift out:
... mapped_size >> 1))
> panic("initrd too large to handle, " > "disabling initrd (%lld needed, %lld available)\n",
The string in this panic() call should be a single line only for easier grepping. With another patch though.
> - ramdisk_size, mapped_size>>1); > + ramdisk->size, mapped_size>>1);
Space that shift out too.
> > - printk(KERN_INFO "RAMDISK: [mem %#010llx-%#010llx]\n", ramdisk_image, > - ramdisk_end - 1); > + printk(KERN_INFO "RAMDISK: [mem %#010llx-%#010llx]\n", > + ramdisk->start_addr, ramdisk->end_addr - 1); > > - if (pfn_range_is_mapped(PFN_DOWN(ramdisk_image), > - PFN_DOWN(ramdisk_end))) { > + if (pfn_range_is_mapped(PFN_DOWN(ramdisk->start_addr), > + PFN_DOWN(ramdisk->end_addr))) { > /* All are mapped, easy case */ > - initrd_start = ramdisk_image + PAGE_OFFSET; > - initrd_end = initrd_start + ramdisk_size; > + initrd_start = ramdisk->start_addr + PAGE_OFFSET; > + initrd_end = initrd_start + ramdisk->size; > return; > } > > - relocate_initrd(); > + relocate_initrd(ramdisk); > > - memblock_free(ramdisk_image, ramdisk_end - ramdisk_image); > + memblock_free(ramdisk->start_addr, > + ramdisk->end_addr - ramdisk->start_addr);
Arg alignment should start at the function's opening brace.
> } > #else > -static void __init early_reserve_initrd(void) > +static u64 __init get_ramdisk_image(void) > { > + return 0; > } > -static void __init reserve_initrd(void) > +static u64 __init get_ramdisk_size(void) > +{ > + return 0; > +} > +static void __init early_reserve_initrd(struct ramdisk *ramdisk) > +{ > + > +} > +static void __init reserve_initrd(struct ramdisk *ramdisk) > { > } > #endif /* CONFIG_BLK_DEV_INITRD */ > @@ -844,13 +847,21 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p) > * > * Note: On x86_64, fixmaps are ready for use even before this is called. > */ > - > void __init setup_arch(char **cmdline_p) > { > + struct ramdisk ramdisk = { > + .start_addr = get_ramdisk_image(), > + .size = PAGE_ALIGN(get_ramdisk_size()), > + .reserve_ramdisk = true > + };
More readable:
struct ramdisk ramdisk = { .start_addr = get_ramdisk_image(), .size = PAGE_ALIGN(get_ramdisk_size()), .reserve_ramdisk = true, };
> + > + /* Assume end is not page aligned */ > + ramdisk.end_addr = PAGE_ALIGN(ramdisk.start_addr + ramdisk.size); > + > memblock_reserve(__pa_symbol(_text), > (unsigned long)__bss_stop - (unsigned long)_text); > > - early_reserve_initrd(); > + early_reserve_initrd(&ramdisk); > > /* > * At this point everything still needed from the boot loader > @@ -1135,7 +1146,7 @@ void __init setup_arch(char **cmdline_p) > /* Allocate bigger log buffer */ > setup_log_buf(1); > > - reserve_initrd(); > + reserve_initrd(&ramdisk); > > #if defined(CONFIG_ACPI) && defined(CONFIG_BLK_DEV_INITRD) > acpi_initrd_override((void *)initrd_start, initrd_end - initrd_start); > -- > 2.7.2.335.g3476f70 >
-- Regards/Gruss, Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
| |