Hi David, ---- On Tue, 12 Jul 2022 15:31:08 +0800 David Hildenbrand <david@xxxxxxxxxx> wrote --- > On 12.07.22 06:23, Li Chen wrote: > > Hi David, > > ---- On Mon, 11 Jul 2022 22:53:36 +0800 David Hildenbrand <david@xxxxxxxxxx> wrote --- > > > On 11.07.22 14:24, Li Chen wrote: > > > > From: Li Chen <lchen@xxxxxxxxxxxxx> > > > > > > > > When mhp use sparse_add_section, don't check no-map region, > > > > so that to allow no-map reserved memory to get struct page > > > > support. > > > > > > > > Signed-off-by: Li Chen <lchen@xxxxxxxxxxxxx> > > > > Change-Id: I0d2673cec1b66adf695251037a00c240976b226f > > > > --- > > > > mm/sparse.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/mm/sparse.c b/mm/sparse.c > > > > index 120bc8ea5293..a29cd1e7014f 100644 > > > > --- a/mm/sparse.c > > > > +++ b/mm/sparse.c > > > > @@ -690,7 +690,9 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages) > > > > > > > > if (bitmap_empty(map, SUBSECTIONS_PER_SECTION)) > > > > rc = -EINVAL; > > > > - else if (bitmap_intersects(map, subsection_map, SUBSECTIONS_PER_SECTION)) > > > > + else if (memblock_is_map_memory(PFN_PHYS(pfn)) && > > > > + bitmap_intersects(map, subsection_map, > > > > + SUBSECTIONS_PER_SECTION)) > > > > rc = -EEXIST; > > > > else > > > > bitmap_or(subsection_map, map, subsection_map, > > > > > > I'm not sure I follow completely what you are trying to achieve. But if > > > you have to add memblock hacks into mm/sparse.c you're most probably > > > doing something wrong. > > > > > > Please explain why that change is necessary, and why it is safe. > > > > In the current sparse memory model, free_area_init will insert all memblock.memory into subsection_map and no-map rmem is also a > > memblock.memory. So, without this change, fill_subsection_map will return -EEXIST. > > > > I would say it's not a good idea to insert no-map memblock into subsection_map, and I have no idea why sparse do this. > > So, I simply skip no-map region here. > > The thing is: > > if the subsection map is set, then there already *is* a memmap and you > would simply be ignoring it (and overwriting a memmap in e.g., > ZONE_NORMAL to be in ZONE_DEVICE suddenly, which is wrong). > > > Reading memblock_mark_nomap(): > > "The memory regions marked with %MEMBLOCK_NOMAP will not be added to the > direct mapping of the physical memory. These regions will still be > covered by the memory map. The struct page representing NOMAP memory > frames in the memory map will be PageReserved()" > > > So having a memmap for these ranges is expected, and a direct map is not > desired. What you propose is a hack. You either have to reuse the > existing memmap (which is !ZONE_DEVICE -- not sure if that's a problem) > or we'd have to look into teaching init code to not allocate a memmap > for sub-sections that are fully nomap. > > But not sure who depends on the existing memmap for nomap memory. Points taken, thanks! I will try to dig into it. Regards, Li > > > > As for safety: > > 1. The caller of fill_subsection_map are mhp and *_memremap_pages functions, no-map regions are not related to them, so existing codes won't be broken. > > 2. This change doesn't change memblock and subsection_map. > > > > Sorry, but AFAIKT it's a hack and we need a clean way to deal with nomap > memory that already has a memmap instead. > > > -- > Thanks, > > David / dhildenb > >
