On Fri, Oct 04, 2013 at 08:25:31PM +0530, Bharat Bhushan wrote: > lookup_linux_pte() was searching for a pte and also sets access > flags is writable. This function now searches only pte while > access flag setting is done explicitly. So in order to reduce some code duplication, you have added code duplication in the existing callers of this function. I'm not convinced it's an overall win. What's left in this function is pretty trivial, just a call to find_linux_pte_or_hugepte() and some pagesize computations. I would prefer you found a way to do what you want without adding code duplication at the existing call sites. Maybe you could have a new find_linux_pte_and_check_pagesize() and call that from the existing lookup_linux_pte(). The other thing you've done, without commenting on why you have done it, is to add a pte_present check without having looked at _PAGE_BUSY. kvmppc_read_update_linux_pte() only checks _PAGE_PRESENT after checking that _PAGE_BUSY is clear, so this is a semantic change, which I think is wrong for server processors. So, on the whole, NACK from me for this patch. Paul. -- 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