[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