On 02/28/20 at 03:27pm, David Hildenbrand wrote: > 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" ... Fine to me, I will replace it with this. Thanks. > > > 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 :) ) Yeah, as we discussed, I will remove it. > > Reviewed-by: David Hildenbrand <david@xxxxxxxxxx> > > -- > Thanks, > > David / dhildenb