From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v4] xen: arm: Only lookup kernel/initrd bootmodule once while building dom0. Date: Mon, 21 Jul 2014 13:30:16 +0100 Message-ID: <53CD07D8.6000604@linaro.org> References: <1405944556-8372-1-git-send-email-ian.campbell@citrix.com> <53CD05A6.2020203@linaro.org> <1405945441.25022.48.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1405945441.25022.48.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: stefano.stabellini@eu.citrix.com, tim@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 07/21/2014 01:24 PM, Ian Campbell wrote: > On Mon, 2014-07-21 at 13:20 +0100, Julien Grall wrote: >> On 07/21/2014 01:09 PM, Ian Campbell wrote: >>> Signed-off-by: Ian Campbell >>> --- >>> v4: Call the new field kernel_bootmodule for clarity >>> Use the new field throughout kernel.c as well as in domain_build.c >>> Add and use initrd_bootmodule too. >>> Const up the uses >>> v3: New patch >>> --- >>> xen/arch/arm/domain_build.c | 6 +++--- >>> xen/arch/arm/kernel.c | 2 ++ >>> xen/arch/arm/kernel.h | 1 + >>> 3 files changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >>> index 154367e..23261e4 100644 >>> --- a/xen/arch/arm/domain_build.c >>> +++ b/xen/arch/arm/domain_build.c >>> @@ -405,7 +405,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >>> int res = 0; >>> int had_dom0_bootargs = 0; >>> >>> - struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL); >>> + const struct bootmodule *mod = kinfo->kernel_bootmodule; >>> >>> if ( mod && mod->cmdline[0] ) >>> bootargs = &mod->cmdline[0]; >>> @@ -455,7 +455,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >>> >>> if ( dt_node_path_is_equal(node, "/chosen") ) >>> { >>> - struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_RAMDISK); >>> + const struct bootmodule *mod = kinfo->initrd_bootmodule; >> >> Technically speaking, we only execute once this part of the code. So >> having a new field in kernel_info doesn't seem useful. > > It seemed to me to be consistent to only lookup the ramdisk once too, > instead of here and in initrd_load(). > >> BTW, the function already have a variable "mod" defined at the begining. >> I would rename this variable to modinitrd or smth different to shadow >> the previous variable. > > I noticed that too. Please do. Ok. I will send a follow-up to fix it. Acked-by: Julien Grall Regards, -- Julien Grall