Re: [PATCH v3] Xen: Spread boot time page scrubbing across all available CPU's

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Apr 11, 2014 at 08:19:27PM +0100, Julien Grall wrote:
> Hi Konrad,
> 
> This patch will break ARM build (see below).
> 
> On 04/11/2014 07:08 PM, Konrad Rzeszutek Wilk wrote:
> > +void __init smp_scrub_heap_pages(void *data)
> > +{
> > +    unsigned long mfn, start, end;
> > +    struct page_info *pg;
> > +    struct scrub_region *r;
> > +    unsigned int temp_cpu, node, cpu_idx = 0;
> > +    unsigned int cpu = smp_processor_id();
> > +
> > +    if ( data )
> > +        r = data;
> > +    else {
> 
> else
> {

Right.
> 
> > +        node = cpu_to_node(cpu);
> > +        if ( node == NUMA_NO_NODE )
> > +            return;
> > +        r = &region[node];
> > +    }
> > +    ASSERT(r != NULL);
> > +
> > +    /* Determine the current CPU's index into CPU's linked to this node*/
> > +    for_each_cpu ( temp_cpu, &r->cpu )
> > +    {
> > +        if ( cpu == temp_cpu )
> > +            break;
> > +        cpu_idx++;
> > +    }
> > +
> > +    /* Calculate the starting mfn for this CPU's memory block */
> > +    start = r->start + (r->per_cpu_sz * cpu_idx) + r->offset;
> > +
> > +    /* Calculate the end mfn into this CPU's memory block for this iteration */
> > +    if ( r->offset + chunk_size > r->per_cpu_sz ) {
> 
> if ( ... )
> {
> 
> [..]
<sigh>
> 
> >  void __init scrub_heap_pages(void)
> >  {
> > -    unsigned long mfn;
> > -    struct page_info *pg;
> > +    cpumask_t node_cpus, temp_cpus, all_worker_cpus = {{ 0 }};
> > +    unsigned int i, j, cpu, sibling;
> > +    unsigned long offset, max_per_cpu_sz = 0;
> > +    unsigned long start, end;
> > +    unsigned long rem = 0;
> >  
> >      if ( !opt_bootscrub )
> >          return;
> >  
> > -    printk("Scrubbing Free RAM: ");
> > +    /* Scrub block size */
> > +    chunk_size = opt_bootscrub_chunk >> PAGE_SHIFT;
> > +    if ( chunk_size == 0 )
> > +        chunk_size = 1;
> >  
> > -    for ( mfn = first_valid_mfn; mfn < max_page; mfn++ )
> > +    /* Round #0 - figure out amounts and which CPUs to use */
> > +    for_each_online_node ( i )
> >      {
> > +        /* Calculate Node memory start and end address */
> > +        start = max(node_start_pfn(i), first_valid_mfn);
> 
> node_start_pfn doesn't exists on ARM.
> As we don't yet support NUMA, you will have to add a dummy #define in
> asm-arm/numa.h which will return the correct value.

OK. Will add that.
> 
> > +        end = min(node_start_pfn(i) + node_spanned_pages(i), max_page);
> > +        /* CPUs that are online and on this node (if none, that it is OK */
> > +        cpumask_and(&node_cpus, &node_to_cpumask(i), &cpu_online_map);
> > +        cpumask_copy(&temp_cpus, &node_cpus);
> > +        /* Rip out threads. */
> > +        for_each_cpu ( j, &temp_cpus )
> > +        {
> > +            cpu = 0;
> > +            for_each_cpu(sibling, per_cpu(cpu_sibling_mask, j)) {
> 
> for_each_cpu( ... )
> {
> 
> > +                if (cpu++ == 0) /* Skip 1st CPU - the core */
> 
> if ( ... )
> 
> > +                    continue;
> > +                cpumask_clear_cpu(sibling, &node_cpus);
> > +            }
> > +        }
> > +        cpumask_or(&all_worker_cpus, &all_worker_cpus, &node_cpus);
> > +        if ( cpumask_empty(&node_cpus) ) { /* No CPUs on this node. */
> 
> if ( ... )
> {
> 
> Regards,

Thanks for your review!
> 
> -- 
> Julien Grall
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel