Thread (1 message) 1 message, 1 author, 2020-10-16

Re: [PATCH v1 05/29] virtio-mem: generalize check for added memory

From: David Hildenbrand <hidden>
Date: 2020-10-16 09:11:56
Also in: linux-mm, lkml

quoted
That's an interesting corner case. Assume you have a 128MB memory block
but only 64MB are plugged.
Since we just plug a part of memory block, this state is OFFLINE_PARTIAL
first. But then we would add these memory and online it. This means the state
of this memory block is ONLINE_PARTIAL.

When this state is changed to OFFLINE_PARTIAL again?
Please note that memory onlining is *completely* controllable by user
space. User space can offline/online memory blocks as it wants. Not
saying this might actually be the right thing to do - but we cannot
trust that user space does the right thing.

So at any point in time, you have to assume that

a) added memory might not get onlined
b) previously onlined memory might get offlined
c) previously offline memory might get onlined
quoted
As long as we have our online_pages callback in place, we can hinder the
unplugged 64MB from getting exposed to the buddy
(virtio_mem_online_page_cb()). However, once we unloaded the driver,
Yes,

virtio_mem_set_fake_offline() would __SetPageOffline() to those pages.
quoted
this is no longer the case. If someone would online that memory block,
we would expose unplugged memory to the buddy - very bad.
Per my understanding, at this point of time, the memory block is at online
state. Even part of it is set to *fake* offline.

So how could user trigger another online from sysfs interface?
Assume we added a partially plugged memory block, which is now offline.
Further assume user space did not online the memory block (e.g., no udev
rules).

User space could happily online the block after unloading the driver.
Again, we have to assume user space could do crazy things.
quoted
So we have to remove these partially plugged, offline memory blocks when
losing control over them.

I tried to document that via:

"After we unregistered our callbacks, user space can online partially
plugged offline blocks. Make sure to remove them."
quoted
Also, during virtio_mem_remove(), we just handle OFFLINE_PARTIAL memory block.
How about memory block in other states? It is not necessary to remove
ONLINE[_PARTIAL] memroy blocks?
Blocks that are fully plugged (ONLINE or OFFLINE) can get
onlined/offlined without us having to care. Works fine - we only have to
care about partially plugged blocks.

While we *could* unplug OFFLINE blocks, there is no way we can
deterministically offline+remove ONLINE blocks. So that memory has to
stay, even after we unloaded the driver (similar to the dax/kmem driver).
For OFFLINE memory blocks, would that leave the situation:

Guest doesn't need those pages, while host still maps them?
Yes, but the guest could online the memory and make use of it.

(again, whoever decides to unload the driver better be knowing what he does)

To do it even more cleanly, we would

a) Have to remove completely plugged offline blocks (not done)
b) Have to remove partially plugged offline blocks (done)
c) Actually send unplug requests to the hypervisor

Right now, only b) is done, because it might actually cause harm (as
discussed). However, the problem is, that c) might actually fail.

Long short: we could add a) if it turns out to be a real issue. But
than, unloading the driver isn't really suggested, the current
implementation just "keeps it working without crashes" - and I guess
that's good enough for now.
quoted
ONLINE_PARTIAL is already taken care of: it cannot get offlined anymore,
as we still hold references to these struct pages
(virtio_mem_set_fake_offline()), and as we no longer have the memory
notifier in place, we can no longer agree to offline this memory (when
going_offline).
Ok, I seems to understand the logic now.

But how we prevent ONLINE_PARTIAL memory block get offlined? There are three
calls in virtio_mem_set_fake_offline(), while all of them adjust page's flag.
How they hold reference to struct page?
Sorry, I should have given you the right pointer. (similar to my other
reply)

We hold a reference either via

1. alloc_contig_range()
2. memmap init code, when not calling generic_online_page().

So these fake-offline pages can never be actually offlined, because we
no longer have the memory notifier registered to fix that up.

-- 
Thanks,

David / dhildenb

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help