[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 05 of 14] Don't trigger unnecessary shadow scans on p2m entry update
At 07:24 -0800 on 24 Nov (1322119466), Andres Lagar-Cavilla wrote: > >>> @@ -2501,7 +2501,8 @@ int sh_remove_all_mappings(struct vcpu * > >>> ; > >>> > >>> perfc_incr(shadow_mappings); > >>> - if ( (page->count_info & PGC_count_mask) == 0 ) > >>> + expected_count = (page->count_info & PGC_allocated) ? 1 : 0; > >>> + if ( (page->count_info & PGC_count_mask) == expected_count ) > >> > >> Is that really valid outside the locked region? > We can move it below to be protected by the lock. It doesn't need to be protected by the lock. All we care about is that at some point after the p2m update, the count has fallen to the correct value. It would probably be better to explicitly snapshot (page->count_info & (PGC_count_mask | PGC_allocated) and make sure that that is either 0 or (1 | PGC_allocated). (Yes, the code you're copying from does it the other way, and it probably shouldn't either). > >> > >>> return 0; > >>> > >>> /* Although this is an externally visible function, we do not know > >>> @@ -2517,7 +2518,6 @@ int sh_remove_all_mappings(struct vcpu * > >>> hash_foreach(v, callback_mask, callbacks, gmfn); > >>> > >>> /* If that didn't catch the mapping, something is very wrong */ > >>> - expected_count = (page->count_info & PGC_allocated) ? 1 : 0; > >> > >> This certainly isn't right - iiuc the count would normally have changed > >> during the hash_foreach() above this. > > I don't think that's a problem. The expected count is still the same. The > shadow scan callback will not change the PGC_allocated flag in the target > page. I don't think there's any interlock stopping another CPU from changing PGC_allocated, though. > Further, the sh_rm_mappings_from_l1 should also be updated. It > breaks out of the loop on a count of zero, but it should also break out on > 1 | PGC_allocated. True. A helper function that does the snapshot-and-test and is called from all three places would be an improvement. > I'll resend this one. Thanks. Tim. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |