Hello Sean,
> NAK, the comparison is necessary as kvm_tdp_mmu_zap_leafs() deliberately invokes
> for_each_valid_tdp_mmu_root_yi
> eld_safe() => __for_each_tdp_mmu_root_yield_safe()
>with -1 to iterate over all address spaces.
> And I don't want to drop the check for __for_each_tdp_mmu_root(), even though
> there aren't any _current_ users that pass -1, as I want to keep the iterators
> symmetrical.
Understood, thanks for the feedback.
Best regards,
Advait
On Tue, 12 Nov 2024 at 00:26, Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Tue, Nov 12, 2024, Advait Dhamorikar wrote:
> > An unsigned value can never be negative,
> > so this test will always evaluate the same way.
> > `_as_id` a u16 is compared to 0.
>
> Please wrap changelogs at ~75 characters.
>
> > Signed-off-by: Advait Dhamorikar <advaitdhamorikar@xxxxxxxxx>
> > ---
> > arch/x86/kvm/mmu/tdp_mmu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index 4508d868f1cd..b4e7b6a264d6 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -153,7 +153,7 @@ static struct kvm_mmu_page *tdp_mmu_next_root(struct kvm *kvm,
> > for (_root = tdp_mmu_next_root(_kvm, NULL, _only_valid); \
> > ({ lockdep_assert_held(&(_kvm)->mmu_lock); }), _root; \
> > _root = tdp_mmu_next_root(_kvm, _root, _only_valid)) \
> > - if (_as_id >= 0 && kvm_mmu_page_as_id(_root) != _as_id) { \
> > + if (kvm_mmu_page_as_id(_root) != _as_id) { \
>
> NAK, the comparison is necessary as kvm_tdp_mmu_zap_leafs() deliberately invokes
> for_each_valid_tdp_mmu_root_yield_safe() => __for_each_tdp_mmu_root_yield_safe()
> with -1 to iterate over all address spaces.
>
> And I don't want to drop the check for __for_each_tdp_mmu_root(), even though
> there aren't any _current_ users that pass -1, as I want to keep the iterators
> symmetrical.
