[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 1/2] migration/ram: Reduce unnecessary rate limiting
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH v4 1/2] migration/ram: Reduce unnecessary rate limiting |
Date: |
Wed, 17 Mar 2021 12:05:27 +0000 |
User-agent: |
Mutt/2.0.5 (2021-01-21) |
* Kunkun Jiang (jiangkunkun@huawei.com) wrote:
> Hi Peter,
>
> On 2021/3/17 5:39, Peter Xu wrote:
> > On Tue, Mar 16, 2021 at 08:57:15PM +0800, Kunkun Jiang wrote:
> > > When the host page is a huge page and something is sent in the
> > > current iteration, migration_rate_limit() should be executed.
> > > If not, it can be omitted.
> > >
> > > Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> > > Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
> > > Reviewed-by: David Edmondson <david.edmondson@oracle.com>
> > > ---
> > > migration/ram.c | 9 +++++++--
> > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/migration/ram.c b/migration/ram.c
> > > index 72143da0ac..3eb5b0d7a7 100644
> > > --- a/migration/ram.c
> > > +++ b/migration/ram.c
> > > @@ -2015,8 +2015,13 @@ static int ram_save_host_page(RAMState *rs,
> > > PageSearchStatus *pss,
> > > pages += tmppages;
> > > pss->page++;
> > > - /* Allow rate limiting to happen in the middle of huge pages */
> > > - migration_rate_limit();
> > > + /*
> > > + * Allow rate limiting to happen in the middle of huge pages if
> > > + * something is sent in the current iteration.
> > > + */
> > > + if (pagesize_bits > 1 && tmppages > 0) {
> > > + migration_rate_limit();
> > > + }
> > Sorry I'm still not a fan of this - I'd even prefer calling that once more
> > just
> > to make sure it won't be forgotten to be called.. Not to say it's merely a
> > noop.
> >
> > I'll leave this to Dave.. Maybe I'm too harsh! :)
> >
> You are very serious and meticulous. I like your character very much.😉
> This patch was used to reviewed by David. So, I want to know what
> his opinion is.
>
> @David
> Hi David, what is your opinion on this patch?
Yes, I think this is OK;
a) The qemu_file_rate_limit in the loop in ram_save_iterate still
gets called, so that covers the 'pagesize_bits > 1' part of the if
b) As soon as we send any part of the hugepage the 'tmppages > 0'
triggers and we get the check back again.
So I guess this mostly helps the case where we have big huge pages
which are mostly not-dirty, and we spend a lot of time in
ram_save_host_page searching for the target page to send.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Thanks,
> Kunkun Jiang
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK