On Thu, Aug 9, 2012 at 4:00 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > Il 09/08/2012 09:28, liu ping fan ha scritto: >>> > VCPU thread I/O thread >>> > ===================================================================== >>> > get MMIO request >>> > rcu_read_lock() >>> > walk memory map >>> > qdev_unmap() >>> > lock_devtree() >>> > ... >>> > unlock_devtree >>> > unref dev -> refcnt=0, free enqueued >>> > ref() >> No ref() for dev here, while we have ref to flatview+radix in my patches. >> I use rcu to protect radix+flatview+mr refered. As to dev, its ref has >> inc when it is added into mem view -- that is >> memory_region_add_subregion -> memory_region_get() { >> if(atomic_add_and_return()) dev->ref++ }. >> So not until reclaimer of mem view, the dev's ref is hold by mem view. >> In a short word, rcu protect mem view, while device is protected by refcnt. > > But the RCU critical section should not include the whole processing of > MMIO, only the walk of the memory map. > Yes, you are right. And I think cur_map_get() can be broken into the style "lock, ref++, phys_page_find(); unlock". easily. > And in general I think this is a bit too tricky... I understand not > adding refcounting to all of bottom halves, timers, etc., but if you are > using a device you should have explicit ref/unref pairs. > Actually, there are pairs -- when dev enter mem view, inc ref; and when it leave, dec ref. But as Avi has pointed out, the mr->refcnt introduce complicate and no gain. So I will discard this design Thanks and regards, pingfan > Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html