On Wed, Aug 28, 2013 at 06:13:43PM +0800, Xiao Guangrong wrote: > On 08/28/2013 05:46 PM, Gleb Natapov wrote: > > On Wed, Aug 28, 2013 at 05:33:49PM +0800, Xiao Guangrong wrote: > >>> Or what if desc is moved to another rmap, but then it > >>> is moved back to initial rmap (but another place in the desc list) so > >>> the check here will not catch that we need to restart walking? > >> > >> It is okay. We always add the new desc to the head, then we will walk > >> all the entires under this case. > >> > > Which races another question: What if desc is added in front of the list > > behind the point where lockless walker currently is? > > That case is new spte is being added into the rmap. We need not to care the > new sptes since it will set the dirty-bitmap then they can be write-protected > next time. > OK. > > > >> Right? > > Not sure. While lockless walker works on a desc rmap can be completely > > destroyed and recreated again. It can be any order. > > I think the thing is very similar as include/linux/rculist_nulls.h include/linux/rculist_nulls.h is for implementing hash tables, so they may not care about add/del/lookup race for instance, but may be we are (you are saying above that we are not), so similarity does not prove correctness for our case. BTW I do not see rcu_assign_pointer()/rcu_dereference() in your patches which hints on incorrect usage of RCU. I think any access to slab pointers will need to use those. -- Gleb. -- 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