| Message ID | 1421938053-10318-8-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 01/22 15:47, Paolo Bonzini wrote: > Do the entire lookup under RCU, which avoids atomic operations. address_space_get_flatview() already is RCU protected, I don't see why this patch is necessary. Could you explain? And there is one question below: > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > memory.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/memory.c b/memory.c > index a844ced..577e87c 100644 > --- a/memory.c > +++ b/memory.c > @@ -1828,7 +1828,8 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, > } > range = addrrange_make(int128_make64(addr), int128_make64(size)); > > - view = address_space_get_flatview(as); > + rcu_read_lock(); > + view = atomic_rcu_read(&as->current_map); > fr = flatview_lookup(view, range); > if (!fr) { > flatview_unref(view); Following lines are: return ret; } Which requires a rcu_read_unlock. Fam > @@ -1850,7 +1851,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, > ret.readonly = fr->readonly; > memory_region_ref(ret.mr); > > - flatview_unref(view); > + rcu_read_unlock(); > return ret; > } > > -- > 1.8.3.1 > >
On 26/01/2015 07:24, Fam Zheng wrote: >> > Do the entire lookup under RCU, which avoids atomic operations. > address_space_get_flatview() already is RCU protected, I don't see why this > patch is necessary. Could you explain? It's just an optimization; flatview_ref/flatview_unref do atomic_inc/atomic_dec and this patch avoids that. Paolo
diff --git a/memory.c b/memory.c index a844ced..577e87c 100644 --- a/memory.c +++ b/memory.c @@ -1828,7 +1828,8 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, } range = addrrange_make(int128_make64(addr), int128_make64(size)); - view = address_space_get_flatview(as); + rcu_read_lock(); + view = atomic_rcu_read(&as->current_map); fr = flatview_lookup(view, range); if (!fr) { flatview_unref(view); @@ -1850,7 +1851,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, ret.readonly = fr->readonly; memory_region_ref(ret.mr); - flatview_unref(view); + rcu_read_unlock(); return ret; }
Do the entire lookup under RCU, which avoids atomic operations. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- memory.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)