Re: [PATCH 2/3] mm: Introduce subsection_dev_map

[Dan Williams]

On Wed, Nov 13, 2019 at 10:51 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
>
> On 08.11.19 20:13, Dan Williams wrote:
> > On Thu, Nov 7, 2019 at 4:15 PM Toshiki Fukasawa
> > <t-fukasawa@xxxxxxxxxxxxx> wrote:
> >>
> >> Currently, there is no way to identify pfn on ZONE_DEVICE.
> >> Identifying pfn on system memory can be done by using a
> >> section-level flag. On the other hand, identifying pfn on
> >> ZONE_DEVICE requires a subsection-level flag since ZONE_DEVICE
> >> can be created in units of subsections.
> >>
> >> This patch introduces a new bitmap subsection_dev_map so that
> >> we can identify pfn on ZONE_DEVICE.
> >>
> >> Also, subsection_dev_map is used to prove that struct pages
> >> included in the subsection have been initialized since it is
> >> set after memmap_init_zone_device(). We can avoid accessing
> >> pages currently being initialized by checking subsection_dev_map.
> >>
> >> Signed-off-by: Toshiki Fukasawa <t-fukasawa@xxxxxxxxxxxxx>
> >> ---
> >>   include/linux/mmzone.h | 19 +++++++++++++++++++
> >>   mm/memremap.c          |  2 ++
> >>   mm/sparse.c            | 32 ++++++++++++++++++++++++++++++++
> >>   3 files changed, 53 insertions(+)
> >>
> >> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> >> index bda2028..11376c4 100644
> >> --- a/include/linux/mmzone.h
> >> +++ b/include/linux/mmzone.h
> >> @@ -1174,11 +1174,17 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
> >>
> >>   struct mem_section_usage {
> >>          DECLARE_BITMAP(subsection_map, SUBSECTIONS_PER_SECTION);
> >> +#ifdef CONFIG_ZONE_DEVICE
> >> +       DECLARE_BITMAP(subsection_dev_map, SUBSECTIONS_PER_SECTION);
> >> +#endif
> >
> > Hi Toshiki,
> >
> > There is currently an effort to remove the PageReserved() flag as some
> > code is using that to detect ZONE_DEVICE. In reviewing those patches
> > we realized that what many code paths want is to detect online memory.
> > So instead of a subsection_dev_map add a subsection_online_map. That
> > way pfn_to_online_page() can reliably avoid ZONE_DEVICE ranges. I
> > otherwise question the use case for pfn_walkers to return pages for
> > ZONE_DEVICE pages, I think the skip behavior when pfn_to_online_page()
> > == false is the right behavior.
>
> To be more precise, I recommended an subsection_active_map, to indicate
> which memmaps were fully initialized and can safely be touched (e.g., to
> read the zone/nid). This map would also be set when the devmem memmaps
> were initialized (race between adding memory/growing the section and
> initializing the memmap).
>
> See
>
> https://lkml.org/lkml/2019/10/10/87
>
> and
>
> https://www.spinics.net/lists/linux-driver-devel/msg130012.html

I'm still struggling to understand the motivation of distinguishing
"active" as something distinct from "online". As long as the "online"
granularity is improved from sections down to subsections then most
code paths are good to go. The others can use get_devpagemap() to
check for ZONE_DEVICE in a race free manner as they currently do.

> I dislike a map that is specific to ZONE_DEVICE or (currently)
> !ZONE_DEVICE. I rather want an indication "this memmap is safe to
> touch". As discussed along the mentioned threads, we can combine this
> later with RCU to handle some races that are currently possible.

The rcu protection is independent of the pfn_active vs pfn_online
distinction afaics.