* [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass
@ 2019-08-06 0:46 Wei Yang
2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Wei Yang @ 2019-08-06 0:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
When looking into function postcopy_chunk_hostpages_pass(), we could use
alignment calculation to simplify it.
Wei Yang (2):
migration/postcopy: simplify calculation of run_start and
fixup_start_addr
migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset
migration/ram.c | 37 +++++++------------------------------
1 file changed, 7 insertions(+), 30 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr
2019-08-06 0:46 [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Wei Yang
@ 2019-08-06 0:46 ` Wei Yang
2019-08-06 14:51 ` Dr. David Alan Gilbert
2019-08-06 0:46 ` [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset Wei Yang
2019-08-07 18:43 ` [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Dr. David Alan Gilbert
2 siblings, 1 reply; 7+ messages in thread
From: Wei Yang @ 2019-08-06 0:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
The purpose of the calculation is to find a HostPage which is partially
dirty.
* fixup_start_addr points to the start of the HostPage to discard
* run_start points to the next HostPage to check
While in the middle stage, there would two cases for run_start:
* aligned with HostPage means this is not partially dirty
* not aligned means this is partially dirty
When it is aligned, no work and calculation is necessary. run_start
already points to the start of next HostPage and is ready to continue.
When it is not aligned, the calculation could be simplified with:
* fixup_start_addr = QEMU_ALIGN_DOWN(run_start, host_ratio)
* run_start = QEMU_ALIGN_UP(run_start, host_ratio)
By doing so, run_start always points to the next HostPage to check.
fixup_start_addr always points to the HostPage to discard.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 34 +++++++---------------------------
1 file changed, 7 insertions(+), 27 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index c9585487ac..d86661a015 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2956,7 +2956,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
}
while (run_start < pages) {
- unsigned long fixup_start_addr;
unsigned long host_offset;
/*
@@ -2964,45 +2963,26 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
* page, then we need to fixup this host page.
*/
host_offset = run_start % host_ratio;
- if (host_offset) {
- fixup_start_addr = run_start - host_offset;
- /*
- * This host page has gone, the next loop iteration starts
- * from after the fixup
- */
- run_start = fixup_start_addr + host_ratio;
- } else {
+ if (!host_offset) {
/* Find the end of this run */
- unsigned long run_end;
if (unsent_pass) {
- run_end = find_next_bit(unsentmap, pages, run_start + 1);
+ run_start = find_next_bit(unsentmap, pages, run_start + 1);
} else {
- run_end = find_next_zero_bit(bitmap, pages, run_start + 1);
+ run_start = find_next_zero_bit(bitmap, pages, run_start + 1);
}
/*
* If the end isn't at the start of a host page, then the
* run doesn't finish at the end of a host page
* and we need to discard.
*/
- host_offset = run_end % host_ratio;
- if (host_offset) {
- fixup_start_addr = run_end - host_offset;
- /*
- * This host page has gone, the next loop iteration starts
- * from after the fixup
- */
- run_start = fixup_start_addr + host_ratio;
- } else {
- /*
- * No discards on this iteration, next loop starts from
- * next sent/dirty page
- */
- run_start = run_end + 1;
- }
+ host_offset = run_start % host_ratio;
}
if (host_offset) {
unsigned long page;
+ unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start,
+ host_ratio);
+ run_start = QEMU_ALIGN_UP(run_start, host_ratio);
/* Tell the destination to discard this page */
if (unsent_pass || !test_bit(fixup_start_addr, unsentmap)) {
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset
2019-08-06 0:46 [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Wei Yang
2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang
@ 2019-08-06 0:46 ` Wei Yang
2019-08-06 15:47 ` Dr. David Alan Gilbert
2019-08-07 18:43 ` [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Dr. David Alan Gilbert
2 siblings, 1 reply; 7+ messages in thread
From: Wei Yang @ 2019-08-06 0:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
Use QEMU_IS_ALIGNED for the check, it would be more consistent with
other align calculations.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index d86661a015..dfbf71c580 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2956,14 +2956,12 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
}
while (run_start < pages) {
- unsigned long host_offset;
/*
* If the start of this run of pages is in the middle of a host
* page, then we need to fixup this host page.
*/
- host_offset = run_start % host_ratio;
- if (!host_offset) {
+ if (QEMU_IS_ALIGNED(run_start, host_ratio)) {
/* Find the end of this run */
if (unsent_pass) {
run_start = find_next_bit(unsentmap, pages, run_start + 1);
@@ -2975,10 +2973,9 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
* run doesn't finish at the end of a host page
* and we need to discard.
*/
- host_offset = run_start % host_ratio;
}
- if (host_offset) {
+ if (!QEMU_IS_ALIGNED(run_start, host_ratio)) {
unsigned long page;
unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start,
host_ratio);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr
2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang
@ 2019-08-06 14:51 ` Dr. David Alan Gilbert
2019-08-07 0:14 ` Wei Yang
0 siblings, 1 reply; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-06 14:51 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, quintela
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> The purpose of the calculation is to find a HostPage which is partially
> dirty.
>
> * fixup_start_addr points to the start of the HostPage to discard
> * run_start points to the next HostPage to check
>
> While in the middle stage, there would two cases for run_start:
>
> * aligned with HostPage means this is not partially dirty
> * not aligned means this is partially dirty
>
> When it is aligned, no work and calculation is necessary. run_start
> already points to the start of next HostPage and is ready to continue.
>
> When it is not aligned, the calculation could be simplified with:
>
> * fixup_start_addr = QEMU_ALIGN_DOWN(run_start, host_ratio)
> * run_start = QEMU_ALIGN_UP(run_start, host_ratio)
>
> By doing so, run_start always points to the next HostPage to check.
> fixup_start_addr always points to the HostPage to discard.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Yes that's a lot shorter; note it doess it on top of your cleanup
from a couple of weeks back.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/ram.c | 34 +++++++---------------------------
> 1 file changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index c9585487ac..d86661a015 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2956,7 +2956,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
> }
>
> while (run_start < pages) {
> - unsigned long fixup_start_addr;
> unsigned long host_offset;
>
> /*
> @@ -2964,45 +2963,26 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
> * page, then we need to fixup this host page.
> */
> host_offset = run_start % host_ratio;
> - if (host_offset) {
> - fixup_start_addr = run_start - host_offset;
> - /*
> - * This host page has gone, the next loop iteration starts
> - * from after the fixup
> - */
> - run_start = fixup_start_addr + host_ratio;
> - } else {
> + if (!host_offset) {
> /* Find the end of this run */
> - unsigned long run_end;
> if (unsent_pass) {
> - run_end = find_next_bit(unsentmap, pages, run_start + 1);
> + run_start = find_next_bit(unsentmap, pages, run_start + 1);
> } else {
> - run_end = find_next_zero_bit(bitmap, pages, run_start + 1);
> + run_start = find_next_zero_bit(bitmap, pages, run_start + 1);
> }
> /*
> * If the end isn't at the start of a host page, then the
> * run doesn't finish at the end of a host page
> * and we need to discard.
> */
> - host_offset = run_end % host_ratio;
> - if (host_offset) {
> - fixup_start_addr = run_end - host_offset;
> - /*
> - * This host page has gone, the next loop iteration starts
> - * from after the fixup
> - */
> - run_start = fixup_start_addr + host_ratio;
> - } else {
> - /*
> - * No discards on this iteration, next loop starts from
> - * next sent/dirty page
> - */
> - run_start = run_end + 1;
> - }
> + host_offset = run_start % host_ratio;
> }
>
> if (host_offset) {
> unsigned long page;
> + unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start,
> + host_ratio);
> + run_start = QEMU_ALIGN_UP(run_start, host_ratio);
>
> /* Tell the destination to discard this page */
> if (unsent_pass || !test_bit(fixup_start_addr, unsentmap)) {
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset
2019-08-06 0:46 ` [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset Wei Yang
@ 2019-08-06 15:47 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-06 15:47 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, quintela
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> Use QEMU_IS_ALIGNED for the check, it would be more consistent with
> other align calculations.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/ram.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index d86661a015..dfbf71c580 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2956,14 +2956,12 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
> }
>
> while (run_start < pages) {
> - unsigned long host_offset;
>
> /*
> * If the start of this run of pages is in the middle of a host
> * page, then we need to fixup this host page.
> */
> - host_offset = run_start % host_ratio;
> - if (!host_offset) {
> + if (QEMU_IS_ALIGNED(run_start, host_ratio)) {
> /* Find the end of this run */
> if (unsent_pass) {
> run_start = find_next_bit(unsentmap, pages, run_start + 1);
> @@ -2975,10 +2973,9 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
> * run doesn't finish at the end of a host page
> * and we need to discard.
> */
> - host_offset = run_start % host_ratio;
> }
>
> - if (host_offset) {
> + if (!QEMU_IS_ALIGNED(run_start, host_ratio)) {
> unsigned long page;
> unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start,
> host_ratio);
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr
2019-08-06 14:51 ` Dr. David Alan Gilbert
@ 2019-08-07 0:14 ` Wei Yang
0 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2019-08-07 0:14 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: quintela, Wei Yang, qemu-devel
On Tue, Aug 06, 2019 at 03:51:17PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> The purpose of the calculation is to find a HostPage which is partially
>> dirty.
>>
>> * fixup_start_addr points to the start of the HostPage to discard
>> * run_start points to the next HostPage to check
>>
>> While in the middle stage, there would two cases for run_start:
>>
>> * aligned with HostPage means this is not partially dirty
>> * not aligned means this is partially dirty
>>
>> When it is aligned, no work and calculation is necessary. run_start
>> already points to the start of next HostPage and is ready to continue.
>>
>> When it is not aligned, the calculation could be simplified with:
>>
>> * fixup_start_addr = QEMU_ALIGN_DOWN(run_start, host_ratio)
>> * run_start = QEMU_ALIGN_UP(run_start, host_ratio)
>>
>> By doing so, run_start always points to the next HostPage to check.
>> fixup_start_addr always points to the HostPage to discard.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>Yes that's a lot shorter; note it doess it on top of your cleanup
>from a couple of weeks back.
>
You are right, I forget to mention it :-)
>Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>
>> ---
>> migration/ram.c | 34 +++++++---------------------------
>> 1 file changed, 7 insertions(+), 27 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index c9585487ac..d86661a015 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -2956,7 +2956,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
>> }
>>
>> while (run_start < pages) {
>> - unsigned long fixup_start_addr;
>> unsigned long host_offset;
>>
>> /*
>> @@ -2964,45 +2963,26 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
>> * page, then we need to fixup this host page.
>> */
>> host_offset = run_start % host_ratio;
>> - if (host_offset) {
>> - fixup_start_addr = run_start - host_offset;
>> - /*
>> - * This host page has gone, the next loop iteration starts
>> - * from after the fixup
>> - */
>> - run_start = fixup_start_addr + host_ratio;
>> - } else {
>> + if (!host_offset) {
>> /* Find the end of this run */
>> - unsigned long run_end;
>> if (unsent_pass) {
>> - run_end = find_next_bit(unsentmap, pages, run_start + 1);
>> + run_start = find_next_bit(unsentmap, pages, run_start + 1);
>> } else {
>> - run_end = find_next_zero_bit(bitmap, pages, run_start + 1);
>> + run_start = find_next_zero_bit(bitmap, pages, run_start + 1);
>> }
>> /*
>> * If the end isn't at the start of a host page, then the
>> * run doesn't finish at the end of a host page
>> * and we need to discard.
>> */
>> - host_offset = run_end % host_ratio;
>> - if (host_offset) {
>> - fixup_start_addr = run_end - host_offset;
>> - /*
>> - * This host page has gone, the next loop iteration starts
>> - * from after the fixup
>> - */
>> - run_start = fixup_start_addr + host_ratio;
>> - } else {
>> - /*
>> - * No discards on this iteration, next loop starts from
>> - * next sent/dirty page
>> - */
>> - run_start = run_end + 1;
>> - }
>> + host_offset = run_start % host_ratio;
>> }
>>
>> if (host_offset) {
>> unsigned long page;
>> + unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start,
>> + host_ratio);
>> + run_start = QEMU_ALIGN_UP(run_start, host_ratio);
>>
>> /* Tell the destination to discard this page */
>> if (unsent_pass || !test_bit(fixup_start_addr, unsentmap)) {
>> --
>> 2.17.1
>>
>--
>Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass
2019-08-06 0:46 [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Wei Yang
2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang
2019-08-06 0:46 ` [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset Wei Yang
@ 2019-08-07 18:43 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-07 18:43 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, quintela
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> When looking into function postcopy_chunk_hostpages_pass(), we could use
> alignment calculation to simplify it.
>
> Wei Yang (2):
> migration/postcopy: simplify calculation of run_start and
> fixup_start_addr
> migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset
>
> migration/ram.c | 37 +++++++------------------------------
> 1 file changed, 7 insertions(+), 30 deletions(-)
Queued
> --
> 2.17.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-07 18:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06 0:46 [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Wei Yang
2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang
2019-08-06 14:51 ` Dr. David Alan Gilbert
2019-08-07 0:14 ` Wei Yang
2019-08-06 0:46 ` [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset Wei Yang
2019-08-06 15:47 ` Dr. David Alan Gilbert
2019-08-07 18:43 ` [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.