| Message ID | 1455935721-8804-2-git-send-email-arei.gonglei@huawei.com |
|---|---|
| State | New |
| Headers | show |
On Sat, 02/20 10:35, Gonglei wrote: > Each RAM memory region has a unique corresponding RAMBlock. > In the current realization, the memory region only stored > the ram_addr which means the offset of RAM address space, > We need to qurey the global ram.list to find the ram block > by ram_addr if we want to get the ram block, which is very > expensive. > > Now, we store the RAMBlock pointer into memory region > structure. So, if we know the mr, we can easily get the > RAMBlock. > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > exec.c | 2 ++ > include/exec/memory.h | 1 + > memory.c | 1 + > 3 files changed, 4 insertions(+) > > diff --git a/exec.c b/exec.c > index 1f24500..e29e369 100644 > --- a/exec.c > +++ b/exec.c > @@ -1717,6 +1717,8 @@ ram_addr_t qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size, > error_propagate(errp, local_err); > return -1; > } > + /* store the ram block pointer into memroy region */ The comment is superfluous IMHO, the code is quite self-explanatory. > + mr->ram_block = new_block; > return addr; > } > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index c92734a..23e2e3e 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -172,6 +172,7 @@ struct MemoryRegion { > bool global_locking; > uint8_t dirty_log_mask; > ram_addr_t ram_addr; > + void *ram_block; /* RAMBlock pointer */ Why not add typedef struct RAMBlock RAMBlock; then RAMBlock *ram_block; ? > Object *owner; > const MemoryRegionIOMMUOps *iommu_ops; > > diff --git a/memory.c b/memory.c > index 09041ed..b4451dd 100644 > --- a/memory.c > +++ b/memory.c > @@ -912,6 +912,7 @@ void memory_region_init(MemoryRegion *mr, > } > mr->name = g_strdup(name); > mr->owner = owner; > + mr->ram_block = NULL; > > if (name) { > char *escaped_name = memory_region_escape_name(name); > -- > 1.8.5.2 > > >
Hi Fam, > From: Fam Zheng [mailto:famz@redhat.com] > Sent: Monday, February 22, 2016 10:46 AM > > On Sat, 02/20 10:35, Gonglei wrote: > > Each RAM memory region has a unique corresponding RAMBlock. > > In the current realization, the memory region only stored > > the ram_addr which means the offset of RAM address space, > > We need to qurey the global ram.list to find the ram block > > by ram_addr if we want to get the ram block, which is very > > expensive. > > > > Now, we store the RAMBlock pointer into memory region > > structure. So, if we know the mr, we can easily get the > > RAMBlock. > > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > > --- > > exec.c | 2 ++ > > include/exec/memory.h | 1 + > > memory.c | 1 + > > 3 files changed, 4 insertions(+) > > > > diff --git a/exec.c b/exec.c > > index 1f24500..e29e369 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -1717,6 +1717,8 @@ ram_addr_t > qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size, > > error_propagate(errp, local_err); > > return -1; > > } > > + /* store the ram block pointer into memroy region */ > > The comment is superfluous IMHO, the code is quite self-explanatory. > Yes, agree. > > + mr->ram_block = new_block; > > return addr; > > } > > > > diff --git a/include/exec/memory.h b/include/exec/memory.h > > index c92734a..23e2e3e 100644 > > --- a/include/exec/memory.h > > +++ b/include/exec/memory.h > > @@ -172,6 +172,7 @@ struct MemoryRegion { > > bool global_locking; > > uint8_t dirty_log_mask; > > ram_addr_t ram_addr; > > + void *ram_block; /* RAMBlock pointer */ > > Why not add > > typedef struct RAMBlock RAMBlock; > > then > > RAMBlock *ram_block; > > ? > It's clearer. Will fix in v2, thanks :) Regards, -Gonglei > > Object *owner; > > const MemoryRegionIOMMUOps *iommu_ops; > > > > diff --git a/memory.c b/memory.c > > index 09041ed..b4451dd 100644 > > --- a/memory.c > > +++ b/memory.c > > @@ -912,6 +912,7 @@ void memory_region_init(MemoryRegion *mr, > > } > > mr->name = g_strdup(name); > > mr->owner = owner; > > + mr->ram_block = NULL; > > > > if (name) { > > char *escaped_name = memory_region_escape_name(name); > > -- > > 1.8.5.2 > > > > > >
diff --git a/exec.c b/exec.c index 1f24500..e29e369 100644 --- a/exec.c +++ b/exec.c @@ -1717,6 +1717,8 @@ ram_addr_t qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size, error_propagate(errp, local_err); return -1; } + /* store the ram block pointer into memroy region */ + mr->ram_block = new_block; return addr; } diff --git a/include/exec/memory.h b/include/exec/memory.h index c92734a..23e2e3e 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -172,6 +172,7 @@ struct MemoryRegion { bool global_locking; uint8_t dirty_log_mask; ram_addr_t ram_addr; + void *ram_block; /* RAMBlock pointer */ Object *owner; const MemoryRegionIOMMUOps *iommu_ops; diff --git a/memory.c b/memory.c index 09041ed..b4451dd 100644 --- a/memory.c +++ b/memory.c @@ -912,6 +912,7 @@ void memory_region_init(MemoryRegion *mr, } mr->name = g_strdup(name); mr->owner = owner; + mr->ram_block = NULL; if (name) { char *escaped_name = memory_region_escape_name(name);
Each RAM memory region has a unique corresponding RAMBlock. In the current realization, the memory region only stored the ram_addr which means the offset of RAM address space, We need to qurey the global ram.list to find the ram block by ram_addr if we want to get the ram block, which is very expensive. Now, we store the RAMBlock pointer into memory region structure. So, if we know the mr, we can easily get the RAMBlock. Signed-off-by: Gonglei <arei.gonglei@huawei.com> --- exec.c | 2 ++ include/exec/memory.h | 1 + memory.c | 1 + 3 files changed, 4 insertions(+)