| Message ID | 20180105170138.23357-3-dgilbert@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | find_ram_offset cleanups and alignment | expand |
On 01/05/2018 11:01 AM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The dirty bitmaps are built from 'long'sand there is fast-path code s/'long'sand/'long's, and/ > for synchronising the case where the RAMBlock is aligned to the start > of a long boundary. Align the allocation to this boundary > to cause the fast path to be used. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > exec.c | 5 +++++ > 1 file changed, 5 insertions(+) Reviewed-by: Eric Blake <eblake@redhat.com> > > diff --git a/exec.c b/exec.c > index 7966570231..644603f05e 100644 > --- a/exec.c > +++ b/exec.c > @@ -1694,6 +1694,11 @@ static ram_addr_t find_ram_offset(ram_addr_t size) > } > } > > + /* Align blocks to start on a 'long' in the bitmap > + * which makes the bitmap sync'ing take the fast path. > + */ > + end = ROUND_UP(end, BITS_PER_LONG << TARGET_PAGE_BITS); > + > /* If it fits remember our place and remember the size > * of gap, but keep going so that we might find a smaller > * gap to fill so avoiding fragmentation. >
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The dirty bitmaps are built from 'long'sand there is fast-path code > for synchronising the case where the RAMBlock is aligned to the start > of a long boundary. Align the allocation to this boundary > to cause the fast path to be used. > > Offsets before change: > 11398@1515169675.018566:find_ram_offset size: 0x1e0000 @ 0x8000000 > 11398@1515169675.020064:find_ram_offset size: 0x20000 @ 0x81e0000 > 11398@1515169675.020244:find_ram_offset size: 0x20000 @ 0x8200000 > 11398@1515169675.024343:find_ram_offset size: 0x1000000 @ 0x8220000 > 11398@1515169675.025154:find_ram_offset size: 0x10000 @ 0x9220000 > 11398@1515169675.027682:find_ram_offset size: 0x40000 @ 0x9230000 > 11398@1515169675.032921:find_ram_offset size: 0x200000 @ 0x9270000 > 11398@1515169675.033307:find_ram_offset size: 0x1000 @ 0x9470000 > 11398@1515169675.033601:find_ram_offset size: 0x1000 @ 0x9471000 > > after change: > 10923@1515169108.818245:find_ram_offset size: 0x1e0000 @ 0x8000000 > 10923@1515169108.819410:find_ram_offset size: 0x20000 @ 0x8200000 > 10923@1515169108.819587:find_ram_offset size: 0x20000 @ 0x8240000 > 10923@1515169108.823708:find_ram_offset size: 0x1000000 @ 0x8280000 > 10923@1515169108.824503:find_ram_offset size: 0x10000 @ 0x9280000 > 10923@1515169108.827093:find_ram_offset size: 0x40000 @ 0x92c0000 > 10923@1515169108.833045:find_ram_offset size: 0x200000 @ 0x9300000 > 10923@1515169108.833504:find_ram_offset size: 0x1000 @ 0x9500000 > 10923@1515169108.833787:find_ram_offset size: 0x1000 @ 0x9540000 > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>
On 05/01/2018 18:01, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The dirty bitmaps are built from 'long'sand there is fast-path code > for synchronising the case where the RAMBlock is aligned to the start > of a long boundary. Align the allocation to this boundary > to cause the fast path to be used. > > Offsets before change: > 11398@1515169675.018566:find_ram_offset size: 0x1e0000 @ 0x8000000 > 11398@1515169675.020064:find_ram_offset size: 0x20000 @ 0x81e0000 > 11398@1515169675.020244:find_ram_offset size: 0x20000 @ 0x8200000 > 11398@1515169675.024343:find_ram_offset size: 0x1000000 @ 0x8220000 > 11398@1515169675.025154:find_ram_offset size: 0x10000 @ 0x9220000 > 11398@1515169675.027682:find_ram_offset size: 0x40000 @ 0x9230000 > 11398@1515169675.032921:find_ram_offset size: 0x200000 @ 0x9270000 > 11398@1515169675.033307:find_ram_offset size: 0x1000 @ 0x9470000 > 11398@1515169675.033601:find_ram_offset size: 0x1000 @ 0x9471000 > > after change: > 10923@1515169108.818245:find_ram_offset size: 0x1e0000 @ 0x8000000 > 10923@1515169108.819410:find_ram_offset size: 0x20000 @ 0x8200000 > 10923@1515169108.819587:find_ram_offset size: 0x20000 @ 0x8240000 > 10923@1515169108.823708:find_ram_offset size: 0x1000000 @ 0x8280000 > 10923@1515169108.824503:find_ram_offset size: 0x10000 @ 0x9280000 > 10923@1515169108.827093:find_ram_offset size: 0x40000 @ 0x92c0000 > 10923@1515169108.833045:find_ram_offset size: 0x200000 @ 0x9300000 > 10923@1515169108.833504:find_ram_offset size: 0x1000 @ 0x9500000 > 10923@1515169108.833787:find_ram_offset size: 0x1000 @ 0x9540000 > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > exec.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/exec.c b/exec.c > index 7966570231..644603f05e 100644 > --- a/exec.c > +++ b/exec.c > @@ -1694,6 +1694,11 @@ static ram_addr_t find_ram_offset(ram_addr_t size) > } > } > > + /* Align blocks to start on a 'long' in the bitmap > + * which makes the bitmap sync'ing take the fast path. > + */ > + end = ROUND_UP(end, BITS_PER_LONG << TARGET_PAGE_BITS); I think this should be done before the nested loop. Otherwise, "next - end" can become negative, which is always going to be >= size. It's also very large, so it's likely to fail the mingap test, but it's buggy nevertheless. In fact "end" could be renamed to "candidate" which explains more clearly what's going on. I'll post a v2 shortly... Thanks, Paolo > /* If it fits remember our place and remember the size > * of gap, but keep going so that we might find a smaller > * gap to fill so avoiding fragmentation. >
diff --git a/exec.c b/exec.c index 7966570231..644603f05e 100644 --- a/exec.c +++ b/exec.c @@ -1694,6 +1694,11 @@ static ram_addr_t find_ram_offset(ram_addr_t size) } } + /* Align blocks to start on a 'long' in the bitmap + * which makes the bitmap sync'ing take the fast path. + */ + end = ROUND_UP(end, BITS_PER_LONG << TARGET_PAGE_BITS); + /* If it fits remember our place and remember the size * of gap, but keep going so that we might find a smaller * gap to fill so avoiding fragmentation.