On Tue, Jul 7, 2026 at 2:05 AM Peter Xu <[email protected]> wrote:
>
> On Tue, Jun 30, 2026 at 01:49:39PM +0530, Aadeshveer Singh wrote:
> > In fast snapshot load, we would like to serve faults as soon as possible
> > hence loading pages directly instead of requesting a source
> >
> > Add postcopy_mapped_ram_load_page() function which serves single page
> > fault by reading the snapshot file. It uses bitmap_test_and_clear_atomic
> > on pending_bmap to coordinate between threads so each page is loaded
> > exactly once. Non-zero pages are read using qemu_get_buffer_at into a
> > temporary page (for loading page atomically), which is then placed using
> > postcopy_place_page. Zero pages are placed directly using
> > postcopy_place_page_zero.
> >
> > Update postcopy_ram_fault_thread to call postcopy_mapped_ram_load_page
> > instead of requesting source in case of fast snapshot load. to_src_file
> > check is bypassed in fast snapshot load case as there is no source.
> >
> > Add postcopy-blocktime support by calling mark_postcopy_blocktime_begin
> > on every fault. We need to aqcuire the page_request_mutex lock and check
> > the recv bitmap to make sure we prevent a race condition when a fault
> > occurs but before the fault thread can mark the page for tracking the
> > eager thread loads in the page making us fail the assert for recv bitmap
> > being clear while marking in mark_postcopy_blocktime_begin().
> >
> > Allocate another channel in postcopy_temp_pages_setup(like the preempt
> > case), for both the fault thread and eager thread to load pages
> > independently.
> >
> > Signed-off-by: Aadeshveer Singh <[email protected]>
> > ---
> > migration/postcopy-ram.c | 121 +++++++++++++++++++++++++++++++++------
> > 1 file changed, 104 insertions(+), 17 deletions(-)
> >
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 96a65aa976..be9edac572 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -949,6 +949,68 @@ int postcopy_wake_shared(struct PostCopyFD *pcfd,
> > pagesize);
> > }
> >
> > +/**
> > + * postcopy_mapped_ram_load_page() - Load a page to given host address.
> > + * @mis: Migration Incoming State.
> > + * @rb: RAMBlock from where page is loaded.
> > + * @rb_offset: Offset of page in RAMBlock.
> > + * @haddr: Base of page where to load in page.
> > + * @channel: Used to identify between threads and use corresponding temp.
>
> Missing @errp.
>
> > + *
> > + * Load a page from RAMBlock at offset at given host address. Used by
> > postcopy
> > + * ram fault thread and eager thread in fast snapshot load case.
> > + *
> > + * Return: True on success.
> > + */
> > +static bool postcopy_mapped_ram_load_page(MigrationIncomingState *mis,
> > + RAMBlock *rb, ram_addr_t
> > rb_offset,
> > + uint64_t haddr, int channel,
> > + Error **errp)
> > +{
> > + void *place_source = mis->postcopy_tmp_pages[channel].tmp_huge_page;
> > + size_t page;
> > + size_t read;
> > +
> > + page = rb_offset / qemu_ram_pagesize(rb);
>
> Yes, here it is correct to use qemu_ram_pagesize().
>
> > +
> > + if (bitmap_test_and_clear_atomic(rb->pending_bmap, page, 1)) {
> > + if (test_bit(page, rb->file_bmap)) {
>
> We'll need to be careful here when you add huge page support, because
> file_bmap so far is guest-psize based. This line will start to break for
> huge pages, I am not sure if this is the bug you hit when developing huge
> page support, maybe yes?
>
> Not sure how you resolved it there if this is the case, but one idea is
> when postcopy-ram is enabled, you can convert the per-guest-psize bitmap
> into per-host-psize bitmap (when any bit set within the huge page range,
> set the bit in the new bitmap), then here it will be correct.
>
For hugepages, I replaced this test_bit() with a ranged check. I felt
changing the semantics of file_bmap for when postcopy-ram is enabled
might make it more complex with insignificant return.
> > + /*
> > + * This can happen concurrently, but it's thread-safe because
> > + * qemu_get_buffer_at() is thread-safe, and the caller will be
> > using
> > + * different temporary buffers.
> > + */
> > + read = qemu_get_buffer_at(mis->from_src_file, place_source,
> > + qemu_ram_pagesize(rb),
> > + rb->pages_offset + rb_offset);
> > +
> > + if (read != qemu_ram_pagesize(rb)) {
> > + error_setg(errp, "Could not read page %zu from RAM Block
> > %s",
> > + page, rb->idstr);
> > + return false;
> > + }
> > +
> > + if (postcopy_place_page(mis, (void *)haddr, place_source, rb))
> > {
> > + error_setg(errp,
> > + "Failed to place page %zu from RAM Block %s at "
> > + "address %" PRIu64,
> > + page, rb->idstr, haddr);
> > + return false;
> > + }
> > +
>
> Nit: unnecessary newline.
>
> > + } else {
> > + if (postcopy_place_page_zero(mis, (void *)haddr, rb)) {
> > + error_setg(errp,
> > + "Failed to place zero page %zu from RAM Block
> > %s at "
> > + "address %" PRIu64,
> > + page, rb->idstr, haddr);
> > + return false;
> > + }
> > + }
> > + }
> > + return true;
> > +}
> > +
> > /*
> > * NOTE: @tid is only used when postcopy-blocktime feature is enabled, and
> > * also optional: when zero is provided, the fault accounting will be
> > ignored.
> > @@ -1279,6 +1341,7 @@ static void *postcopy_ram_fault_thread(void *opaque)
> > int ret;
> > size_t index;
> > RAMBlock *rb = NULL;
> > + Error *local_err = NULL;
> >
> > trace_postcopy_ram_fault_thread_entry();
> > rcu_register_thread();
> > @@ -1320,11 +1383,13 @@ static void *postcopy_ram_fault_thread(void *opaque)
> > break;
> > }
> >
> > - if (!mis->to_src_file) {
> > + if (!migrate_mapped_ram() && !mis->to_src_file) {
> > /*
> > - * Possibly someone tells us that the return path is
> > - * broken already using the event. We should hold until
> > - * the channel is rebuilt.
> > + * Possibly someone tells us that the return path is broken
> > already
> > + * using the event. We should hold until the channel is
> > rebuilt.
> > + * Fast snapshot load doesn't support pause and recover,
> > because
> > + * it's not necessary: we can fail right away when QEMU just
> > booted
> > + * with nothing to lose.
> > */
> > postcopy_pause_fault_thread(mis);
> > }
> > @@ -1387,18 +1452,37 @@ static void *postcopy_ram_fault_thread(void *opaque)
> > qemu_ram_get_idstr(rb),
> > rb_offset,
> >
> > msg.arg.pagefault.feat.ptid);
> > +
> > + if (migrate_mapped_ram()) {
> > + /* Load page directly in case of fast snapshot load */
> > + WITH_QEMU_LOCK_GUARD(&mis->page_request_mutex) {
> > + if (!ramblock_recv_bitmap_test(
> > + rb, (void *)msg.arg.pagefault.address)) {
> > + mark_postcopy_blocktime_begin(
> > + msg.arg.pagefault.address,
> > + msg.arg.pagefault.feat.ptid, rb);
> > + }
> > + }
>
> This part needs some more thought.
>
> First of all, if the page is set already in receivedmap, it means it's
> already installed, we shouldn't bother postcopy_mapped_ram_load_page().
>
> But the problem is not only about that.
>
> So far looks like the blocktime feature relies on the receivedmap, then it
> means the easy way for this series is to support receivedmap too. Then
> with receivedmap, we should unify the receivedmap detection rather than
> testing the bit at multiple places.
>
> Say, you can provide a small helper like this:
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 278cad502a..79f737bc69 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -573,12 +573,10 @@ int
> migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
> return migrate_send_rp_message(mis, msg_type, msglen, bufc);
> }
>
> -int migrate_send_rp_req_pages(MigrationIncomingState *mis,
> - RAMBlock *rb, ram_addr_t start, uint64_t haddr,
> - uint32_t tid)
> +bool postcopy_page_received(MigrationIncomingState *mis, RAMBlock *rb,
> + ram_addr_t start, uint64_t haddr, uint32_t tid)
> {
> - void *aligned = (void *)(uintptr_t)ROUND_DOWN(haddr,
> qemu_ram_pagesize(rb));
> - bool received = false;
> + bool received;
>
> WITH_QEMU_LOCK_GUARD(&mis->page_request_mutex) {
> received = ramblock_recv_bitmap_test_byte_offset(rb, start);
> @@ -598,11 +596,20 @@ int migrate_send_rp_req_pages(MigrationIncomingState
> *mis,
> }
> }
>
> + return received;
> +}
> +
> +int migrate_send_rp_req_pages(MigrationIncomingState *mis,
> + RAMBlock *rb, ram_addr_t start, uint64_t haddr,
> + uint32_t tid)
> +{
> + void *aligned = (void *)(uintptr_t)ROUND_DOWN(haddr,
> qemu_ram_pagesize(rb));
> +
> /*
> * If the page is there, skip sending the message. We don't even need
> the
> * lock because as long as the page arrived, it'll be there forever.
> */
> - if (received) {
> + if (postcopy_page_received(mis, rb, start, aligned, tid)) {
> return 0;
> }
>
> Then I think we can use postcopy_page_received() here to avoid duplicating
> the code.
>
> It means with your feature we will also maintain the tree of page_requested
> but I think it's fine; if we really want we can make that optional based on
> whether mapped-ram enabled, but not yet required.
>
> PS: I feel like there was a bug in existing code passing "haddr" (rather
> than "aligned") into mark_postcopy_blocktime_begin() in the current impl of
> migrate_send_rp_req_pages(). Please help to check if it's an issue, if
> true we can have a separate patch fixing that first. In general, I think
> we should use a page-aligned address in both the tree and blocktime.
>
Thanks for the snippet, I will integrate it in the next version.
You are right to point out the alignment bug, I will resolve that in
the next version too.
> > + if (!postcopy_mapped_ram_load_page(
> > + mis, rb, rb_offset, msg.arg.pagefault.address,
> > + RAM_CHANNEL_POSTCOPY, &local_err)) {
> > + error_report_err(local_err);
> > + break;
> > + }
> > + } else {
> > retry:
> > - /*
> > - * Send the request to the source - we want to request one
> > - * of our host page sizes (which is >= TPS)
> > - */
> > - ret = postcopy_request_page(mis, rb, rb_offset,
> > - msg.arg.pagefault.address,
> > - msg.arg.pagefault.feat.ptid);
> > - if (ret) {
> > - /* May be network failure, try to wait for recovery */
> > - postcopy_pause_fault_thread(mis);
> > - goto retry;
> > + /*
> > + * Send the request to the source - we want to request one
> > + * of our host page sizes (which is >= TPS)
> > + */
> > + ret = postcopy_request_page(mis, rb, rb_offset,
> > + msg.arg.pagefault.address,
> > + msg.arg.pagefault.feat.ptid);
> > + if (ret) {
> > + /* May be network failure, try to wait for recovery */
> > + postcopy_pause_fault_thread(mis);
> > + goto retry;
> > + }
> > }
> > }
> >
> > @@ -1470,8 +1554,11 @@ static int
> > postcopy_temp_pages_setup(MigrationIncomingState *mis, Error **errp)
> > unsigned i, channels;
> > void *temp_page;
> >
> > - if (migrate_postcopy_preempt()) {
> > - /* If preemption enabled, need extra channel for urgent requests */
> > + if (migrate_postcopy_preempt() || migrate_mapped_ram()) {
> > + /*
> > + * If preemption enabled or it is fast snapshot load, need extra
> > channel
> > + * for urgent requests/faults
> > + */
> > mis->postcopy_channels = RAM_CHANNEL_MAX;
> > } else {
> > /* Both precopy/postcopy on the same channel */
> > --
> > 2.54.0
> >
>
> --
> Peter Xu
>
