From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756040AbcBIJQf (ORCPT ); Tue, 9 Feb 2016 04:16:35 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:34065 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753229AbcBIJQa (ORCPT ); Tue, 9 Feb 2016 04:16:30 -0500 Date: Tue, 9 Feb 2016 10:16:26 +0100 From: Ingo Molnar To: Alexander Kuleshov Cc: Ingo Molnar , Thomas Gleixner , "H . Peter Anvin" , Borislav Petkov , Joerg Roedel , Dave Young , Andrew Morton , Jiri Kosina , Baoquan He , Paolo Bonzini , Mark Salter , linux-kernel@vger.kernel.org, x86@kernel.org Subject: Re: [PATCH v2] x86/setup: Merge {early_,}reserve_initrd() to one function Message-ID: <20160209091626.GA12570@gmail.com> References: <1454954546-23074-1-git-send-email-kuleshovmail@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1454954546-23074-1-git-send-email-kuleshovmail@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Alexander Kuleshov wrote: > The check and definitions related to ramdisk are similar in the > early_reserve_initrd() and reserve_initrd(). So we can get rid of > early_reserve_initrd() and and use late or early algorithm for > initrd reservation depends on reserve_initrd() parameter value. > > Save 396 bytes of code: > > text data bss dec hex filename > 9281618 5010736 15474688 29767042 1c63582 vmlinux.orig > > text data bss dec hex filename > 9281222 5010672 15474688 29766582 1c633b6 vmlinux.new > > Signed-off-by: Alexander Kuleshov > --- > Changelog: > > v2: parameter of reserve_initrd() - int -> bool. > commit message updated. > > arch/x86/kernel/setup.c | 29 +++++++++-------------------- > 1 file changed, 9 insertions(+), 20 deletions(-) > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index d3d80e6..0a2fa55 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -349,20 +349,7 @@ static void __init relocate_initrd(void) > relocated_ramdisk, relocated_ramdisk + ramdisk_size - 1); > } > > -static void __init early_reserve_initrd(void) > -{ > - /* 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); > -} > -static void __init reserve_initrd(void) > +static void __init reserve_initrd(bool early) > { > /* Assume only end is not page aligned */ > u64 ramdisk_image = get_ramdisk_image(); > @@ -374,6 +361,11 @@ static void __init reserve_initrd(void) > !ramdisk_image || !ramdisk_size) > return; /* No initrd provided by bootloader */ > > + if (early) { > + memblock_reserve(ramdisk_image, ramdisk_end - ramdisk_image); > + return; > + } > + > initrd_start = 0; > > mapped_size = memblock_mem_size(max_pfn_mapped); > @@ -398,10 +390,7 @@ static void __init reserve_initrd(void) > memblock_free(ramdisk_image, ramdisk_end - ramdisk_image); > } > #else > -static void __init early_reserve_initrd(void) > -{ > -} > -static void __init reserve_initrd(void) > +static void __init reserve_initrd(bool early) > { > } > #endif /* CONFIG_BLK_DEV_INITRD */ > @@ -850,7 +839,7 @@ void __init setup_arch(char **cmdline_p) > memblock_reserve(__pa_symbol(_text), > (unsigned long)__bss_stop - (unsigned long)_text); > > - early_reserve_initrd(); > + reserve_initrd(true); > > /* > * At this point everything still needed from the boot loader > @@ -1135,7 +1124,7 @@ void __init setup_arch(char **cmdline_p) > /* Allocate bigger log buffer */ > setup_log_buf(1); > > - reserve_initrd(); > + reserve_initrd(false); > > #if defined(CONFIG_ACPI) && defined(CONFIG_BLK_DEV_INITRD) > acpi_initrd_override((void *)initrd_start, initrd_end - initrd_start); So I don't think the code got easier to understand - in particular the memblock_reserve()/free() pattern, depending on a flag value, is confusing. The duplication is there - but please factor it out into a helper structure ('struct ramdisk') and a helper function that sets up the structure. Thanks, Ingo