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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to