qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v4 10/19] migration: Postcopy preemption enablement


From: Peter Xu
Subject: Re: [PATCH v4 10/19] migration: Postcopy preemption enablement
Date: Fri, 13 May 2022 15:31:22 -0400

On Sat, May 14, 2022 at 12:23:44AM +0530, manish.mishra wrote:
> 
> On 12/05/22 9:52 pm, Peter Xu wrote:
> > Hi, Manish,
> > 
> > On Wed, May 11, 2022 at 09:24:28PM +0530, manish.mishra wrote:
> > > > @@ -1962,9 +2038,17 @@ static bool get_queued_page(RAMState *rs, 
> > > > PageSearchStatus *pss)
> > > >        RAMBlock  *block;
> > > >        ram_addr_t offset;
> > > > +again:
> > > >        block = unqueue_page(rs, &offset);
> > > > -    if (!block) {
> > > > +    if (block) {
> > > > +        /* See comment above postcopy_preempted_contains() */
> > > > +        if (postcopy_preempted_contains(rs, block, offset)) {
> > > > +            trace_postcopy_preempt_hit(block->idstr, offset);
> > > > +            /* This request is dropped */
> > > > +            goto again;
> > > > +        }
> > > If we continuosly keep on getting new post-copy request. Is it possible 
> > > this
> > > case can starve post-copy request which is in precopy preemtion?
> > I didn't fully get your thoughts, could you elaborate?
> > 
> > Here we're checking against the case where the postcopy requested page is
> > exactly the one that we have preempted in previous precopy sessions.  If
> > true, we drop this postcopy page and continue with the rest.
> > 
> > When there'll be no postcopy requests pending then we'll continue with the
> > precopy page, which is exactly the request we've dropped.
> > 
> > Why we did this is actually in comment above postcopy_preempted_contains(),
> > and quotting from there:
> > 
> > /*
> >   * This should really happen very rarely, because it means when we were 
> > sending
> >   * during background migration for postcopy we're sending exactly the page 
> > that
> >   * some vcpu got faulted on on dest node.  When it happens, we probably 
> > don't
> >   * need to do much but drop the request, because we know right after we 
> > restore
> >   * the precopy stream it'll be serviced.  It'll slightly affect the order 
> > of
> >   * postcopy requests to be serviced (e.g. it'll be the same as we move 
> > current
> >   * request to the end of the queue) but it shouldn't be a big deal.  The 
> > most
> >   * imporant thing is we can _never_ try to send a partial-sent huge page 
> > on the
> >   * POSTCOPY channel again, otherwise that huge page will got "split brain" 
> > on
> >   * two channels (PRECOPY, POSTCOPY).
> >   */
> > 
> > [...]
> 
> Hi Peter, what i meant here is that as we go to precopy sending only when
> there is
> 
> no post-copy request left so if there is some workload which is continuosly
> generating
> 
> new post-copy fault request, It may take very long before we resume on
> precopy channel.
> 
> So basically precopy channel may have a post-copy request pending for very
> long in
> 
> this case? Earlier as it was FCFS there was a guarantee a post-copy request
> will be
> 
> served after a fixed amount of time.

Ah that's a good point.

In that case maybe what we want to do is to restore this preemption
immediately using postcopy_preempt_restore(), however we may also want to
make sure this huge page won't get preempted by any other postcopy pages.

One thing in my mind to do this is to add one more field to the pss
structure, we could call it pss->urgent.

Previously we have only had pss->postcopy_requested showing that whether
one request comes from postcopy and whether we should send this page via
the postcopy preempt channel.  We also use that as a hint so we will never
preempt a huge page when postcopy_requested is set.  Now we probably want
to separate that out of pss->urgent, so postcopy_requested will be mostly
like before (along with the new pss->urgent set to 1 for all postcopy
pages), except that we may want to also set pss->urgent to 1 for this very
special case even for precopy pages, so that we won't preempt this page as
well.

I'm thinking maybe it's not wise to directly change the patch when I
repost.  My current plan is I'll add one more patch at the end, so I won't
easily give away Dave's R-b meanwhile hopefully that could make reviewers
easy.  We could consider squashing that patch in if we'll commit the whole
thing, or we can even keep them separate as a further optimization.

> 
> > > > @@ -2211,7 +2406,16 @@ static int ram_save_host_page(RAMState *rs, 
> > > > PageSearchStatus *pss)
> > > >            return 0;
> > > >        }
> > > > +    if (migrate_postcopy_preempt() && migration_in_postcopy()) {
> > > I see why there is only one extra channel, multiFD is not supported for
> > > postcopy. Peter, Any particular reason for that.
> > We used one channel not because multifd is not enabled - if you read into
> > the series the channels are separately managed because they're servicing
> > different goals.  It's because I don't really know whether multiple
> > channels would be necessary, because postcopy requests should not be the
> > major channel that pages will be sent, kind of a fast-path.
> > 
> > One of the major goal of this series is to avoid interruptions made to
> > postcopy urgent pages due to sending of precopy pages.  One extra channel
> > already serviced it well, so I just stopped there as the initial version.
> > I actually raised that question myself too in the cover letter in the todo
> > section, I think we can always evaluate the possibility of that in the
> > future without major reworks (but we may need another parameter to specify
> > the num of threads just like multifd).
> 
> >because postcopy requests should not be the major channel that pages will
> be sent, kind of a fast-path.
> 
> Yes, agree Peter, but in worst case scenario it is possible we may have to
> transfer full memory of VM
> 
> by post-copy requests? So in that case we may require higher number of
> threads. But agree there can not be
> 
> be binding with number of mutliFD channels as multiFD uses 256KB buffer size
> but here we may have to 4k
> 
> in small page case so there can be big diff in throughput limits. Also
> smaller the buffer size much higher will
> 
> be cpu usage so it needs to be decided carefully.

Right, and I see your point here.

It's just non-trivial to both gain performance and low latency imho.  But
maybe you have a good point in that it also means with preemption mode on
and with an extremely busy VM we could have put multifd into a vain even if
we'll support both multifd+preempt in the future.

But anyway - let's think more of it and let's solve problems one by one.

The worst case is we'll have low bw for such migration but it still keeps
relatively good responsiveness on dest page faults for now.

Thanks,

-- 
Peter Xu




reply via email to

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