Il 29/05/2013 04:11, Liu Ping Fan ha scritto:
> From: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
>
> Currently, phys_node_map and phys_sections are shared by all
> of the AddressSpaceDispatch. When updating mem topology, all
> AddressSpaceDispatch will rebuild dispatch tables sequentially
> on them. In order to use rcu-style, introducing next_node_map
> and next_phys_sections, so that when rebuilding, the new dispatch
> tables will locate on next_*. After all AddressSpaceDispatch
> finished, we can switch to next_* and drop the previous stuff.
>
> This patch still work in the scenario that readers and writer
> can not run in parellel (ie, protected by biglock), so no extra
> lock method needed till now, and the rcu style consistent issue
> will be left to the following patches.
>
> Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
> ---
> exec.c | 202 ++++++++++++++++++++++++++++++++++-----------------------------
> 1 files changed, 109 insertions(+), 93 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 6934c2d..e5335f5 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -80,16 +80,33 @@ int use_icount;
>
> #if !defined(CONFIG_USER_ONLY)
>
> -static MemoryRegionSection *phys_sections;
> -static unsigned phys_sections_nb, phys_sections_nb_alloc;
> #define phys_section_unassigned 0
> #define phys_section_notdirty 1
> #define phys_section_rom 2
> #define phys_section_watch 3
>
> -/* Simple allocator for PhysPageEntry nodes */
> -static PhysPageEntry (*phys_map_nodes)[L2_SIZE];
> -static unsigned phys_map_nodes_nb, phys_map_nodes_nb_alloc;
> +typedef PhysPageEntry Node[L2_SIZE];
> +
> +typedef struct AllocInfo {
> + unsigned phys_sections_nb;
> + unsigned phys_sections_nb_alloc;
> + unsigned phys_map_nodes_nb;
> + unsigned phys_map_nodes_nb_alloc;
> + /* Only used for release purpose */
> + Node *map;
Node *nodes;
> + MemoryRegionSection *sections;
> +} AllocInfo;
> +
> +/* For performance, fetch page map related pointers directly, other than
> + * hiding them inside AllocInfo
> + */
> +static MemoryRegionSection *cur_phys_sections;
> +static Node *cur_map_nodes;
> +static AllocInfo *cur_alloc_info;
> +
> +static MemoryRegionSection *next_phys_sections;
> +static Node *next_map_nodes;
> +static AllocInfo *next_alloc_info;
No need for the "next" variables, just make it "static AllocInfo *next_map".
>
> #define PHYS_MAP_NODE_NIL (((uint16_t)~0) >> 1)
>
> @@ -104,13 +121,15 @@ static MemoryRegion io_mem_watch;
>
> static void phys_map_node_reserve(unsigned nodes)
> {
> - if (phys_map_nodes_nb + nodes > phys_map_nodes_nb_alloc) {
> - typedef PhysPageEntry Node[L2_SIZE];
> - phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc * 2, 16);
> - phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc,
> - phys_map_nodes_nb + nodes);
> - phys_map_nodes = g_renew(Node, phys_map_nodes,
> - phys_map_nodes_nb_alloc);
> + AllocInfo *info = next_alloc_info;
> +
> + if (info->phys_map_nodes_nb + nodes > info->phys_map_nodes_nb_alloc) {
> + info->phys_map_nodes_nb_alloc = MAX(info->phys_map_nodes_nb_alloc *
> 2,
> + 16);
> + info->phys_map_nodes_nb_alloc = MAX(info->phys_map_nodes_nb_alloc,
> + info->phys_map_nodes_nb + nodes);
> + next_map_nodes = g_renew(Node, next_map_nodes,
> + info->phys_map_nodes_nb_alloc);
> }
> }
>
> @@ -118,23 +137,18 @@ static uint16_t phys_map_node_alloc(void)
> {
> unsigned i;
> uint16_t ret;
> + AllocInfo *info = next_alloc_info;
>
> - ret = phys_map_nodes_nb++;
> + ret = info->phys_map_nodes_nb++;
> assert(ret != PHYS_MAP_NODE_NIL);
> - assert(ret != phys_map_nodes_nb_alloc);
> + assert(ret != info->phys_map_nodes_nb_alloc);
> for (i = 0; i < L2_SIZE; ++i) {
> - phys_map_nodes[ret][i].is_leaf = 0;
> - phys_map_nodes[ret][i].ptr = PHYS_MAP_NODE_NIL;
> + next_map_nodes[ret][i].is_leaf = 0;
> + next_map_nodes[ret][i].ptr = PHYS_MAP_NODE_NIL;
> }
> return ret;
> }
>
> -static void phys_map_nodes_reset(void)
> -{
> - phys_map_nodes_nb = 0;
> -}
> -
> -
> static void phys_page_set_level(PhysPageEntry *lp, hwaddr *index,
> hwaddr *nb, uint16_t leaf,
> int level)
> @@ -145,7 +159,7 @@ static void phys_page_set_level(PhysPageEntry *lp, hwaddr
> *index,
>
> if (!lp->is_leaf && lp->ptr == PHYS_MAP_NODE_NIL) {
> lp->ptr = phys_map_node_alloc();
> - p = phys_map_nodes[lp->ptr];
> + p = next_map_nodes[lp->ptr];
> if (level == 0) {
> for (i = 0; i < L2_SIZE; i++) {
> p[i].is_leaf = 1;
> @@ -153,7 +167,7 @@ static void phys_page_set_level(PhysPageEntry *lp, hwaddr
> *index,
> }
> }
> } else {
> - p = phys_map_nodes[lp->ptr];
> + p = next_map_nodes[lp->ptr];
> }
> lp = &p[(*index >> (level * L2_BITS)) & (L2_SIZE - 1)];
>
> @@ -180,20 +194,29 @@ static void phys_page_set(AddressSpaceDispatch *d,
> phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1);
> }
>
> -static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr
> index)
> +static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr
> index, bool cur)
> {
> PhysPageEntry lp = d->phys_map;
> PhysPageEntry *p;
> int i;
> + Node *map_nodes;
> + MemoryRegionSection *sections;
>
> + if (likely(cur)) {
> + map_nodes = cur_map_nodes;
> + sections = cur_phys_sections;
> + } else {
> + map_nodes = next_map_nodes;
> + sections = next_phys_sections;
> + }
Just pass the AllocInfo here.
> for (i = P_L2_LEVELS - 1; i >= 0 && !lp.is_leaf; i--) {
> if (lp.ptr == PHYS_MAP_NODE_NIL) {
> - return &phys_sections[phys_section_unassigned];
> + return §ions[phys_section_unassigned];
> }
> - p = phys_map_nodes[lp.ptr];
> + p = map_nodes[lp.ptr];
> lp = p[(index >> (i * L2_BITS)) & (L2_SIZE - 1)];
> }
> - return &phys_sections[lp.ptr];
> + return §ions[lp.ptr];
> }
>
> bool memory_region_is_unassigned(MemoryRegion *mr)
> @@ -205,7 +228,7 @@ bool memory_region_is_unassigned(MemoryRegion *mr)
> static MemoryRegionSection *address_space_lookup_region(AddressSpace *as,
> hwaddr addr)
> {
> - return phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS);
> + return phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS, true);
> }
>
> MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
> @@ -213,7 +236,7 @@ MemoryRegionSection *address_space_translate(AddressSpace
> *as, hwaddr addr,
> bool is_write)
> {
> IOMMUTLBEntry iotlb;
> - MemoryRegionSection *section;
> + MemoryRegionSection *section, *base = cur_phys_sections;
> hwaddr len = *plen;
>
> for (;;) {
> @@ -235,7 +258,7 @@ MemoryRegionSection *address_space_translate(AddressSpace
> *as, hwaddr addr,
> | (addr & iotlb.addr_mask));
> len = MIN(len, (addr | iotlb.addr_mask) - addr + 1);
> if (!(iotlb.perm & (1 << is_write))) {
> - section = &phys_sections[phys_section_unassigned];
> + section = &base[phys_section_unassigned];
> break;
> }
>
> @@ -677,7 +700,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
> iotlb |= phys_section_rom;
> }
> } else {
> - iotlb = section - phys_sections;
> + iotlb = section - cur_phys_sections;
> iotlb += xlat;
> }
>
> @@ -710,67 +733,31 @@ typedef struct subpage_t {
> static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
> uint16_t section);
> static subpage_t *subpage_init(hwaddr base);
> -static void destroy_page_desc(uint16_t section_index)
> -{
> - MemoryRegionSection *section = &phys_sections[section_index];
> - MemoryRegion *mr = section->mr;
> -
> - if (mr->subpage) {
> - subpage_t *subpage = container_of(mr, subpage_t, iomem);
> - memory_region_destroy(&subpage->iomem);
> - g_free(subpage);
> - }
> -}
> -
> -static void destroy_l2_mapping(PhysPageEntry *lp, unsigned level)
> -{
> - unsigned i;
> - PhysPageEntry *p;
> -
> - if (lp->ptr == PHYS_MAP_NODE_NIL) {
> - return;
> - }
> -
> - p = phys_map_nodes[lp->ptr];
> - for (i = 0; i < L2_SIZE; ++i) {
> - if (!p[i].is_leaf) {
> - destroy_l2_mapping(&p[i], level - 1);
> - } else {
> - destroy_page_desc(p[i].ptr);
> - }
> - }
> - lp->is_leaf = 0;
> - lp->ptr = PHYS_MAP_NODE_NIL;
> -}
> -
> -static void destroy_all_mappings(AddressSpaceDispatch *d)
> -{
> - destroy_l2_mapping(&d->phys_map, P_L2_LEVELS - 1);
> - phys_map_nodes_reset();
> -}
>
> static uint16_t phys_section_add(MemoryRegionSection *section)
> {
> + AllocInfo *info = next_alloc_info;
> /* The physical section number is ORed with a page-aligned
> * pointer to produce the iotlb entries. Thus it should
> * never overflow into the page-aligned value.
> */
> - assert(phys_sections_nb < TARGET_PAGE_SIZE);
> + assert(info->phys_sections_nb < TARGET_PAGE_SIZE);
>
> - if (phys_sections_nb == phys_sections_nb_alloc) {
> - phys_sections_nb_alloc = MAX(phys_sections_nb_alloc * 2, 16);
> - phys_sections = g_renew(MemoryRegionSection, phys_sections,
> - phys_sections_nb_alloc);
> + if (info->phys_sections_nb == info->phys_sections_nb_alloc) {
> + info->phys_sections_nb_alloc = MAX(info->phys_sections_nb_alloc * 2,
> + 16);
> + next_phys_sections = g_renew(MemoryRegionSection, next_phys_sections,
> + info->phys_sections_nb_alloc);
> }
> - phys_sections[phys_sections_nb] = *section;
> + next_phys_sections[info->phys_sections_nb] = *section;
> memory_region_ref(section->mr);
> - return phys_sections_nb++;
> + return info->phys_sections_nb++;
> }
>
> -static void phys_sections_clear(void)
> +static void phys_sections_clear(unsigned cnt, MemoryRegionSection *mrs)
> {
> - while (phys_sections_nb > 0) {
> - MemoryRegionSection *section = &phys_sections[--phys_sections_nb];
> + while (cnt > 0) {
> + MemoryRegionSection *section = &mrs[--cnt];
> memory_region_unref(section->mr);
> }
> }
> @@ -780,7 +767,8 @@ static void register_subpage(AddressSpaceDispatch *d,
> MemoryRegionSection *secti
> subpage_t *subpage;
> hwaddr base = section->offset_within_address_space
> & TARGET_PAGE_MASK;
> - MemoryRegionSection *existing = phys_page_find(d, base >>
> TARGET_PAGE_BITS);
> + MemoryRegionSection *existing = phys_page_find(d,
> + base >> TARGET_PAGE_BITS, false);
> MemoryRegionSection subsection = {
> .offset_within_address_space = base,
> .size = TARGET_PAGE_SIZE,
> @@ -1572,13 +1560,13 @@ static uint64_t subpage_read(void *opaque, hwaddr
> addr,
> unsigned int idx = SUBPAGE_IDX(addr);
> uint64_t val;
>
> - MemoryRegionSection *section;
> + MemoryRegionSection *section, *base = cur_phys_sections;
> #if defined(DEBUG_SUBPAGE)
> printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d\n",
> __func__,
> mmio, len, addr, idx);
> #endif
>
> - section = &phys_sections[mmio->sub_section[idx]];
> + section = &base[mmio->sub_section[idx]];
> addr += mmio->base;
> addr -= section->offset_within_address_space;
> addr += section->offset_within_region;
> @@ -1591,14 +1579,14 @@ static void subpage_write(void *opaque, hwaddr addr,
> {
> subpage_t *mmio = opaque;
> unsigned int idx = SUBPAGE_IDX(addr);
> - MemoryRegionSection *section;
> + MemoryRegionSection *section, *base = cur_phys_sections;
> #if defined(DEBUG_SUBPAGE)
> printf("%s: subpage %p len %d addr " TARGET_FMT_plx
> " idx %d value %"PRIx64"\n",
> __func__, mmio, len, addr, idx, value);
> #endif
>
> - section = &phys_sections[mmio->sub_section[idx]];
> + section = &base[mmio->sub_section[idx]];
> addr += mmio->base;
> addr -= section->offset_within_address_space;
> addr += section->offset_within_region;
> @@ -1610,14 +1598,14 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
> {
> subpage_t *mmio = opaque;
> unsigned int idx = SUBPAGE_IDX(addr);
> - MemoryRegionSection *section;
> + MemoryRegionSection *section, *base = cur_phys_sections;;
> #if defined(DEBUG_SUBPAGE)
> printf("%s: subpage %p %c len %d addr " TARGET_FMT_plx
> " idx %d\n", __func__, mmio,
> is_write ? 'w' : 'r', len, addr, idx);
> #endif
>
> - section = &phys_sections[mmio->sub_section[idx]];
> + section = &base[mmio->sub_section[idx]];
> addr += mmio->base;
> addr -= section->offset_within_address_space;
> addr += section->offset_within_region;
> @@ -1676,8 +1664,8 @@ static int subpage_register (subpage_t *mmio, uint32_t
> start, uint32_t end,
> printf("%s: %p start %08x end %08x idx %08x eidx %08x mem %ld\n",
> __func__,
> mmio, start, end, idx, eidx, memory);
> #endif
> - if (memory_region_is_ram(phys_sections[section].mr)) {
> - MemoryRegionSection new_section = phys_sections[section];
> + if (memory_region_is_ram(next_phys_sections[section].mr)) {
> + MemoryRegionSection new_section = next_phys_sections[section];
> new_section.mr = &io_mem_subpage_ram;
> section = phys_section_add(&new_section);
> }
> @@ -1721,7 +1709,7 @@ static uint16_t dummy_section(MemoryRegion *mr)
>
> MemoryRegion *iotlb_to_region(hwaddr index)
> {
> - return phys_sections[index & ~TARGET_PAGE_MASK].mr;
> + return cur_phys_sections[index & ~TARGET_PAGE_MASK].mr;
> }
>
> static void io_mem_init(void)
> @@ -1741,7 +1729,6 @@ static void mem_begin(MemoryListener *listener)
> {
> AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch,
> listener);
>
> - destroy_all_mappings(d);
> d->phys_map.ptr = PHYS_MAP_NODE_NIL;
> }
>
> @@ -1749,7 +1736,7 @@ static void core_begin(MemoryListener *listener)
> {
> uint16_t n;
>
> - phys_sections_clear();
> + next_alloc_info = g_malloc0(sizeof(AllocInfo));
> n = dummy_section(&io_mem_unassigned);
> assert(phys_section_unassigned == n);
> n = dummy_section(&io_mem_notdirty);
> @@ -1760,6 +1747,35 @@ static void core_begin(MemoryListener *listener)
> assert(phys_section_watch == n);
> }
>
> +static void release_dispatch_map(AllocInfo *info)
> +{
> + phys_sections_clear(info->phys_sections_nb, info->sections);
> + g_free(info->map);
> + g_free(info->sections);
> + g_free(info);
> +}
> +
> +/* This listener's commit run after the other AddressSpaceDispatch
> listeners'.
> + * It means that AddressSpaceDispatch's deleter has finished, so it can be
> + * the place for call_rcu()
> + */
> +static void core_commit(MemoryListener *listener)
> +{
> + AllocInfo *info = cur_alloc_info;
> + info->map = cur_map_nodes;
> + info->sections = cur_phys_sections;
> +
> + /* Fix me, in future, will be protected by wr seqlock when in parellel
> + * with reader
> + */
> + cur_map_nodes = next_map_nodes;
> + cur_phys_sections = next_phys_sections;
> + cur_alloc_info = next_alloc_info;
> +
> + /* Fix me, will changed to call_rcu */
> + release_dispatch_map(info);
> +}
> +
> static void tcg_commit(MemoryListener *listener)
> {
> CPUArchState *env;
> @@ -1802,6 +1818,7 @@ static void io_region_del(MemoryListener *listener,
>
> static MemoryListener core_memory_listener = {
> .begin = core_begin,
> + .commit = core_commit,
> .log_global_start = core_log_global_start,
> .log_global_stop = core_log_global_stop,
> .priority = 1,
> @@ -1837,7 +1854,6 @@ void address_space_destroy_dispatch(AddressSpace *as)
> AddressSpaceDispatch *d = as->dispatch;
>
> memory_listener_unregister(&d->listener);
> - destroy_l2_mapping(&d->phys_map, P_L2_LEVELS - 1);
> g_free(d);
> as->dispatch = NULL;
> }
>
