Re: [PATCH v2 5/5] KVM: s390: vsie: Use mmu cache to allocate rmap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Infinite loop under memory pressure due to MMU cache depletion during shadow fault retries.
--

commit 3403f763f43a4ac3038b79807dbaad26fc1a966a
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: vsie: Use mmu cache to allocate rmap

The patch modifies gmap_insert_rmap() to allocate rmaps using
kvm_s390_mmu_cache_alloc_rmap() rather than kzalloc_obj() with GFP_ATOMIC.
This aims to ensure forward progress by leveraging pre-allocated cache items.

> diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
> index 52d55ddea8d4f..1d289f8fa3b20 100644
> --- a/arch/s390/kvm/gmap.c
> +++ b/arch/s390/kvm/gmap.c
> @@ -1000,7 +1000,8 @@ int gmap_pv_destroy_range(struct gmap *gmap, gfn_t start, gfn_t end, bool interr
>  	return 0;
>  }
>  
> -int gmap_insert_rmap(struct gmap *sg, gfn_t p_gfn, gfn_t r_gfn, int level)
> +int gmap_insert_rmap(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gfn_t p_gfn,
> +		     gfn_t r_gfn, int level)
>  {
>  	struct vsie_rmap *rmap __free(kvfree) = NULL;
>  	struct vsie_rmap *temp;
> @@ -1010,7 +1011,7 @@ int gmap_insert_rmap(struct gmap *sg, gfn_t p_gfn, gfn_t r_gfn, int level)
>  	KVM_BUG_ON(!is_shadow(sg), sg->kvm);
>  	lockdep_assert_held(&sg->host_to_rmap_lock);
>  
> -	rmap = kzalloc_obj(*rmap, GFP_ATOMIC);
> +	rmap = kvm_s390_mmu_cache_alloc_rmap(mc);

[Severity: High]
Does this code leak the pre-allocated cache items during retries?

If gmap_insert_rmap() finds a duplicate mapping, it returns 0 without returning
the allocated rmap to the MMU cache. Instead, the __free(kvfree) cleanup
attribute returns it to the generic slab allocator:

arch/s390/kvm/gmap.c:gmap_insert_rmap() {
	...
	rmap = kvm_s390_mmu_cache_alloc_rmap(mc);
	if (!rmap)
		return -ENOMEM;

	rmap->r_gfn = r_gfn;
	...
	if (slot) {
		rmap->next = radix_tree_deref_slot_protected(slot, &sg->host_to_rmap_lock);
		for (temp = rmap->next; temp; temp = temp->next) {
			if (temp->val == rmap->val)
				return 0; // rmap freed via __free(kvfree)
	...
}

In the retry loop inside _gaccess_shadow_fault(), memory allocation failures
trigger a retry:

	do {
		radix_tree_preload(GFP_KERNEL);
		scoped_guard(read_lock, &vcpu->kvm->mmu_lock)
			rc = ___gaccess_shadow_fault(vcpu, sg, saddr, seq, walk);
		radix_tree_preload_end();
	} while (rc == -ENOMEM);

The MMU cache is only topped up outside of this loop. Because the pre-allocated
cache items can be quickly depleted across partial retries and freed to the slab
allocator rather than the cache, kvm_s390_mmu_cache_alloc_rmap() will fall back
to kzalloc_obj() with GFP_ATOMIC.

Under memory pressure, this atomic allocation will consistently fail, returning
-ENOMEM. Since the loop never yields, reclaims memory, or re-tops the cache,
could this create a deterministic infinite loop, leading to a soft lockup?

>  	if (!rmap)
>  		return -ENOMEM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610165247.238366-1-imbrenda@linux.ibm.com?part=5