Re: [RFC PATCH v2] mm: initialize struct pages reserved by ZONE_DEVICE driver.

[Dan Williams]

On Mon, Sep 9, 2019 at 1:11 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
[..]
> >> It seems that SECTION_IS_ONLINE and SECTION_MARKED_PRESENT can be used to
> >> distinguish uninitialized struct pages if we can apply them to ZONE_DEVICE,
> >> but that is no longer necessary with this approach.
> >
> > Let's take a step back here to understand the issues I am aware of. I
> > think we should solve this for good now:
> >
> > A PFN walker takes a look at a random PFN at a random point in time. It
> > finds a PFN with SECTION_MARKED_PRESENT && !SECTION_IS_ONLINE. The
> > options are:
> >
> > 1. It is buddy memory (add_memory()) that has not been online yet. The
> > memmap contains garbage. Don't access.
> >
> > 2. It is ZONE_DEVICE memory with a valid memmap. Access it.
> >
> > 3. It is ZONE_DEVICE memory with an invalid memmap, because the section
> > is only partially present: E.g., device starts at offset 64MB within a
> > section or the device ends at offset 64MB within a section. Don't access it.
> >
> > 4. It is ZONE_DEVICE memory with an invalid memmap, because the memmap
> > was not initialized yet. memmap_init_zone_device() did not yet succeed
> > after dropping the mem_hotplug lock in mm/memremap.c. Don't access it.
> >
> > 5. It is reserved ZONE_DEVICE memory ("pages mapped, but reserved for
> > driver") with an invalid memmap. Don't access it.
> >
> > I can see that your patch tries to make #5 vanish by initializing the
> > memmap, fair enough. #3 and #4 can't be detected. The PFN walker could
> > still stumble over uninitialized memmaps.
> >
>
> FWIW, I thinkg having something like pfn_zone_device(), similarly
> implemented like pfn_zone_device_reserved() could be one solution to
> most issues.

I've been thinking of a replacement for PTE_DEVMAP with section-level,
or sub-section level flags. The section-level flag would still require
a call to get_dev_pagemap() to validate that the pfn is not section in
the subsection case which seems to be entirely too much overhead. If
ZONE_DEVICE is to be a first class citizen in pfn walkers I think it
would be worth the cost to double the size of subsection_map and to
identify whether a sub-section is ZONE_DEVICE, or not.

Thoughts?