[Skiboot] [PATCH] Adjust skiboot_cpu_stacks region size according to real max PIR

Benjamin Herrenschmidt benh at kernel.crashing.org
Wed Apr 29 20:21:30 AEST 2015


On Wed, 2015-04-29 at 20:20 +1000, Benjamin Herrenschmidt wrote:
> Cosmetic comments only:
> 
> > diff --git a/core/init.c b/core/init.c
> > index 445272a..0e91a9d 100644
> > --- a/core/init.c
> > +++ b/core/init.c
> > @@ -634,6 +634,8 @@ void __noreturn main_cpu_entry(const void *fdt, u32 master_cpu)
> >  
> >  	/* Initialize the rest of the cpu thread structs */
> >  	init_all_cpus();
> > +	/* We now know real max PIR, so adjust mem region appropriately */
> > +	adjust_cpu_stacks_len_to_max_pir();
> 
> I like having a blank line between comment+function :-)
> 
> Also I tend to dislike enormous_function_names_from_hell().
> 
> What about adjust_cpu_stacks_alloc() ?

Actually, I have an even better idea, why not call it from
init_all_cpus() ?

> >  	/* Allocate our split trace buffers now. Depends add_opal_node() */
> >  	init_trace_buffers();
> > diff --git a/core/mem_region.c b/core/mem_region.c
> > index 5a496aa..569f8fd 100644
> > --- a/core/mem_region.c
> > +++ b/core/mem_region.c
> > @@ -725,6 +725,15 @@ struct mem_region *find_mem_region(const char *name)
> >  	return NULL;
> >  }
> >  
> > +void adjust_cpu_stacks_len_to_max_pir(void)
> > +{
> > +	/* CPU stacks start at 0, then when we know max possible PIR,
> > +	 * we adjust, then when we bring all CPUs online we know the
> > +	 * runtime max PIR, so we adjust this a few times during boot.
> > +	 */
> > +	skiboot_cpu_stacks.len = (cpu_max_pir + 1) * STACK_SIZE;
> > +}
> > +
> >  /* Trawl through device tree, create memory regions from nodes. */
> >  void mem_region_init(void)
> >  {
> > @@ -773,8 +782,7 @@ void mem_region_init(void)
> >  		unlock(&mem_region_lock);
> >  	}
> >  
> > -	/* Now we know how many CPU stacks we have, fix that up. */
> > -	skiboot_cpu_stacks.len = (cpu_max_pir + 1) * STACK_SIZE;
> > +	adjust_cpu_stacks_len_to_max_pir();
> 
> In fact the comment used to be right... it's just that I later on
> shuffled things around and made the mem region init happen earlier ...
> 
> >  	lock(&mem_region_lock);
> >  
> > diff --git a/include/mem_region.h b/include/mem_region.h
> > index 23ee18c..96dfddc 100644
> > --- a/include/mem_region.h
> > +++ b/include/mem_region.h
> > @@ -60,7 +60,7 @@ void mem_region_release_unused(void);
> >  extern struct mem_region skiboot_heap;
> >  
> >  void mem_region_init(void);
> > -
> > +void adjust_cpu_stacks_len_to_max_pir(void);
> >  void mem_region_add_dt_reserved(void);
> >  
> >  /* Mark memory as reserved */
> 




More information about the Skiboot mailing list