All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] migration: clear the memory region dirty bitmap when skipping free pages
@ 2021-07-15  7:53 Wei Wang
  2021-07-15  9:28 ` David Hildenbrand
  0 siblings, 1 reply; 5+ messages in thread
From: Wei Wang @ 2021-07-15  7:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: mst, david, dgilbert, peterx, quintela

When skipping free pages to send, their corresponding dirty bits in the
memory region dirty bitmap need to be cleared. Otherwise the skipped
pages will be sent in the next round after the migration thread syncs
dirty bits from the memory region dirty bitmap.

Cc: David Hildenbrand <david@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Reported-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 migration/ram.c | 72 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 54 insertions(+), 18 deletions(-)

v1->v2 changelog:
- move migration_clear_memory_region_dirty_bitmap under bitmap_mutex as
  we lack confidence to have it outside the lock for now.
- clean the unnecessary subproject commit.

diff --git a/migration/ram.c b/migration/ram.c
index b5fc454b2f..69e06b55ec 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -789,6 +789,51 @@ unsigned long migration_bitmap_find_dirty(RAMState *rs, RAMBlock *rb,
     return find_next_bit(bitmap, size, start);
 }
 
+static void migration_clear_memory_region_dirty_bitmap(RAMState *rs,
+                                                       RAMBlock *rb,
+                                                       unsigned long page)
+{
+    uint8_t shift;
+    hwaddr size, start;
+
+    if (!rb->clear_bmap || !clear_bmap_test_and_clear(rb, page)) {
+        return;
+    }
+
+    shift = rb->clear_bmap_shift;
+    /*
+     * CLEAR_BITMAP_SHIFT_MIN should always guarantee this... this
+     * can make things easier sometimes since then start address
+     * of the small chunk will always be 64 pages aligned so the
+     * bitmap will always be aligned to unsigned long. We should
+     * even be able to remove this restriction but I'm simply
+     * keeping it.
+     */
+    assert(shift >= 6);
+
+    size = 1ULL << (TARGET_PAGE_BITS + shift);
+    start = (((ram_addr_t)page) << TARGET_PAGE_BITS) & (-size);
+    trace_migration_bitmap_clear_dirty(rb->idstr, start, size, page);
+    memory_region_clear_dirty_bitmap(rb->mr, start, size);
+}
+
+static void
+migration_clear_memory_region_dirty_bitmap_range(RAMState *rs,
+                                                 RAMBlock *rb,
+                                                 unsigned long start,
+                                                 unsigned long npages)
+{
+    unsigned long page_to_clear, i, nchunks;
+    unsigned long chunk_pages = 1UL << rb->clear_bmap_shift;
+
+    nchunks = (start + npages) / chunk_pages - start / chunk_pages + 1;
+
+    for (i = 0; i < nchunks; i++) {
+        page_to_clear = start + i * chunk_pages;
+        migration_clear_memory_region_dirty_bitmap(rs, rb, page_to_clear);
+    }
+}
+
 static inline bool migration_bitmap_clear_dirty(RAMState *rs,
                                                 RAMBlock *rb,
                                                 unsigned long page)
@@ -803,26 +848,9 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
      * the page in the chunk we clear the remote dirty bitmap for all.
      * Clearing it earlier won't be a problem, but too late will.
      */
-    if (rb->clear_bmap && clear_bmap_test_and_clear(rb, page)) {
-        uint8_t shift = rb->clear_bmap_shift;
-        hwaddr size = 1ULL << (TARGET_PAGE_BITS + shift);
-        hwaddr start = (((ram_addr_t)page) << TARGET_PAGE_BITS) & (-size);
-
-        /*
-         * CLEAR_BITMAP_SHIFT_MIN should always guarantee this... this
-         * can make things easier sometimes since then start address
-         * of the small chunk will always be 64 pages aligned so the
-         * bitmap will always be aligned to unsigned long.  We should
-         * even be able to remove this restriction but I'm simply
-         * keeping it.
-         */
-        assert(shift >= 6);
-        trace_migration_bitmap_clear_dirty(rb->idstr, start, size, page);
-        memory_region_clear_dirty_bitmap(rb->mr, start, size);
-    }
+    migration_clear_memory_region_dirty_bitmap(rs, rb, page);
 
     ret = test_and_clear_bit(page, rb->bmap);
-
     if (ret) {
         rs->migration_dirty_pages--;
     }
@@ -2741,6 +2769,14 @@ void qemu_guest_free_page_hint(void *addr, size_t len)
         npages = used_len >> TARGET_PAGE_BITS;
 
         qemu_mutex_lock(&ram_state->bitmap_mutex);
+        /*
+         * The skipped free pages are equavelent to be sent from clear_bmap's
+         * perspective, so clear the bits from the memory region bitmap which
+         * are initially set. Otherwise those skipped pages will be sent in
+         * the next round after syncing from the memory region bitmap.
+         */
+        migration_clear_memory_region_dirty_bitmap_range(ram_state, block,
+                                                         start, npages);
         ram_state->migration_dirty_pages -=
                       bitmap_count_one_with_offset(block->bmap, start, npages);
         bitmap_clear(block->bmap, start, npages);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] migration: clear the memory region dirty bitmap when skipping free pages
  2021-07-15  7:53 [PATCH v2] migration: clear the memory region dirty bitmap when skipping free pages Wei Wang
@ 2021-07-15  9:28 ` David Hildenbrand
  2021-07-16  6:15   ` Wang, Wei W
  0 siblings, 1 reply; 5+ messages in thread
From: David Hildenbrand @ 2021-07-15  9:28 UTC (permalink / raw)
  To: Wei Wang, qemu-devel; +Cc: mst, dgilbert, peterx, quintela

On 15.07.21 09:53, Wei Wang wrote:
> When skipping free pages to send, their corresponding dirty bits in the
> memory region dirty bitmap need to be cleared. Otherwise the skipped
> pages will be sent in the next round after the migration thread syncs
> dirty bits from the memory region dirty bitmap.
> 
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Reported-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
>   migration/ram.c | 72 ++++++++++++++++++++++++++++++++++++-------------
>   1 file changed, 54 insertions(+), 18 deletions(-)
> 
> v1->v2 changelog:
> - move migration_clear_memory_region_dirty_bitmap under bitmap_mutex as
>    we lack confidence to have it outside the lock for now.
> - clean the unnecessary subproject commit.
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index b5fc454b2f..69e06b55ec 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -789,6 +789,51 @@ unsigned long migration_bitmap_find_dirty(RAMState *rs, RAMBlock *rb,
>       return find_next_bit(bitmap, size, start);
>   }
>   
> +static void migration_clear_memory_region_dirty_bitmap(RAMState *rs,
> +                                                       RAMBlock *rb,
> +                                                       unsigned long page)
> +{
> +    uint8_t shift;
> +    hwaddr size, start;
> +
> +    if (!rb->clear_bmap || !clear_bmap_test_and_clear(rb, page)) {
> +        return;
> +    }
> +
> +    shift = rb->clear_bmap_shift;

You could initialize this right at the beginning of the function without doing any harm.

> +    /*
> +     * CLEAR_BITMAP_SHIFT_MIN should always guarantee this... this
> +     * can make things easier sometimes since then start address
> +     * of the small chunk will always be 64 pages aligned so the
> +     * bitmap will always be aligned to unsigned long. We should
> +     * even be able to remove this restriction but I'm simply
> +     * keeping it.
> +     */
> +    assert(shift >= 6);
> +
> +    size = 1ULL << (TARGET_PAGE_BITS + shift);
> +    start = (((ram_addr_t)page) << TARGET_PAGE_BITS) & (-size);

these as well as.

> +    trace_migration_bitmap_clear_dirty(rb->idstr, start, size, page);
> +    memory_region_clear_dirty_bitmap(rb->mr, start, size);
> +}
> +
> +static void
> +migration_clear_memory_region_dirty_bitmap_range(RAMState *rs,
> +                                                 RAMBlock *rb,
> +                                                 unsigned long start,
> +                                                 unsigned long npages)
> +{
> +    unsigned long page_to_clear, i, nchunks;
> +    unsigned long chunk_pages = 1UL << rb->clear_bmap_shift;
> +
> +    nchunks = (start + npages) / chunk_pages - start / chunk_pages + 1;

Wouldn't you have to align the start and the end range up/down
to properly calculate the number of chunks?

The following might be better and a little easier to grasp:

unsigned long chunk_pages = 1ULL << rb->clear_bmap_shift;
unsigned long aligned_start = QEMU_ALIGN_DOWN(start, chunk_pages);
unsigned long aligned_end = QEMU_ALIGN_UP(start + npages, chunk_pages)

/*
  * Clear the clar_bmap of all covered chunks. It's sufficient to call it for
  * one page within a chunk.
  */
for (start = aligned_start, start != aligned_end, start += chunk_pages) {
     migration_clear_memory_region_dirty_bitmap(rs, rb, start);
}

> +
> +    for (i = 0; i < nchunks; i++) {
> +        page_to_clear = start + i * chunk_pages;
> +        migration_clear_memory_region_dirty_bitmap(rs, rb, page_to_clear);
> +    }
> +}
> +
>   static inline bool migration_bitmap_clear_dirty(RAMState *rs,
>                                                   RAMBlock *rb,
>                                                   unsigned long page)
> @@ -803,26 +848,9 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
>        * the page in the chunk we clear the remote dirty bitmap for all.
>        * Clearing it earlier won't be a problem, but too late will.
>        */
> -    if (rb->clear_bmap && clear_bmap_test_and_clear(rb, page)) {
> -        uint8_t shift = rb->clear_bmap_shift;
> -        hwaddr size = 1ULL << (TARGET_PAGE_BITS + shift);
> -        hwaddr start = (((ram_addr_t)page) << TARGET_PAGE_BITS) & (-size);
> -
> -        /*
> -         * CLEAR_BITMAP_SHIFT_MIN should always guarantee this... this
> -         * can make things easier sometimes since then start address
> -         * of the small chunk will always be 64 pages aligned so the
> -         * bitmap will always be aligned to unsigned long.  We should
> -         * even be able to remove this restriction but I'm simply
> -         * keeping it.
> -         */
> -        assert(shift >= 6);
> -        trace_migration_bitmap_clear_dirty(rb->idstr, start, size, page);
> -        memory_region_clear_dirty_bitmap(rb->mr, start, size);
> -    }
> +    migration_clear_memory_region_dirty_bitmap(rs, rb, page);
>   
>       ret = test_and_clear_bit(page, rb->bmap);
> -

unrelated change but ok for me.

>       if (ret) {
>           rs->migration_dirty_pages--;
>       }
> @@ -2741,6 +2769,14 @@ void qemu_guest_free_page_hint(void *addr, size_t len)
>           npages = used_len >> TARGET_PAGE_BITS;
>   
>           qemu_mutex_lock(&ram_state->bitmap_mutex);
> +        /*
> +         * The skipped free pages are equavelent to be sent from clear_bmap's

s/equavelent/equivalent/

> +         * perspective, so clear the bits from the memory region bitmap which
> +         * are initially set. Otherwise those skipped pages will be sent in
> +         * the next round after syncing from the memory region bitmap.
> +         */
> +        migration_clear_memory_region_dirty_bitmap_range(ram_state, block,
> +                                                         start, npages);
>           ram_state->migration_dirty_pages -=
>                         bitmap_count_one_with_offset(block->bmap, start, npages);
>           bitmap_clear(block->bmap, start, npages);
> 

Apart from that, lgtm.

(although I find the use of "start" to describe a PFN and not an
address very confusing, but it's already in the current code ...
start_pfn or just pfn as used in the kernel would be much clearer)

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v2] migration: clear the memory region dirty bitmap when skipping free pages
  2021-07-15  9:28 ` David Hildenbrand
