| Message ID | 1421120079-987-2-git-send-email-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
El Mon, 12 Jan 2015 22:34:26 -0500 John Snow <jsnow@redhat.com> escribió: > Move the list-specific initialization over into > malloc.c, to keep all of the list implementation > details within the same file. > > The allocation and freeing of these structures are > now both back within the same layer. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > tests/libqos/malloc-pc.c | 20 ++++---------------- > tests/libqos/malloc.c | 17 +++++++++++++++++ > tests/libqos/malloc.h | 1 + > 3 files changed, 22 insertions(+), 16 deletions(-) > > diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c > index c9c48fd..36a0740 100644 > --- a/tests/libqos/malloc-pc.c > +++ b/tests/libqos/malloc-pc.c > @@ -32,31 +32,19 @@ void pc_alloc_uninit(QGuestAllocator *allocator) > > QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags) > { > - QGuestAllocator *s = g_malloc0(sizeof(*s)); > + QGuestAllocator *s; > uint64_t ram_size; > QFWCFG *fw_cfg = pc_fw_cfg_init(); > - MemBlock *node; > + > + ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE); > + s = alloc_init(1 << 20, MIN(ram_size, 0xE0000000)); > > s->opts = flags; > s->page_size = PAGE_SIZE; Is there a reason to leave page_size out of the function? (flags is considered in a patch later, ok). I think it would be interesting to have both, so pc_alloc_init_flags can forget about the fields in QGuestAllocator. Thanks Marc
On 01/13/2015 03:54 AM, Marc Marí wrote: > El Mon, 12 Jan 2015 22:34:26 -0500 > John Snow <jsnow@redhat.com> escribió: >> Move the list-specific initialization over into >> malloc.c, to keep all of the list implementation >> details within the same file. >> >> The allocation and freeing of these structures are >> now both back within the same layer. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> tests/libqos/malloc-pc.c | 20 ++++---------------- >> tests/libqos/malloc.c | 17 +++++++++++++++++ >> tests/libqos/malloc.h | 1 + >> 3 files changed, 22 insertions(+), 16 deletions(-) >> >> diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c >> index c9c48fd..36a0740 100644 >> --- a/tests/libqos/malloc-pc.c >> +++ b/tests/libqos/malloc-pc.c >> @@ -32,31 +32,19 @@ void pc_alloc_uninit(QGuestAllocator *allocator) >> >> QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags) >> { >> - QGuestAllocator *s = g_malloc0(sizeof(*s)); >> + QGuestAllocator *s; >> uint64_t ram_size; >> QFWCFG *fw_cfg = pc_fw_cfg_init(); >> - MemBlock *node; >> + >> + ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE); >> + s = alloc_init(1 << 20, MIN(ram_size, 0xE0000000)); >> >> s->opts = flags; >> s->page_size = PAGE_SIZE; > > Is there a reason to leave page_size out of the function? (flags is > considered in a patch later, ok). I think it would be interesting to > have both, so pc_alloc_init_flags can forget about the fields in > QGuestAllocator. > > Thanks > Marc > There was no strong motivation, I just saw it as something that perhaps other architectures may want to change. A setter method would also work well for not needing to know the internal representation of the allocator object. Thanks, --js
On 13/01/2015 04:34, John Snow wrote: > Move the list-specific initialization over into > malloc.c, to keep all of the list implementation > details within the same file. > > The allocation and freeing of these structures are > now both back within the same layer. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > tests/libqos/malloc-pc.c | 20 ++++---------------- > tests/libqos/malloc.c | 17 +++++++++++++++++ > tests/libqos/malloc.h | 1 + > 3 files changed, 22 insertions(+), 16 deletions(-) > > diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c > index c9c48fd..36a0740 100644 > --- a/tests/libqos/malloc-pc.c > +++ b/tests/libqos/malloc-pc.c > @@ -32,31 +32,19 @@ void pc_alloc_uninit(QGuestAllocator *allocator) > > QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags) > { > - QGuestAllocator *s = g_malloc0(sizeof(*s)); > + QGuestAllocator *s; > uint64_t ram_size; > QFWCFG *fw_cfg = pc_fw_cfg_init(); > - MemBlock *node; > + > + ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE); > + s = alloc_init(1 << 20, MIN(ram_size, 0xE0000000)); > > s->opts = flags; > s->page_size = PAGE_SIZE; > > - ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE); > - > - /* Start at 1MB */ > - s->start = 1 << 20; > - > - /* Respect PCI hole */ > - s->end = MIN(ram_size, 0xE0000000); > - > /* clean-up */ > g_free(fw_cfg); > > - QTAILQ_INIT(&s->used); > - QTAILQ_INIT(&s->free); > - > - node = mlist_new(s->start, s->end - s->start); > - QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME); > - > return s; > } > > diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c > index 5debf18..0d34ecd 100644 > --- a/tests/libqos/malloc.c > +++ b/tests/libqos/malloc.c > @@ -268,3 +268,20 @@ void guest_free(QGuestAllocator *allocator, uint64_t addr) > mlist_check(allocator); > } > } > + > +QGuestAllocator *alloc_init(uint64_t start, uint64_t end) > +{ > + QGuestAllocator *s = g_malloc0(sizeof(*s)); > + MemBlock *node; > + > + s->start = start; > + s->end = end; > + > + QTAILQ_INIT(&s->used); > + QTAILQ_INIT(&s->free); > + > + node = mlist_new(s->start, s->end - s->start); > + QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME); > + > + return s; > +} > diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h > index 465efeb..677db77 100644 > --- a/tests/libqos/malloc.h > +++ b/tests/libqos/malloc.h > @@ -50,4 +50,5 @@ void alloc_uninit(QGuestAllocator *allocator); > uint64_t guest_alloc(QGuestAllocator *allocator, size_t size); > void guest_free(QGuestAllocator *allocator, uint64_t addr); > > +QGuestAllocator *alloc_init(uint64_t start, uint64_t end); > #endif > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c index c9c48fd..36a0740 100644 --- a/tests/libqos/malloc-pc.c +++ b/tests/libqos/malloc-pc.c @@ -32,31 +32,19 @@ void pc_alloc_uninit(QGuestAllocator *allocator) QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags) { - QGuestAllocator *s = g_malloc0(sizeof(*s)); + QGuestAllocator *s; uint64_t ram_size; QFWCFG *fw_cfg = pc_fw_cfg_init(); - MemBlock *node; + + ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE); + s = alloc_init(1 << 20, MIN(ram_size, 0xE0000000)); s->opts = flags; s->page_size = PAGE_SIZE; - ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE); - - /* Start at 1MB */ - s->start = 1 << 20; - - /* Respect PCI hole */ - s->end = MIN(ram_size, 0xE0000000); - /* clean-up */ g_free(fw_cfg); - QTAILQ_INIT(&s->used); - QTAILQ_INIT(&s->free); - - node = mlist_new(s->start, s->end - s->start); - QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME); - return s; } diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c index 5debf18..0d34ecd 100644 --- a/tests/libqos/malloc.c +++ b/tests/libqos/malloc.c @@ -268,3 +268,20 @@ void guest_free(QGuestAllocator *allocator, uint64_t addr) mlist_check(allocator); } } + +QGuestAllocator *alloc_init(uint64_t start, uint64_t end) +{ + QGuestAllocator *s = g_malloc0(sizeof(*s)); + MemBlock *node; + + s->start = start; + s->end = end; + + QTAILQ_INIT(&s->used); + QTAILQ_INIT(&s->free); + + node = mlist_new(s->start, s->end - s->start); + QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME); + + return s; +} diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h index 465efeb..677db77 100644 --- a/tests/libqos/malloc.h +++ b/tests/libqos/malloc.h @@ -50,4 +50,5 @@ void alloc_uninit(QGuestAllocator *allocator); uint64_t guest_alloc(QGuestAllocator *allocator, size_t size); void guest_free(QGuestAllocator *allocator, uint64_t addr); +QGuestAllocator *alloc_init(uint64_t start, uint64_t end); #endif
Move the list-specific initialization over into malloc.c, to keep all of the list implementation details within the same file. The allocation and freeing of these structures are now both back within the same layer. Signed-off-by: John Snow <jsnow@redhat.com> --- tests/libqos/malloc-pc.c | 20 ++++---------------- tests/libqos/malloc.c | 17 +++++++++++++++++ tests/libqos/malloc.h | 1 + 3 files changed, 22 insertions(+), 16 deletions(-)