[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 5/6] migration/postcopy: Handle RAMBlocks with a RamDiscar

From: David Hildenbrand
Subject: Re: [PATCH v2 5/6] migration/postcopy: Handle RAMBlocks with a RamDiscardManager on the destination
Date: Fri, 23 Jul 2021 21:01:42 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

On 23.07.21 20:52, Peter Xu wrote:
On Fri, Jul 23, 2021 at 08:36:32PM +0200, David Hildenbrand wrote:
+static int postcopy_request_page(MigrationIncomingState *mis, RAMBlock *rb,
+                                 ram_addr_t start, uint64_t haddr)
+    /*
+     * Discarded pages (via RamDiscardManager) are never migrated. On unlikely
+     * access, place a zeropage, which will also set the relevant bits in the
+     * recv_bitmap accordingly, so we won't try placing a zeropage twice.
+     */
+    if (ramblock_page_is_discarded(rb, start)) {
+        bool received = ramblock_recv_bitmap_test_byte_offset(rb, start);

Will received be set for any case with the current code base?  As I thought
virtio-mem forbids plug/unplug during the whole lifecycle of migration.

receive would only be set if you have two CPUs faulting on the same address
at the same time and the first one already placed a zeropage on this code
path (as the comment said, that will implicitly set it in the rceivedmask).

Ah I see; or just ignore the error of postcopy_place_page_zero() here because
per my understanding this whole path is not expected after all.

See below, if placing would go wrong in this corner case, I think we would still want to know it instead of letting a guest thread not make progress because nobody would wake it up.

Does that make sense?

So, pretty unlikely to happen, but if the stars align ... :)

+        return received ? 0 : postcopy_place_page_zero(mis, (void *)haddr, rb);

(now we can fill up pages in two threads.. but looks thread-safe)

Meanwhile if this is highly not wanted, maybe worth an error_report_once() so
the admin could see something?

You mean, if postcopy_place_page_zero() fails?

I meant ramblock_page_is_discarded() shouldn't really trigger for a sane guest,
right?  Because it means the guest is accessing some unplugged memory.

It can happen in corner cases and is valid: with the current virtio-mem spec, guests are allowed to read unplugged memory. This will, for example, happen on older Linux guests when reading /proc/kcore or (with even older guests) when dumping guest memory via kdump. These corner cases were the main reason why the spec allows for it -- until we have guests properly adjusted such that it won't happen even in corner cases.

A future feature bit will disallow it for the guest: required for supporting shmem/hugetlb cleanly. With that in place, I agree that we would want to warn in this case!


David / dhildenb

reply via email to

[Prev in Thread] Current Thread [Next in Thread]