On 20.02.20 05:33, Baoquan He wrote: > Wrap the codes filling subsection map from section_activate() into "Factor out the code that fills the subsection" ... > fill_subsection_map(), this makes section_activate() cleaner and > easier to follow. > > Signed-off-by: Baoquan He <bhe@xxxxxxxxxx> > --- > mm/sparse.c | 45 ++++++++++++++++++++++++++++++++++----------- > 1 file changed, 34 insertions(+), 11 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index b8e52c8fed7f..977b47acd38d 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -790,24 +790,28 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages, > ms->section_mem_map = (unsigned long)NULL; > } > > -static struct page * __meminit section_activate(int nid, unsigned long pfn, > - unsigned long nr_pages, struct vmem_altmap *altmap) > +/** > + * fill_subsection_map - fill subsection map of a memory region > + * @pfn - start pfn of the memory range > + * @nr_pages - number of pfns to add in the region > + * > + * This fills the related subsection map inside one section, and only > + * intended for hotplug. > + * > + * Return: > + * * 0 - On success. > + * * -EINVAL - Invalid memory region. > + * * -EEXIST - Subsection map has been set. > + */ Without this comment (or a massively reduced one :) ) Reviewed-by: David Hildenbrand <david@xxxxxxxxxx> -- Thanks, David / dhildenb