find_ram_offset cleanups and alignment
cover find_ram_offset cleanups and alignment
From: "Dr. David Alan Gilbert" <[email protected]>
Hi,
This is mainly the alignment change that Paolo asked for
in the 2nd commit, but while I was there I added some comments and tracing that I've split out.
Dave
Dr. David Alan Gilbert (2):
find_ram_offset: Add comments and tracing
find_ram_offset: Align ram_addr_t allocation on long boundaries
exec.c | 22 +++++++++++++++++++++- trace-events | 3 +++ 2 files changed, 24 insertions(+), 1 deletion(-)
Signature · 1 line
2.14.3
PATCH 1/2 find_ram_offset: Add comments and tracing
From: "Dr. David Alan Gilbert" <[email protected]>
Add some comments so I can understand the various nested loops. Add some tracing so I can see what they're doing.
--- exec.c | 17 ++++++++++++++++- trace-events | 3 +++ 2 files changed, 19 insertions(+), 1 deletion(-)
exec.c +16 -1
@@ -1665,7 +1665,10 @@ static void *file_ram_alloc(RAMBlock *block, }#endif -/* Called with the ramlist lock held. */ +/* Allocate space within the ram_addr_t space that governs the +* dirty bitmaps. +* Called with the ramlist lock held. +*/ static ram_addr_t find_ram_offset (ram_addr_t size) {RAMBlock *block, *next_block;
@@ -1682,15 +1685,25 @@ static ram_addr_t find_ram_offset(ram_addr_t size) end = block->offset + block->max_length; +/* Search for the closest following block +* and find the gap. +*/ RAMBLOCK_FOREACH (next_block) {if (next_block->offset >= end) { next =MIN (next, next_block->offset); } } + +/* 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. +*/ if (next - end >= size && next - end < mingap) { offset = end; mingap = next - end; } + +trace_find_ram_offset_loop (size, offset, end, next, mingap); }if (offset ==RAM_ADDR_MAX ) {
@@ -1699,6 +1712,8 @@ static ram_addr_t find_ram_offset(ram_addr_t size)abort (); } +trace_find_ram_offset (size, offset); +return offset; }
trace-events +3 -0
@@ -56,6 +56,9 @@ dma_blk_cb(void *dbs, int ret) "dbs=%p ret=%d" dma_map_wait(void *dbs) "dbs=%p" # exec.c +find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%"PRIx64 " @ 0x%"PRIx64 +find_ram_offset_loop(uint64_t size, uint64_t offset, uint64_t end, uint64_t next, uint64_t mingap) "size: 0x%"PRIx64 " @ 0x%"PRIx64 " end: 0x%"PRIx64" next: 0x%"PRIx64 " mingap: 0x%"PRIx64 + ram_block_discard_range(const char *rbname, void *hva, bool need_madvise, bool need_fallocate, int ret) "%s@%p: madvise: %d fallocate: %d ret: %d" # memory.c
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH 1/2] find_ram_offset: Add comments and tracing
Quoted 12 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Add some comments so I can understand the various nested loops. Add some tracing so I can see what they're doing.
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- exec.c | 17 ++++++++++++++++- trace-events | 3 +++ 2 files changed, 19 insertions(+), 1 deletion(-)
exec.c static ram_addr_t find_ram_offset(ram_addr_t size) · line 1699
Quoted 4 lines
exec.c +4
-0+ +/* 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.
s/so avoiding/in order to avoid/
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 1/2] find_ram_offset: Add comments and tracing
In reply to Dr. David Alan Gilbert (git)Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Add some comments so I can understand the various nested loops. Add some tracing so I can see what they're doing.
Signed-off-by Dr. David Alan Gilbert <[email protected]>
PATCH 2/2 find_ram_offset: Align ram_addr_t allocation on long boundaries
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
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:
[email protected]:find_ram_offset size: 0x1e0000 @ 0x8000000 [email protected]:find_ram_offset size: 0x20000 @ 0x81e0000 [email protected]:find_ram_offset size: 0x20000 @ 0x8200000 [email protected]:find_ram_offset size: 0x1000000 @ 0x8220000 [email protected]:find_ram_offset size: 0x10000 @ 0x9220000 [email protected]:find_ram_offset size: 0x40000 @ 0x9230000 [email protected]:find_ram_offset size: 0x200000 @ 0x9270000 [email protected]:find_ram_offset size: 0x1000 @ 0x9470000 [email protected]:find_ram_offset size: 0x1000 @ 0x9471000
after change:
[email protected]:find_ram_offset size: 0x1e0000 @ 0x8000000 [email protected]:find_ram_offset size: 0x20000 @ 0x8200000 [email protected]:find_ram_offset size: 0x20000 @ 0x8240000 [email protected]:find_ram_offset size: 0x1000000 @ 0x8280000 [email protected]:find_ram_offset size: 0x10000 @ 0x9280000 [email protected]:find_ram_offset size: 0x40000 @ 0x92c0000 [email protected]:find_ram_offset size: 0x200000 @ 0x9300000 [email protected]:find_ram_offset size: 0x1000 @ 0x9500000 [email protected]:find_ram_offset size: 0x1000 @ 0x9540000
--- exec.c | 5 +++++ 1 file changed, 5 insertions(+)
exec.c +5 -0
@@ -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.
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH 2/2] find_ram_offset: Align ram_addr_t allocation on long boundaries
Quoted 3 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The dirty bitmaps are built from 'long'sand there is fast-path code
s/'long'sand/'long's, and/
Quoted 10 lines
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 <[email protected]>Signed-off-by Dr. David Alan Gilbert <[email protected]>--- exec.c | 5 +++++ 1 file changed, 5 insertions(+)
Quoted 18 lines
exec.c +5
-0@@ -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.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 2/2] find_ram_offset: Align ram_addr_t allocation on long boundaries
In reply to Dr. David Alan Gilbert (git)Quoted 29 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
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:
[email protected]:find_ram_offset size:0x1e0000@0x8000000[email protected]:find_ram_offset size:0x20000@0x81e0000[email protected]:find_ram_offset size:0x20000@0x8200000[email protected]:find_ram_offset size:0x1000000@0x8220000[email protected]:find_ram_offset size:0x10000@0x9220000[email protected]:find_ram_offset size:0x40000@0x9230000[email protected]:find_ram_offset size:0x200000@0x9270000[email protected]:find_ram_offset size:0x1000@0x9470000[email protected]:find_ram_offset size:0x1000@0x9471000after change:
[email protected]:find_ram_offset size:0x1e0000@0x8000000[email protected]:find_ram_offset size:0x20000@0x8200000[email protected]:find_ram_offset size:0x20000@0x8240000[email protected]:find_ram_offset size:0x1000000@0x8280000[email protected]:find_ram_offset size:0x10000@0x9280000[email protected]:find_ram_offset size:0x40000@0x92c0000[email protected]:find_ram_offset size:0x200000@0x9300000[email protected]:find_ram_offset size:0x1000@0x9500000[email protected]:find_ram_offset size:0x1000@0x9540000
Quoted 2 lines
Suggested-by Paolo Bonzini <[email protected]>Signed-off-by Dr. David Alan Gilbert <[email protected]>
Re: [Qemu-devel] [PATCH 2/2] find_ram_offset: Align ram_addr_t allocation on long boundaries
In reply to Dr. David Alan Gilbert (git)Quoted 40 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
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:
[email protected]:find_ram_offset size:0x1e0000@0x8000000[email protected]:find_ram_offset size:0x20000@0x81e0000[email protected]:find_ram_offset size:0x20000@0x8200000[email protected]:find_ram_offset size:0x1000000@0x8220000[email protected]:find_ram_offset size:0x10000@0x9220000[email protected]:find_ram_offset size:0x40000@0x9230000[email protected]:find_ram_offset size:0x200000@0x9270000[email protected]:find_ram_offset size:0x1000@0x9470000[email protected]:find_ram_offset size:0x1000@0x9471000after change:
[email protected]:find_ram_offset size:0x1e0000@0x8000000[email protected]:find_ram_offset size:0x20000@0x8200000[email protected]:find_ram_offset size:0x20000@0x8240000[email protected]:find_ram_offset size:0x1000000@0x8280000[email protected]:find_ram_offset size:0x10000@0x9280000[email protected]:find_ram_offset size:0x40000@0x92c0000[email protected]:find_ram_offset size:0x200000@0x9300000[email protected]:find_ram_offset size:0x1000@0x9500000[email protected]:find_ram_offset size:0x1000@0x9540000Suggested-by Paolo Bonzini <[email protected]>Signed-off-by Dr. David Alan Gilbert <[email protected]>--- exec.c | 5 +++++ 1 file changed, 5 insertions(+)exec.c +0
-0
exec.c static ram_addr_t find_ram_offset(ram_addr_t size) · line 1700
Quoted 7 lines
exec.c +4
-0@@ -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
Quoted 4 lines
exec.c +0
-0/* 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.