Re: [GIT PULL v1 18/23] KVM: s390: cmma: Fix dirty tracking when removing memslot

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [Medium] Mixing of `goto`-based error handling with scope-based cleanups =
(`__free()` and `guard()`).
--

--- Patch [18]: [GIT PULL v1 18/23] KVM: s390: cmma: Fix dirty tracking whe=
n removing memslot ---
commit b4b4bb1a465e2a5c28c8cf71b60ac80548c5f396
Author: Claudio Imbrenda <[email protected]>

KVM: s390: cmma: Fix dirty tracking when removing memslot

This patch fixes CMMA dirty tracking when a memslot is removed during
migration mode. It also opportunistically updates the memory region commit
path to use the __free() cleanup helper for the MMU cache.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index e5c5e9f61cb24..ba811f0673d10 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
[ ... ]
>  void kvm_arch_commit_memory_region(struct kvm *kvm,
>  				struct kvm_memory_slot *old,
>  				const struct kvm_memory_slot *new,
>  				enum kvm_mr_change change)
>  {
> -	struct kvm_s390_mmu_cache *mc =3D NULL;
> +	const struct dat_walk_ops ops =3D { .pte_entry =3D cmma_d_count_pte, };
> +	struct kvm_s390_mmu_cache *mc __free(kvm_s390_mmu_cache) =3D NULL;
>  	int rc =3D 0;
> =20
> +	guard(mutex)(&kvm->slots_arch_lock);

[Severity: Medium]
Does this code mix goto-based error handling with scope-based cleanups?

According to the guidelines in include/linux/cleanup.h, functions should
convert all resources that need a goto cleanup to scope-based cleanup,
or convert none of them. This function introduces guard() and __free()
declarations here, but it still retains the `out:` label and the `goto out;`
error path when kvm_s390_new_mmu_cache() fails.

Also, is it possible to combine the __free() declaration and initialization=
 of
mc at the point of allocation? Declaring it at the top as =3D NULL and
assigning it later makes LIFO ordering mistakes more likely. Could the goto
path be replaced with a direct return, and the mc variable be defined where
it is first allocated?

[ ... ]
> @@ -5851,7 +5873,6 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
>  out:
>  	if (rc)
>  		pr_warn("failed to commit memory region\n");
> -	kvm_s390_free_mmu_cache(mc);
>  	return;
>  }

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803160924.2368=
[email protected]?part=3D18