From: Peter Xu <peterx@redhat.com>
To: Denis Plotnikov <dplotnikov@virtuozzo.com>
Cc: den_plotnic@mail.ru, quintela@redhat.com, armbru@redhat.com,
qemu-devel@nongnu.org, pbonzini@redhat.com, den@openvz.org,
dgilbert@redhat.com
Subject: Re: [PATCH v0 3/4] migration: add background snapshot
Date: Wed, 29 Jul 2020 12:58:07 -0400 [thread overview]
Message-ID: <20200729165807.GC89946@xz-x1.hitronhub.home> (raw)
In-Reply-To: <f025a2c9-fe4e-5d0d-5fb8-bed35da77952@virtuozzo.com>
On Wed, Jul 29, 2020 at 03:54:46PM +0300, Denis Plotnikov wrote:
> > Besides current solution, do you think we can make it simpler by only deliver
> > the fault request to the background thread? We can let the background thread
> > to do all the rests and IIUC we can drop all the complicated sync bitmaps and
> > so on by doing so. The process can look like:
> >
> > - background thread runs the general precopy migration, and,
> >
> > - it only does the ram_bulk_stage, which is the first loop, because for
> > snapshot no reason to send a page twice..
> >
> > - After copy one page, do ram_set_rw() always, so accessing of this page
> > will never trigger write-protect page fault again,
> >
> > - take requests from the unqueue_page() just like what's done in this
> > series, but instead of copying the page, the page request should look
> > exactly like the postcopy one. We don't need copy_page because the page
> > won't be changed before we unprotect it, so it shiould be safe. These
> > pages will still be with high priority because when queued it means vcpu
> > writed to this protected page and fault in userfaultfd. We need to
> > migrate these pages first to unblock them.
> >
> > - the fault handler thread only needs to do:
> >
> > - when get a uffd-wp message, translate into a postcopy-like request
> > (calculate ramblock and offset), then queue it. That's all.
> >
> > I believe we can avoid the copy_page parameter that was passed around, and we
> > can also save the two extra bitmaps and the complicated synchronizations.
> >
> > Do you think this would work?
>
> Yes, it would. This scheme is much simpler. I like it, in general.
>
> I use such a complicated approach to reduce all possible vCPU delays:
> if the storage where the snapshot is being saved is quite slow, it could
> lead
> to vCPU freezing until the page is fully written to the storage.
> So, with the current approach, if not take into account a number of page
> copies limitation,
> the worst case is all VM's ram is copied and then written to the storage.
> Other words, the current scheme provides minimal vCPU delays and thus
> minimal VM cpu
> performance slowdown with the cost of host's memory consumption.
> The new scheme is simple, doesn't consume extra host memory but can freeze
> vCPUs for
> longer time r because:
> * usually memory page coping is faster then memory page writing to a storage
> (think of HDD disk)
> * writing page to a disk depends on disk performance and current disk load
>
> So it seems that we have two different strategies:
> 1. lower CPU delays
> 2. lower memory usage
>
> To be honest I would start from the yours scheme as it much simler and the
> other if needed in the future.
>
> What do you think?
Looks good to me.
Btw, IIUC scheme 1 can also be seen as a way to buffer the duplicated pages in
RAM. If that's the case, another implementation (even if we want to implement
that in the future, but I still doubt it...) is to grant tuables to the current
migration channel to take more pages in the buffer cache. Currently, the
migration channel does buffering in QEMUFile.buf[IO_BUF_SIZE], where
IO_BUF_SIZE is 32K as constant.
Anyway, we can start with the simple scheme.
Thanks!
--
Peter Xu
next prev parent reply other threads:[~2020-07-29 17:00 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-22 8:11 [PATCH v0 0/4] background snapshot Denis Plotnikov
2020-07-22 8:11 ` [PATCH v0 1/4] bitops: add some atomic versions of bitmap operations Denis Plotnikov
2020-07-22 8:11 ` [PATCH v0 2/4] migration: add background snapshot capability Denis Plotnikov
2020-07-23 22:21 ` Peter Xu
2020-07-22 8:11 ` [PATCH v0 3/4] migration: add background snapshot Denis Plotnikov
2020-07-23 22:15 ` Peter Xu
2020-07-29 12:27 ` Denis Plotnikov
2020-07-24 0:08 ` Peter Xu
2020-07-29 12:54 ` Denis Plotnikov
2020-07-29 16:58 ` Peter Xu [this message]
2020-07-27 16:48 ` Dr. David Alan Gilbert
2020-07-28 9:34 ` Denis Plotnikov
2020-07-29 13:27 ` Dr. David Alan Gilbert
2020-07-29 13:57 ` Denis Plotnikov
2020-07-22 8:11 ` [PATCH v0 4/4] background snapshot: add trace events for page fault processing Denis Plotnikov
2020-07-22 14:50 ` [PATCH v0 0/4] background snapshot Peter Xu
2020-07-22 15:42 ` Denis Plotnikov
2020-07-22 15:47 ` Denis Plotnikov
2020-07-22 16:30 ` Peter Xu
2020-07-23 8:03 ` Denis Plotnikov
2020-07-23 17:39 ` Peter Xu
2020-07-24 8:06 ` Denis Plotnikov
2020-07-24 16:31 ` Peter Xu
2020-07-27 16:59 ` Dr. David Alan Gilbert
2020-08-04 13:11 ` Zhanghailiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200729165807.GC89946@xz-x1.hitronhub.home \
--to=peterx@redhat.com \
--cc=armbru@redhat.com \
--cc=den@openvz.org \
--cc=den_plotnic@mail.ru \
--cc=dgilbert@redhat.com \
--cc=dplotnikov@virtuozzo.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).