Re: [PATCH v2 2/2] mm/memory-failure.c: Don't access uninitialized memmaps in memory_failure()

[David Hildenbrand]

On 19.10.19 04:05, Andrew Morton wrote:
> On Thu, 10 Oct 2019 09:17:42 +0200 David Hildenbrand <david@xxxxxxxxxx> wrote:
>
> > > > -	pgmap = get_dev_pagemap(pfn, NULL);
> > > > -	if (pgmap)
> > > > -		return memory_failure_dev_pagemap(pfn, flags, pgmap);
> > > > -
> > > > -	p = pfn_to_page(pfn);
> > >
> > > This change seems to assume that memory_failure_dev_pagemap() is never
> > > called for online pages. Is it an intended behavior?
> > > Or the concept "online pages" is not applicable to zone device pages?
> >
> > Yes, that's the real culprit. ZONE_DEVICE/devmem pages are never online
> > (SECTION_IS_ONLINE). The terminology "online" only applies to pages that
> > were given to the buddy. And as we support sup-section hotadd for
> > devmem, we cannot easily make use of the section flag it. I already
> > proposed somewhere to convert SECTION_IS_ONLINE to a subsection bitmap
> > and call it something like pfn_active().
> >
> > pfn_online() would then be "pfn_active() && zone != ZONE_DEVICE". And we
> > could use pfn_active() everywhere to test for initialized memmaps (well,
> > besides some special cases like device reserved memory that does not
> > span full sub-sections). Until now, nobody volunteered and I have other
> > things to do.
>
> Is it worth a code comment or two to make this clearer?

You mean something like

/* Only pages managed by the buddy are online (not ZONE_DEVICE). */

?

--

Thanks,

David / dhildenb