@ 2021-07-16  6:15   ` Wang, Wei W
  2021-07-16  8:26     ` David Hildenbrand
  0 siblings, 1 reply; 5+ messages in thread
From: Wang, Wei W @ 2021-07-16  6:15 UTC (permalink / raw)
  To: David Hildenbrand, qemu-devel; +Cc: mst, dgilbert, peterx, quintela

On Thursday, July 15, 2021 5:29 PM, David Hildenbrand wrote:
> On 15.07.21 09:53, Wei Wang wrote:
> > When skipping free pages to send, their corresponding dirty bits in
> > the memory region dirty bitmap need to be cleared. Otherwise the
> > skipped pages will be sent in the next round after the migration
> > thread syncs dirty bits from the memory region dirty bitmap.
> >
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Peter Xu <peterx@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Reported-by: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> > ---
> >   migration/ram.c | 72
> ++++++++++++++++++++++++++++++++++++-------------
> >   1 file changed, 54 insertions(+), 18 deletions(-)
> >
> > v1->v2 changelog:
> > - move migration_clear_memory_region_dirty_bitmap under bitmap_mutex
> as
> >    we lack confidence to have it outside the lock for now.
> > - clean the unnecessary subproject commit.
> >
> > diff --git a/migration/ram.c b/migration/ram.c index
> > b5fc454b2f..69e06b55ec 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -789,6 +789,51 @@ unsigned long
> migration_bitmap_find_dirty(RAMState *rs, RAMBlock *rb,
> >       return find_next_bit(bitmap, size, start);
> >   }
> >
> > +static void migration_clear_memory_region_dirty_bitmap(RAMState *rs,
> > +
> RAMBlock *rb,
> > +
> unsigned long
> > +page) {
> > +    uint8_t shift;
> > +    hwaddr size, start;
> > +
> > +    if (!rb->clear_bmap || !clear_bmap_test_and_clear(rb, page)) {
> > +        return;
> > +    }
> > +
> > +    shift = rb->clear_bmap_shift;
> 
> You could initialize this right at the beginning of the function without doing any
> harm.
> 
> > +    /*
> > +     * CLEAR_BITMAP_SHIFT_MIN should always guarantee this... this
> > +     * can make things easier sometimes since then start address
> > +     * of the small chunk will always be 64 pages aligned so the
> > +     * bitmap will always be aligned to unsigned long. We should
> > +     * even be able to remove this restriction but I'm simply
> > +     * keeping it.
> > +     */
> > +    assert(shift >= 6);
> > +
> > +    size = 1ULL << (TARGET_PAGE_BITS + shift);
> > +    start = (((ram_addr_t)page) << TARGET_PAGE_BITS) & (-size);
> 
> these as well as.

Is there any coding style requirement for this?
My thought was that those operations could mostly be avoided if they don't pass the
above if condition (e.g. just once per 1GB chunk).

> 
> > +    trace_migration_bitmap_clear_dirty(rb->idstr, start, size, page);
> > +    memory_region_clear_dirty_bitmap(rb->mr, start, size); }
> > +
> > +static void
> > +migration_clear_memory_region_dirty_bitmap_range(RAMState *rs,
> > +                                                 RAMBlock *rb,
> > +                                                 unsigned long
> start,
> > +                                                 unsigned long
> > +npages) {
> > +    unsigned long page_to_clear, i, nchunks;
> > +    unsigned long chunk_pages = 1UL << rb->clear_bmap_shift;
> > +
> > +    nchunks = (start + npages) / chunk_pages - start / chunk_pages +
> > + 1;
> 
> Wouldn't you have to align the start and the end range up/down to properly
> calculate the number of chunks?

No, divide will round it to the integer (beginning of the chunk to clear).

> 
> The following might be better and a little easier to grasp:
> 
> unsigned long chunk_pages = 1ULL << rb->clear_bmap_shift; unsigned long
> aligned_start = QEMU_ALIGN_DOWN(start, chunk_pages); unsigned long
> aligned_end = QEMU_ALIGN_UP(start + npages, chunk_pages)
> 
> /*
>   * Clear the clar_bmap of all covered chunks. It's sufficient to call it for
>   * one page within a chunk.
>   */
> for (start = aligned_start, start != aligned_end, start += chunk_pages) {

What if "aligned_end == start + npages"?
i.e the above start + npages is aligned by itself without QEMU_ALIGN_UP().
For example, chunk size is 1GB, and start+npages=2GB, which is right at the beginning of [2GB,3GB) chunk.
Then aligned_end is also 2GB, but we need to clear the [2GB, 3GB) chunk, right?

Best,
Wei

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] migration: clear the memory region dirty bitmap when skipping free pages
  2021-07-16  6:15   ` Wang, Wei W
@ 2021-07-16  8:26     ` David Hildenbrand
  2021-07-19  5:18       ` Wang, Wei W
  0 siblings, 1 reply; 5+ messages in thread
From: David Hildenbrand @ 2021-07-16  8:26 UTC (permalink / raw)
  To: Wang, Wei W, qemu-devel; +Cc: mst, dgilbert, peterx, quintela

>>> +    /*
>>> +     * CLEAR_BITMAP_SHIFT_MIN should always guarantee this... this
>>> +     * can make things easier sometimes since then start address
>>> +     * of the small chunk will always be 64 pages aligned so the
>>> +     * bitmap will always be aligned to unsigned long. We should
>>> +     * even be able to remove this restriction but I'm simply
>>> +     * keeping it.
>>> +     */
>>> +    assert(shift >= 6);
>>> +
>>> +    size = 1ULL << (TARGET_PAGE_BITS + shift);
>>> +    start = (((ram_addr_t)page) << TARGET_PAGE_BITS) & (-size);
>>
>> these as well as.
> 
> Is there any coding style requirement for this?

Don't think so. It simply results in less LOC and less occurrences of 
variables.

> My thought was that those operations could mostly be avoided if they don't pass the
> above if condition (e.g. just once per 1GB chunk).

Usually the compiler will reshuffle as possible to optimize. But in this 
case, due to clear_bmap_test_and_clear(), it might not be able to move 
the computations behind that call. So the final code might actually differ.

Not that we really care about this micro-optimization, though.

> 
>>
>>> +    trace_migration_bitmap_clear_dirty(rb->idstr, start, size, page);
>>> +    memory_region_clear_dirty_bitmap(rb->mr, start, size); }
>>> +
>>> +static void
>>> +migration_clear_memory_region_dirty_bitmap_range(RAMState *rs,
>>> +                                                 RAMBlock *rb,
>>> +                                                 unsigned long
>> start,
>>> +                                                 unsigned long
>>> +npages) {
>>> +    unsigned long page_to_clear, i, nchunks;
>>> +    unsigned long chunk_pages = 1UL << rb->clear_bmap_shift;
>>> +
>>> +    nchunks = (start + npages) / chunk_pages - start / chunk_pages +
>>> + 1;
>>
>> Wouldn't you have to align the start and the end range up/down to properly
>> calculate the number of chunks?
> 
> No, divide will round it to the integer (beginning of the chunk to clear).


nchunks = (start + npages) / chunk_pages - start / chunk_pages + 1;

For simplicity:

nchunks = (addr + size) / chunk_size - addr / chunk_size + 1;

addr=1GB
size=3GB
chunk_size=2GB

So for that range [1GB, 3GB), we'd have to clear [0GB,2GB), [2GB,4GB)

Range:    [      ]
Chunks: [ - ][ - ][ - ][ - ] ...
         ^0   ^2   ^4   ^6

nchunks = (1 + 3) / 2 - 1 / 2 + 1
	= 4 / 2 - 0 + 1
	= 2 + 1
	= 3

Which is wrong.

While my variant will give you

aligned_start = 0GB
aligned_end = 4GB

And consequently clear [0GB,2GB) and [2GB,4GB).


Am I making a stupid mistake and should rather get another cup of coffee? :)


> 
>>
>> The following might be better and a little easier to grasp:
>>
>> unsigned long chunk_pages = 1ULL << rb->clear_bmap_shift; unsigned long
>> aligned_start = QEMU_ALIGN_DOWN(start, chunk_pages); unsigned long
>> aligned_end = QEMU_ALIGN_UP(start + npages, chunk_pages)
>>
>> /*
>>    * Clear the clar_bmap of all covered chunks. It's sufficient to call it for
>>    * one page within a chunk.
>>    */
>> for (start = aligned_start, start != aligned_end, start += chunk_pages) {
> 
> What if "aligned_end == start + npages"?
> i.e the above start + npages is aligned by itself without QEMU_ALIGN_UP().
> For example, chunk size is 1GB, and start+npages=2GB, which is right at the beginning of [2GB,3GB) chunk.
> Then aligned_end is also 2GB, but we need to clear the [2GB, 3GB) chunk, right?

Again, let's work with sizes instead of PFNs:

addr=1GB
size=1GB
chunk_size=1GB

Range:       [   ]
Chunks: [ - ][ - ][ - ][ - ] ...
         ^0   ^1   ^2   ^3

aligned_start = 1GB
aligned_end = 2GB

As you say, we'd clear the [1GB,2GB) chunk, but not the [2GB,3GB) chunk. 
But that's correct, as our range to hint is actually [start, 
start+npages) == [1GB,2GB).

> 
> Best,
> Wei
> 


-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v2] migration: clear the memory region dirty bitmap when skipping free pages
  2021-07-16  8:26     ` David Hildenbrand
@ 2021-07-19  5:18       ` Wang, Wei W
  0 siblings, 0 replies; 5+ messages in thread
From: Wang, Wei W @ 2021-07-19  5:18 UTC (permalink / raw)
  To: David Hildenbrand, qemu-devel; +Cc: mst, dgilbert, peterx, quintela

On Friday, July 16, 2021 4:26 PM, David Hildenbrand wrote:
> >>> +    /*
> >>> +     * CLEAR_BITMAP_SHIFT_MIN should always guarantee this... this
> >>> +     * can make things easier sometimes since then start address
> >>> +     * of the small chunk will always be 64 pages aligned so the
> >>> +     * bitmap will always be aligned to unsigned long. We should
> >>> +     * even be able to remove this restriction but I'm simply
> >>> +     * keeping it.
> >>> +     */
> >>> +    assert(shift >= 6);
> >>> +
> >>> +    size = 1ULL << (TARGET_PAGE_BITS + shift);
> >>> +    start = (((ram_addr_t)page) << TARGET_PAGE_BITS) & (-size);
> >>
> >> these as well as.
> >
> > Is there any coding style requirement for this?
> 
> Don't think so. It simply results in less LOC and less occurrences of variables.
> 
> > My thought was that those operations could mostly be avoided if they
> > don't pass the above if condition (e.g. just once per 1GB chunk).
> 
> Usually the compiler will reshuffle as possible to optimize. But in this case, due
> to clear_bmap_test_and_clear(), it might not be able to move the
> computations behind that call. So the final code might actually differ.
> 
> Not that we really care about this micro-optimization, though.

OK, looks that's just a personal favor. I'm inclined to keeping the micro-optimization.

> 
> >
> >>
> >>> +    trace_migration_bitmap_clear_dirty(rb->idstr, start, size, page);
> >>> +    memory_region_clear_dirty_bitmap(rb->mr, start, size); }
> >>> +
> >>> +static void
> >>> +migration_clear_memory_region_dirty_bitmap_range(RAMState *rs,
> >>> +                                                 RAMBlock
> *rb,
> >>> +                                                 unsigned
> long
> >> start,
> >>> +                                                 unsigned
> long
> >>> +npages) {
> >>> +    unsigned long page_to_clear, i, nchunks;
> >>> +    unsigned long chunk_pages = 1UL << rb->clear_bmap_shift;
> >>> +
> >>> +    nchunks = (start + npages) / chunk_pages - start / chunk_pages
> >>> + + 1;
> >>
> >> Wouldn't you have to align the start and the end range up/down to
> >> properly calculate the number of chunks?
> >
> > No, divide will round it to the integer (beginning of the chunk to clear).
> 
> 
> nchunks = (start + npages) / chunk_pages - start / chunk_pages + 1;

I had a mistake on the right boundary, it should be [start, start + npages), instead of [start, start + npages].
i.e. nchunks = (start + npages - 1) / chunk_pages - start / chunk_pages + 1

But I can take your approach here, thanks.

Best,
Wei


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-07-19  5:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15  7:53 [PATCH v2] migration: clear the memory region dirty bitmap when skipping free pages Wei Wang
2021-07-15  9:28 ` David Hildenbrand
2021-07-16  6:15   ` Wang, Wei W
2021-07-16  8:26     ` David Hildenbrand
2021-07-19  5:18       ` Wang, Wei W

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.