* [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently
@ 2017-03-15 3:37 Lidong Chen
2017-03-15 10:21 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2017-03-15 11:10 ` [Qemu-devel] " Fam Zheng
0 siblings, 2 replies; 6+ messages in thread
From: Lidong Chen @ 2017-03-15 3:37 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, famz, quintela, dgilbert, qemu-block, Lidong Chen
Increase bmds->cur_dirty after submit io, so reduce the frequency
involve into blk_drain, and improve the performance obviously
when block migration.
The performance test result of this patch:
During the block dirty save phase, this patch improve guest os IOPS
from 4.0K to 9.5K. and improve the migration speed from
505856 rsec/s to 855756 rsec/s.
Signed-off-by: Lidong Chen <jemmy858585@gmail.com>
---
migration/block.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/migration/block.c b/migration/block.c
index 6741228..7734ff7 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -576,6 +576,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
}
bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors);
+ sector += nr_sectors;
+ bmds->cur_dirty = sector;
+
break;
}
sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently
2017-03-15 3:37 [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently Lidong Chen
@ 2017-03-15 10:21 ` Kevin Wolf
2017-03-15 11:10 ` [Qemu-devel] " Fam Zheng
1 sibling, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2017-03-15 10:21 UTC (permalink / raw)
To: Lidong Chen; +Cc: qemu-devel, famz, qemu-block, quintela, dgilbert, stefanha
Am 15.03.2017 um 04:37 hat Lidong Chen geschrieben:
> Increase bmds->cur_dirty after submit io, so reduce the frequency
> involve into blk_drain, and improve the performance obviously
> when block migration.
>
> The performance test result of this patch:
>
> During the block dirty save phase, this patch improve guest os IOPS
> from 4.0K to 9.5K. and improve the migration speed from
> 505856 rsec/s to 855756 rsec/s.
>
> Signed-off-by: Lidong Chen <jemmy858585@gmail.com>
> ---
> migration/block.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/migration/block.c b/migration/block.c
> index 6741228..7734ff7 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -576,6 +576,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
> }
>
> bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors);
> + sector += nr_sectors;
sector += BDRV_SECTORS_PER_DIRTY_CHUNK would be acceptable here...
> + bmds->cur_dirty = sector;
> +
> break;
> }
> sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
> bmds->cur_dirty = sector;
> }
...which would make it the same code as the rest of the loop. It could
be nicer to reuse the same code and just set a bool when a request is
submitted so that the loop terminates after that.
But that's just a matter of taste, so I'll still give:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently
2017-03-15 3:37 [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently Lidong Chen
2017-03-15 10:21 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
@ 2017-03-15 11:10 ` Fam Zheng
2017-03-15 17:31 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 6+ messages in thread
From: Fam Zheng @ 2017-03-15 11:10 UTC (permalink / raw)
To: Lidong Chen; +Cc: qemu-devel, stefanha, quintela, dgilbert, qemu-block
On Wed, 03/15 11:37, Lidong Chen wrote:
> Increase bmds->cur_dirty after submit io, so reduce the frequency
> involve into blk_drain, and improve the performance obviously
> when block migration.
>
> The performance test result of this patch:
>
> During the block dirty save phase, this patch improve guest os IOPS
> from 4.0K to 9.5K. and improve the migration speed from
> 505856 rsec/s to 855756 rsec/s.
>
> Signed-off-by: Lidong Chen <jemmy858585@gmail.com>
> ---
> migration/block.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/migration/block.c b/migration/block.c
> index 6741228..7734ff7 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -576,6 +576,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
> }
>
> bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors);
> + sector += nr_sectors;
> + bmds->cur_dirty = sector;
> +
> break;
> }
> sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
> --
> 1.8.3.1
>
Nice catch above all, thank you!
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently
2017-03-15 11:10 ` [Qemu-devel] " Fam Zheng
@ 2017-03-15 17:31 ` Dr. David Alan Gilbert
2017-03-16 1:33 ` Fam Zheng
0 siblings, 1 reply; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2017-03-15 17:31 UTC (permalink / raw)
To: Fam Zheng; +Cc: Lidong Chen, qemu-devel, stefanha, quintela, qemu-block
* Fam Zheng (famz@redhat.com) wrote:
> On Wed, 03/15 11:37, Lidong Chen wrote:
> > Increase bmds->cur_dirty after submit io, so reduce the frequency
> > involve into blk_drain, and improve the performance obviously
> > when block migration.
> >
> > The performance test result of this patch:
> >
> > During the block dirty save phase, this patch improve guest os IOPS
> > from 4.0K to 9.5K. and improve the migration speed from
> > 505856 rsec/s to 855756 rsec/s.
> >
> > Signed-off-by: Lidong Chen <jemmy858585@gmail.com>
> > ---
> > migration/block.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/migration/block.c b/migration/block.c
> > index 6741228..7734ff7 100644
> > --- a/migration/block.c
> > +++ b/migration/block.c
> > @@ -576,6 +576,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
> > }
> >
> > bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors);
> > + sector += nr_sectors;
> > + bmds->cur_dirty = sector;
> > +
> > break;
> > }
> > sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
> > --
> > 1.8.3.1
> >
>
> Nice catch above all, thank you!
>
> Reviewed-by: Fam Zheng <famz@redhat.com>
Are you taking that via a block pull?
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently
2017-03-15 17:31 ` Dr. David Alan Gilbert
@ 2017-03-16 1:33 ` Fam Zheng
2017-03-16 7:59 ` Juan Quintela
0 siblings, 1 reply; 6+ messages in thread
From: Fam Zheng @ 2017-03-16 1:33 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Lidong Chen, qemu-devel, stefanha, quintela, qemu-block
On Wed, 03/15 17:31, Dr. David Alan Gilbert wrote:
> * Fam Zheng (famz@redhat.com) wrote:
> > On Wed, 03/15 11:37, Lidong Chen wrote:
> > > Increase bmds->cur_dirty after submit io, so reduce the frequency
> > > involve into blk_drain, and improve the performance obviously
> > > when block migration.
> > >
> > > The performance test result of this patch:
> > >
> > > During the block dirty save phase, this patch improve guest os IOPS
> > > from 4.0K to 9.5K. and improve the migration speed from
> > > 505856 rsec/s to 855756 rsec/s.
> > >
> > > Signed-off-by: Lidong Chen <jemmy858585@gmail.com>
> > > ---
> > > migration/block.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/migration/block.c b/migration/block.c
> > > index 6741228..7734ff7 100644
> > > --- a/migration/block.c
> > > +++ b/migration/block.c
> > > @@ -576,6 +576,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
> > > }
> > >
> > > bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors);
> > > + sector += nr_sectors;
> > > + bmds->cur_dirty = sector;
> > > +
> > > break;
> > > }
> > > sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
> > > --
> > > 1.8.3.1
> > >
> >
> > Nice catch above all, thank you!
> >
> > Reviewed-by: Fam Zheng <famz@redhat.com>
>
> Are you taking that via a block pull?
I can do that, but I'm not sure whether it should go to 2.9. This is a
performance improvement, which usually doesn't qualify as bug fixes. But this
also looks like a mistake in original code.
Fam
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently
2017-03-16 1:33 ` Fam Zheng
@ 2017-03-16 7:59 ` Juan Quintela
0 siblings, 0 replies; 6+ messages in thread
From: Juan Quintela @ 2017-03-16 7:59 UTC (permalink / raw)
To: Fam Zheng
Cc: Dr. David Alan Gilbert, Lidong Chen, qemu-devel, stefanha,
qemu-block
Fam Zheng <famz@redhat.com> wrote:
> On Wed, 03/15 17:31, Dr. David Alan Gilbert wrote:
>> * Fam Zheng (famz@redhat.com) wrote:
>> > On Wed, 03/15 11:37, Lidong Chen wrote:
>> > > Increase bmds->cur_dirty after submit io, so reduce the frequency
>> > > involve into blk_drain, and improve the performance obviously
>> > > when block migration.
>> > >
>> > > The performance test result of this patch:
>> > >
>> > > During the block dirty save phase, this patch improve guest os IOPS
>> > > from 4.0K to 9.5K. and improve the migration speed from
>> > > 505856 rsec/s to 855756 rsec/s.
>> > >
>> > > Signed-off-by: Lidong Chen <jemmy858585@gmail.com>
>> > > ---
>> > > migration/block.c | 3 +++
>> > > 1 file changed, 3 insertions(+)
>> > >
>> > > diff --git a/migration/block.c b/migration/block.c
>> > > index 6741228..7734ff7 100644
>> > > --- a/migration/block.c
>> > > +++ b/migration/block.c
>> > > @@ -576,6 +576,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
>> > > }
>> > >
>> > > bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors);
>> > > + sector += nr_sectors;
>> > > + bmds->cur_dirty = sector;
>> > > +
>> > > break;
>> > > }
>> > > sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
>> > > --
>> > > 1.8.3.1
>> > >
>> >
>> > Nice catch above all, thank you!
>> >
>> > Reviewed-by: Fam Zheng <famz@redhat.com>
>>
>> Are you taking that via a block pull?
>
> I can do that, but I'm not sure whether it should go to 2.9. This is a
> performance improvement, which usually doesn't qualify as bug fixes. But this
> also looks like a mistake in original code.
>
> Fam
I am taking it through migration and push it. I agree with your description.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-16 7:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-15 3:37 [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently Lidong Chen
2017-03-15 10:21 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2017-03-15 11:10 ` [Qemu-devel] " Fam Zheng
2017-03-15 17:31 ` Dr. David Alan Gilbert
2017-03-16 1:33 ` Fam Zheng
2017-03-16 7:59 ` Juan Quintela
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).