Hi Peter, On 8/19/19 10:11 AM, Peter Xu wrote: > On Tue, Jul 30, 2019 at 07:21:30PM +0200, Eric Auger wrote: > > [...] > >> + mapping = g_tree_lookup(domain->mappings, (gpointer)(&interval)); >> + >> + while (mapping) { >> + viommu_interval current; >> + uint64_t low = mapping->virt_addr; >> + uint64_t high = mapping->virt_addr + mapping->size - 1; >> + >> + current.low = low; >> + current.high = high; >> + >> + if (low == interval.low && size >= mapping->size) { >> + g_tree_remove(domain->mappings, (gpointer)(¤t)); >> + interval.low = high + 1; >> + trace_virtio_iommu_unmap_left_interval(current.low, >> current.high, >> + interval.low, interval.high); >> + } else if (high == interval.high && size >= mapping->size) { >> + trace_virtio_iommu_unmap_right_interval(current.low, >> current.high, >> + interval.low, interval.high); >> + g_tree_remove(domain->mappings, (gpointer)(¤t)); >> + interval.high = low - 1; >> + } else if (low > interval.low && high < interval.high) { >> + trace_virtio_iommu_unmap_inc_interval(current.low, >> current.high); >> + g_tree_remove(domain->mappings, (gpointer)(¤t)); >> + } else { >> + break; >> + } >> + if (interval.low >= interval.high) { >> + return VIRTIO_IOMMU_S_OK; >> + } else { >> + mapping = g_tree_lookup(domain->mappings, >> (gpointer)(&interval)); >> + } >> + } >> + >> + if (mapping) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "****** %s: Unmap 0x%"PRIx64" size=0x%"PRIx64 >> + " from 0x%"PRIx64" size=0x%"PRIx64" is not >> supported\n", >> + __func__, interval.low, size, >> + mapping->virt_addr, mapping->size); >> + } else { >> + return VIRTIO_IOMMU_S_OK; >> + } >> + >> + return VIRTIO_IOMMU_S_INVAL; > > Could the above chunk be simplified as something like below? > > while ((mapping = g_tree_lookup(domain->mappings, &interval))) { > g_tree_remove(domain->mappings, mapping); > } Indeed the code could be simplified. I only need to make sure I don't split an existing mapping.
Also I needed to use g_tree_lookup_extended to retrieve the actual key to remove. The usage of g_tree_lookup_extended() allows me to remove the virt_addr and size fields from the mapping value value struct as those info can be retrieved from the key. Thanks! Eric > > Thanks, >