[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAH1kMwSvLj8oK46V8m+FUM=t8h5Zch_Pi+zui+AYq6efDR0Sgw@mail.gmail.com>
Date: Mon, 15 Nov 2021 17:03:18 +0530
From: Vihas Mak <makvihas@...il.com>
To: Vitaly Kuznetsov <vkuznets@...hat.com>
Cc: pbonzini@...hat.com, seanjc@...gle.com, wanpengli@...cent.com,
jmattson@...gle.com, joro@...tes.org, tglx@...utronix.de,
mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
x86@...nel.org, hpa@...or.com, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: x86: fix cocci warnings
> and I find '|=' to not be very natural with booleans. I'm not sure it's
> worth changing though.
I see. But there are many functions in which '|=' is used on booleans.
get_mmio_spte(), __rmap_write_protect(), kvm_handle_gfn_range and many more.
That's why I thought it would be better if the code follows the same convention.
Thanks,
Vihas
On Mon, Nov 15, 2021 at 3:29 PM Vitaly Kuznetsov <vkuznets@...hat.com> wrote:
>
> Vihas Mak <makvihas@...il.com> writes:
>
> > change 0 to false and 1 to true to fix following cocci warnings:
> >
> > arch/x86/kvm/mmu/mmu.c:1485:9-10: WARNING: return of 0/1 in function 'kvm_set_pte_rmapp' with return type bool
> > arch/x86/kvm/mmu/mmu.c:1636:10-11: WARNING: return of 0/1 in function 'kvm_test_age_rmapp' with return type bool
> >
> > Signed-off-by: Vihas Mak <makvihas@...il.com>
> > Cc: Sean Christopherson <seanjc@...gle.com>
> > Cc: Vitaly Kuznetsov <vkuznets@...hat.com>
> > Cc: Wanpeng Li <wanpengli@...cent.com>
> > Cc: Jim Mattson <jmattson@...gle.com>
> > Cc: Joerg Roedel <joro@...tes.org>
> > ---
> > arch/x86/kvm/mmu/mmu.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 337943799..2fcea4a78 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -1454,7 +1454,7 @@ static bool kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
> > {
> > u64 *sptep;
> > struct rmap_iterator iter;
> > - int need_flush = 0;
> > + bool need_flush = false;
> > u64 new_spte;
> > kvm_pfn_t new_pfn;
> >
> > @@ -1466,7 +1466,7 @@ static bool kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
> > rmap_printk("spte %p %llx gfn %llx (%d)\n",
> > sptep, *sptep, gfn, level);
> >
> > - need_flush = 1;
> > + need_flush = true;
> >
> > if (pte_write(pte)) {
> > pte_list_remove(kvm, rmap_head, sptep);
> > @@ -1482,7 +1482,7 @@ static bool kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
> >
> > if (need_flush && kvm_available_flush_tlb_with_range()) {
> > kvm_flush_remote_tlbs_with_address(kvm, gfn, 1);
> > - return 0;
> > + return false;
> > }
> >
> > return need_flush;
> > @@ -1623,8 +1623,8 @@ static bool kvm_test_age_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
> >
> > for_each_rmap_spte(rmap_head, &iter, sptep)
> > if (is_accessed_spte(*sptep))
> > - return 1;
> > - return 0;
> > + return true;
> > + return false;
> > }
> >
> > #define RMAP_RECYCLE_THRESHOLD 1000
>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@...hat.com>
>
> One minor remark: 'kvm_set_pte_rmapp()' handler is passed to
> 'kvm_handle_gfn_range()' which does
>
> bool ret = false;
>
> for_each_slot_rmap_range(...)
> ret |= handler(...);
>
> and I find '|=' to not be very natural with booleans. I'm not sure it's
> worth changing though.
>
> --
> Vitaly
>
Powered by blists - more mailing lists