Re: [PATCH v2 2/2] KVM: Account mem_attr_array nodes to the caller's memcg

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional cond_resched() and blocking MMU invalidations are called inside an RCU read-side critical section in KVM memory attribute paths.
--

commit c6abdd494ec28371324dab717f8c1bf7e27a654d
Author: David Ballesteros <davimaba.v@proton.me>

KVM: Account mem_attr_array nodes to the caller's memcg

This patch sets XA_FLAGS_ACCOUNT for mem_attr_array so that nodes allocated
by xas_alloc() with GFP_NOWAIT are properly charged to the caller's memcg.
This ensures per-tenant memory limits are respected and bounds the growth
of host memory allocations.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f623c5f07344e..14ca73aedbaf9 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1127,7 +1127,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
>  	rcuwait_init(&kvm->mn_memslots_update_rcuwait);
>  	xa_init(&kvm->vcpu_array);
>  #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
> -	xa_init(&kvm->mem_attr_array);
> +	xa_init_flags(&kvm->mem_attr_array, XA_FLAGS_ACCOUNT);
>  	kvm->mem_attr_gfn_count = 0;
>  #endif

While reviewing this, I noticed some pre-existing regressions in the same file
related to the memory attribute paths:

virt/kvm/kvm_main.c:kvm_count_mem_attr_entries() {
    ...
    guard(rcu)();
    xas_for_each(&xas, entry, ULONG_MAX) {
        if (xa_is_retry(entry)) {
            xas_pause(&xas);
            continue;
        }
        if (entry)
            count++;

        if (need_resched()) {
            xas_pause(&xas);
            cond_resched();

[Severity: High]
This isn't a bug introduced by this patch, but does this call cond_resched()
inside the block-scoped RCU read-side section established by guard(rcu)()?

        }
    }
    ...
}


virt/kvm/kvm_main.c:kvm_vm_set_mem_attributes() {
    ...
    if (!attributes) {
        XA_STATE(xas, &kvm->mem_attr_array, start);

        kvm_handle_gfn_range(kvm, &pre_set_range);

        guard(rcu)();
        xas_for_each(&xas, entry, end - 1) {
            if (xa_is_retry(entry)) {
                xas_pause(&xas);
                continue;
            }
            xa_erase(&kvm->mem_attr_array, xas.xa_index);
            freed++;
            cond_resched();

[Severity: High]
This is a pre-existing issue, but would this unconditional cond_resched()
also execute within the RCU read lock scope established by guard(rcu)()?

        }

        kvm_handle_gfn_range(kvm, &post_set_range);

[Severity: High]
This is a pre-existing issue, but is a potentially blocking operation executed
via kvm_handle_gfn_range() (where may_block = true) before the block-scoped
guard(rcu)() ends?

        /* Exactly @freed entries left the array; no resync needed. */
        ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911184819.101123-1-davimaba.v@proton.me?part=2