Re: [Xen-devel] [PATCH v4] xen: arm: Only lookup kernel/initrd bootmodule once while building dom0.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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 <ian.campbell@xxxxxxxxxx>
> > ---
> > 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.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel