[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/9] vga: do not dynamically allocate chain4_ali
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 4/9] vga: do not dynamically allocate chain4_alias |
Date: |
Thu, 31 Jul 2014 14:06:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 |
Il 31/07/2014 14:01, Peter Crosthwaite ha scritto:
> On Wed, Jul 30, 2014 at 8:27 PM, Paolo Bonzini <address@hidden> wrote:
>> Instead, add a boolean variable to indicate the presence of the region.
>>
>
> Patch is good. Can you add a sentence on why you are doing this
> though? Is it just the save on the repeated malloc and free (which is
> sane in its own right) or something bigger in the context of your
> series?
Even more than the repeated malloc and free, it's the repeated
add_child/unparent.
Paolo
>> Signed-off-by: Paolo Bonzini <address@hidden>
>
> Otherwise,
>
> Reviewed-by: Peter Crosthwaite <address@hidden>
>
>> ---
>> hw/display/vga.c | 24 ++++++++++--------------
>> hw/display/vga_int.h | 3 ++-
>> 2 files changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/display/vga.c b/hw/display/vga.c
>> index 4b089a3..68cfee2 100644
>> --- a/hw/display/vga.c
>> +++ b/hw/display/vga.c
>> @@ -168,15 +168,18 @@ static uint8_t expand4to8[16];
>>
>> static void vga_update_memory_access(VGACommonState *s)
>> {
>> - MemoryRegion *region, *old_region = s->chain4_alias;
>> hwaddr base, offset, size;
>>
>> if (s->legacy_address_space == NULL) {
>> return;
>> }
>>
>> - s->chain4_alias = NULL;
>> -
>> + if (s->has_chain4_alias) {
>> + memory_region_del_subregion(s->legacy_address_space,
>> &s->chain4_alias);
>> + memory_region_destroy(&s->chain4_alias);
>> + s->has_chain4_alias = false;
>> + s->plane_updated = 0xf;
>> + }
>> if ((s->sr[VGA_SEQ_PLANE_WRITE] & VGA_SR02_ALL_PLANES) ==
>> VGA_SR02_ALL_PLANES && s->sr[VGA_SEQ_MEMORY_MODE] &
>> VGA_SR04_CHN_4M) {
>> offset = 0;
>> @@ -201,18 +204,11 @@ static void vga_update_memory_access(VGACommonState *s)
>> break;
>> }
>> base += isa_mem_base;
>> - region = g_malloc(sizeof(*region));
>> - memory_region_init_alias(region, memory_region_owner(&s->vram),
>> + memory_region_init_alias(&s->chain4_alias,
>> memory_region_owner(&s->vram),
>> "vga.chain4", &s->vram, offset, size);
>> memory_region_add_subregion_overlap(s->legacy_address_space, base,
>> - region, 2);
>> - s->chain4_alias = region;
>> - }
>> - if (old_region) {
>> - memory_region_del_subregion(s->legacy_address_space, old_region);
>> - memory_region_destroy(old_region);
>> - g_free(old_region);
>> - s->plane_updated = 0xf;
>> + &s->chain4_alias, 2);
>> + s->has_chain4_alias = true;
>> }
>> }
>>
>> @@ -1321,7 +1317,7 @@ static void vga_draw_text(VGACommonState *s, int
>> full_update)
>> s->font_offsets[1] = offset;
>> full_update = 1;
>> }
>> - if (s->plane_updated & (1 << 2) || s->chain4_alias) {
>> + if (s->plane_updated & (1 << 2) || s->has_chain4_alias) {
>> /* if the plane 2 was modified since the last display, it
>> indicates the font may have been modified */
>> s->plane_updated = 0;
>> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
>> index 5320abd..641f8f4 100644
>> --- a/hw/display/vga_int.h
>> +++ b/hw/display/vga_int.h
>> @@ -94,7 +94,8 @@ typedef struct VGACommonState {
>> uint32_t vram_size;
>> uint32_t vram_size_mb; /* property */
>> uint32_t latch;
>> - MemoryRegion *chain4_alias;
>> + bool has_chain4_alias;
>> + MemoryRegion chain4_alias;
>> uint8_t sr_index;
>> uint8_t sr[256];
>> uint8_t gr_index;
>> --
>> 1.8.3.1
>>
>>
>>
- [Qemu-devel] [PATCH for-2.2 0/9] memory: remove memory_region_destroy, Paolo Bonzini, 2014/07/30
- [Qemu-devel] [PATCH 1/9] qom: object: delete properties before calling instance_finalize, Paolo Bonzini, 2014/07/30
- [Qemu-devel] [PATCH 4/9] vga: do not dynamically allocate chain4_alias, Paolo Bonzini, 2014/07/30
- [Qemu-devel] [PATCH 2/9] qom: object: move unparenting to the child property's release callback, Paolo Bonzini, 2014/07/30
- [Qemu-devel] [PATCH 3/9] sysbus: remove unused function sysbus_del_io, Paolo Bonzini, 2014/07/30
- [Qemu-devel] [PATCH 5/9] nic: do not destroy memory regions in cleanup functions, Paolo Bonzini, 2014/07/30
- [Qemu-devel] [PATCH 6/9] ioport: split deletion and destruction, Paolo Bonzini, 2014/07/30
- [Qemu-devel] [PATCH 7/9] memory: convert memory_region_destroy to object_unparent, Paolo Bonzini, 2014/07/30