On 02.10.19 10:20, David Hildenbrand wrote:
> On 02.10.19 10:07, Thomas Huth wrote:
>> 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?
>
> Ah, makes sense.
>
>>
>>> Instead of the two manual calls we could also call kvm_arch_exit().
>>
>> Huh, isn't that what this patch is doing here?
>
> Lol, still tired and thought only the two labels would get removed. Even
> better :)
So I guess we should not take your Reviewed-by: then? ;-)
>
>>
>> To me, the patch is looking fine, so
>> Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
>>
>
>
