On 08/27/18 at 01:35pm, Baoquan He wrote:
 
> Is it possible to take num_immovable_mem out from the #ifdef
> CONFIG_MEMORY_HOTREMOVE region so that you can check it earlier to see
> if the old way need be taken? This way, we can reduce one level of
> indentation in the for loop. Just personal thought.

Sorry, there's something wrong with my offlineimap, I thought it was not
sent out, so replied twice. Please ignore this one.

> 
> static bool process_mem_region(struct mem_vector *region,
>                             unsigned long long minimum,
>                             unsigned long long image_size)
> {
>       if (!num_immovable_mem) {
>               slots_count(region, minimum, image_size);
>               
>               if (slot_area_index == MAX_SLOT_AREA) {
>                       debug_putstr("Aborted e820/efi memmap scan (slot_areas 
> full)!\n");
>                       return 1;
>               }
>               return 0;
>       }
> 
> 
> #ifdef CONFIG_MEMORY_HOTREMOVE
>       for (i = 0; i < num_immovable_mem; i++) {
>               ......
>       }
> #endif
> }
> 
> > +           int i;
> > +
> > +           for (i = 0; i < num_immovable_mem; i++) {
> > +                   struct mem_vector entry;
> > +                   unsigned long long start, end, entry_end, region_end;
> > +
> > +                   start = immovable_mem[i].start;
> > +                   end = start + immovable_mem[i].size;
> > +                   region_end = region->start + region->size;
> > +
> > +                   entry.start = clamp(region->start, start, end);
> > +                   entry_end = clamp(region_end, start, end);
> > +
> > +                   if (entry.start + image_size < entry_end) {
> > +                           entry.size = entry_end - entry.start;
> > +                           slots_count(&entry, minimum, image_size);
> > +
> > +                           if (slot_area_index == MAX_SLOT_AREA) {
> > +                                   debug_putstr("Aborted e820/efi memmap 
> > scan (slot_areas full)!\n");
> > +                                   return 1;
> > +                           }
> > +                   }
> > +           }
> > +           return 0;
> > +   }
> > +#endif
> > +   /*
> > +    * If no immovable memory found, or MEMORY_HOTREMOVE disabled,
> > +    * walk all the regions, so use region directely.
> > +    */
> > +   slots_count(region, minimum, image_size);
> > +
> > +   if (slot_area_index == MAX_SLOT_AREA) {
> > +           debug_putstr("Aborted e820/efi memmap scan (slot_areas 
> > full)!\n");
> > +           return 1;
> > +   }
> > +   return 0;
> > +}
> > +
> >  #ifdef CONFIG_EFI
> >  /*
> >   * Returns true if mirror region found (and must have been processed
> > @@ -779,11 +829,8 @@ process_efi_entries(unsigned long minimum, unsigned 
> > long image_size)
> >  
> >             region.start = md->phys_addr;
> >             region.size = md->num_pages << EFI_PAGE_SHIFT;
> > -           process_mem_region(&region, minimum, image_size);
> > -           if (slot_area_index == MAX_SLOT_AREA) {
> > -                   debug_putstr("Aborted EFI scan (slot_areas full)!\n");
> > +           if (process_mem_region(&region, minimum, image_size))
> >                     break;
> > -           }
> >     }
> >     return true;
> >  }
> > @@ -810,11 +857,8 @@ static void process_e820_entries(unsigned long minimum,
> >                     continue;
> >             region.start = entry->addr;
> >             region.size = entry->size;
> > -           process_mem_region(&region, minimum, image_size);
> > -           if (slot_area_index == MAX_SLOT_AREA) {
> > -                   debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> > +           if (process_mem_region(&region, minimum, image_size))
> >                     break;
> > -           }
> >     }
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> > 
> > 

Reply via email to