On 02/10/2019 10.01, David Hildenbrand wrote:
> On 02.10.19 09:56, Janosch Frank wrote:
>> Both kvm_s390_gib_destroy and debug_unregister test if the needed
>> pointers are not NULL and hence can be called unconditionally.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
>> arch/s390/kvm/kvm-s390.c | 18 +++++++-----------
>> 1 file changed, 7 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 895fb2006c0d..66720d69cd24 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -458,16 +458,14 @@ static void kvm_s390_cpu_feat_init(void)
>>
>> int kvm_arch_init(void *opaque)
>> {
>> - int rc;
>> + int rc = -ENOMEM;
>>
>> kvm_s390_dbf = debug_register("kvm-trace", 32, 1, 7 * sizeof(long));
>> if (!kvm_s390_dbf)
>> return -ENOMEM;
>>
>> - if (debug_register_view(kvm_s390_dbf, &debug_sprintf_view)) {
>> - rc = -ENOMEM;
>> - goto out_debug_unreg;
>> - }
>> + if (debug_register_view(kvm_s390_dbf, &debug_sprintf_view))
>> + goto out;
>>
>> kvm_s390_cpu_feat_init();
>>
>> @@ -475,19 +473,17 @@ int kvm_arch_init(void *opaque)
>> rc = kvm_register_device_ops(&kvm_flic_ops, KVM_DEV_TYPE_FLIC);
>> if (rc) {
>> pr_err("A FLIC registration call failed with rc=%d\n", rc);
>> - goto out_debug_unreg;
>> + goto out;
>> }
>>
>> rc = kvm_s390_gib_init(GAL_ISC);
>> if (rc)
>> - goto out_gib_destroy;
>> + goto out;
>>
>> return 0;
>>
>> -out_gib_destroy:
>> - kvm_s390_gib_destroy();
>> -out_debug_unreg:
>> - debug_unregister(kvm_s390_dbf);
>> +out:
>> + kvm_arch_exit();
>> return rc;
>> }
>
> Wonder why "debug_info_t *kvm_s390_dbf" is not declared as static.
Because it is used in the KVM_EVENT macro?
> Instead of the two manual calls we could also call kvm_arch_exit().
Huh, isn't that what this patch is doing here?
To me, the patch is looking fine, so
Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
