Re: [PATCH v5 1/5] mm,memory_hotplug: Allocate memmap from the added memory range

[David Hildenbrand]

On 26.03.21 16:31, Michal Hocko wrote:
> On Fri 26-03-21 15:53:41, David Hildenbrand wrote:
> > On 26.03.21 15:38, Michal Hocko wrote:
> > > On Fri 26-03-21 09:52:58, David Hildenbrand wrote:
> [...]
> > > > 2. We won't allocate kasan shadow memory. We most probably have to do it
> > > > explicitly via kasan_add_zero_shadow()/kasan_remove_zero_shadow(), see
> > > > mm/memremap.c:pagemap_range()
> > >
> > > I think this is similar to the above. Does kasan has to know about
> > > memory which will never be used for anything?
> >
> > IIRC, kasan will track read/writes to the vmemmap as well. So it could
> > theoretically detect if we read from the vmemmap before writing
> > (initializing) it IIUC.
> > This is also why mm/memremap.c does a kasan_add_zero_shadow() before the
> > move_pfn_range_to_zone()->memmap_init_range() for the whole region,
> > including altmap space.
> >
> > Now, I am no expert on KASAN, what would happen in case we have access to
> > non-tracked memory.
> >
> > commit 0207df4fa1a869281ddbf72db6203dbf036b3e1a
> > Author: Andrey Ryabinin <ryabinin.a.a@xxxxxxxxx>
> > Date:   Fri Aug 17 15:47:04 2018 -0700
> >
> >      kernel/memremap, kasan: make ZONE_DEVICE with work with KASAN
> >
> > indicates that kasan will crash the system on "non-existent shadow memory"
>
> Interesting. Thanks for the pointer.
>
> > > > Further a locking rework might be necessary. We hold the device hotplug
> > > > lock, but not the memory hotplug lock. E.g., for get_online_mems(). Might
> > > > have to move that out online_pages.
> > >
> > > Could you be more explicit why this locking is needed? What it would
> > > protect from for vmemmap pages?
> >
> > One example is in mm/kmemleak.c:kmemleak_scan(), where we scan the vmemmap
> > for pointers. We don't want the vmemmap to get unmapped while we are working
> > on it (-> fault).
>   
> Hmm, but they are not going away during offline. They just have a less
> defined state. Or what exactly do you mean by unmapped?

Hmm, also true. We should double check if we're touching other code that 
should better be synchronized with the memory hotplug lock.

--
Thanks,

David / dhildenb