| Message ID | 20181009024116.48603-3-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Trivial code refine for kvm_main.c | expand |
On 09/10/2018 04:41, Wei Yang wrote: > Current implementation writes *false* to @writable in three different > places. Since @writable is an indicate for caller and it will be > overwritten in hva_to_pfn(), it is save to write *false* at the beginning. > > After doing so, we could > > * collapse two error hva case > * remove the check on @writable if memslot_is_readonly() > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > --- > virt/kvm/kvm_main.c | 18 +++++++----------- > 1 file changed, 7 insertions(+), 11 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 660e2e7d382f..4b01e19a9b81 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -1539,23 +1539,19 @@ kvm_pfn_t __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn, > { > unsigned long addr = __gfn_to_hva_many(slot, gfn, NULL, write_fault); > > - if (addr == KVM_HVA_ERR_RO_BAD) { > - if (writable) > - *writable = false; > - return KVM_PFN_ERR_RO_FAULT; > - } > + if (writable) > + *writable = false; > > if (kvm_is_error_hva(addr)) { > - if (writable) > - *writable = false; > - return KVM_PFN_NOSLOT; > + if (addr == KVM_HVA_ERR_RO_BAD) > + return KVM_PFN_ERR_RO_FAULT; > + else > + return KVM_PFN_NOSLOT; > } > > /* Do not map writable pfn in the readonly memslot. */ > - if (writable && memslot_is_readonly(slot)) { > - *writable = false; > + if (memslot_is_readonly(slot)) > writable = NULL; > - } > > return hva_to_pfn(addr, atomic, async, write_fault, writable); > } > I don't find this to be more readable. I've queued patch 1. Paolo
On Mon, Oct 15, 2018 at 07:35:02PM +0200, Paolo Bonzini wrote: >On 09/10/2018 04:41, Wei Yang wrote: >> Current implementation writes *false* to @writable in three different >> places. Since @writable is an indicate for caller and it will be >> overwritten in hva_to_pfn(), it is save to write *false* at the beginning. >> >> After doing so, we could >> >> * collapse two error hva case >> * remove the check on @writable if memslot_is_readonly() >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> --- >> virt/kvm/kvm_main.c | 18 +++++++----------- >> 1 file changed, 7 insertions(+), 11 deletions(-) >> >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index 660e2e7d382f..4b01e19a9b81 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -1539,23 +1539,19 @@ kvm_pfn_t __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn, >> { >> unsigned long addr = __gfn_to_hva_many(slot, gfn, NULL, write_fault); >> >> - if (addr == KVM_HVA_ERR_RO_BAD) { >> - if (writable) >> - *writable = false; >> - return KVM_PFN_ERR_RO_FAULT; >> - } >> + if (writable) >> + *writable = false; >> >> if (kvm_is_error_hva(addr)) { >> - if (writable) >> - *writable = false; >> - return KVM_PFN_NOSLOT; >> + if (addr == KVM_HVA_ERR_RO_BAD) >> + return KVM_PFN_ERR_RO_FAULT; >> + else >> + return KVM_PFN_NOSLOT; >> } >> >> /* Do not map writable pfn in the readonly memslot. */ >> - if (writable && memslot_is_readonly(slot)) { >> - *writable = false; >> + if (memslot_is_readonly(slot)) >> writable = NULL; >> - } >> >> return hva_to_pfn(addr, atomic, async, write_fault, writable); >> } >> > >I don't find this to be more readable. I've queued patch 1. Ah, my bad taste of code. Thanks for your comment. > >Paolo
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 660e2e7d382f..4b01e19a9b81 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1539,23 +1539,19 @@ kvm_pfn_t __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn, { unsigned long addr = __gfn_to_hva_many(slot, gfn, NULL, write_fault); - if (addr == KVM_HVA_ERR_RO_BAD) { - if (writable) - *writable = false; - return KVM_PFN_ERR_RO_FAULT; - } + if (writable) + *writable = false; if (kvm_is_error_hva(addr)) { - if (writable) - *writable = false; - return KVM_PFN_NOSLOT; + if (addr == KVM_HVA_ERR_RO_BAD) + return KVM_PFN_ERR_RO_FAULT; + else + return KVM_PFN_NOSLOT; } /* Do not map writable pfn in the readonly memslot. */ - if (writable && memslot_is_readonly(slot)) { - *writable = false; + if (memslot_is_readonly(slot)) writable = NULL; - } return hva_to_pfn(addr, atomic, async, write_fault, writable); }
Current implementation writes *false* to @writable in three different places. Since @writable is an indicate for caller and it will be overwritten in hva_to_pfn(), it is save to write *false* at the beginning. After doing so, we could * collapse two error hva case * remove the check on @writable if memslot_is_readonly() Signed-off-by: Wei Yang <richard.weiyang@gmail.com> --- virt/kvm/kvm_main.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)