Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 4/6] drivers: hv: vmbus: Fix checkpatch SPLIT_STRING
To:     Michael Kelley <mikelley@microsoft.com>
Cc:     "sashal@kernel.org" <sashal@kernel.org>,
        Tianyu Lan <Tianyu.Lan@microsoft.com>,
        Dexuan Cui <decui@microsoft.com>,
        Sunil Muthuswamy <sunilmut@microsoft.com>,
        "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        KY Srinivasan <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        "wei.liu@kernel.org" <wei.liu@kernel.org>,
        Stephen Hemminger <sthemmin@microsoft.com>
References: <20201115195734.8338-1-matheus@castello.eng.br>
 <20201115195734.8338-5-matheus@castello.eng.br>
 <MW2PR2101MB1052B329DFC5D54F4D7501E9D7E40@MW2PR2101MB1052.namprd21.prod.outlook.com>
From:   Matheus Castello <matheus@castello.eng.br>
Message-ID: <ada0ef93-1443-49a0-914c-1ad03ffa024b@castello.eng.br>
Date:   Tue, 24 Nov 2020 21:54:29 -0300
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.3
MIME-Version: 1.0
In-Reply-To: <MW2PR2101MB1052B329DFC5D54F4D7501E9D7E40@MW2PR2101MB1052.namprd21.prod.outlook.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: pt-BR
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Michael,

Em 11/15/2020 7:25 PM, Michael Kelley escreveu:
> From: Matheus Castello <matheus@castello.eng.br> Sent: Sunday, November 15, 2020 11:58 AM
>>
>> Checkpatch emits WARNING: quoted string split across lines.
>> To keep the code clean and with the 80 column length indentation the
>> check and registration code for kmsg_dump_register has been transferred
>> to a new function hv_kmsg_dump_register.
>>
>> Signed-off-by: Matheus Castello <matheus@castello.eng.br>
>> ---
>>   drivers/hv/vmbus_drv.c | 34 +++++++++++++++++++---------------
>>   1 file changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>> index 61d28c743263..09d8236a51cf 100644
>> --- a/drivers/hv/vmbus_drv.c
>> +++ b/drivers/hv/vmbus_drv.c
>> @@ -1387,6 +1387,23 @@ static struct kmsg_dumper hv_kmsg_dumper = {
>>   	.dump = hv_kmsg_dump,
>>   };
>>
>> +static void hv_kmsg_dump_register(void)
>> +{
>> +	int ret;
>> +
>> +	hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
>> +	if (hv_panic_page) {
>> +		ret = kmsg_dump_register(&hv_kmsg_dumper);
>> +		if (ret) {
>> +			pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
>> +			hv_free_hyperv_page((unsigned long)hv_panic_page);
>> +			hv_panic_page = NULL;
>> +		}
>> +	} else {
>> +		pr_err("Hyper-V: panic message page memory allocation failed");
>> +	}
>> +}
>> +
> 
> The above would be marginally better if organized as follows so that the
> main execution path isn't in an "if" clause.  Also reduces indentation.
> 
> 	hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> 	if (!hv_panic_page) {
> 		pr_err("Hyper-V: panic message page memory allocation failed");
> 		return;
> 	}
> 	ret = kmsg_dump_register(&hv_kmsg_dumper);
> 	if (ret) {
> 		pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
> 		hv_free_hyperv_page((unsigned long)hv_panic_page);
> 		hv_panic_page = NULL;
> 	}
> 
> 

Thanks for the review, great I will use it on the v2.

>>   static struct ctl_table_header *hv_ctl_table_hdr;
>>
>>   /*
>> @@ -1477,21 +1494,8 @@ static int vmbus_bus_init(void)
>>   		 * capability is supported by the hypervisor.
>>   		 */
>>   		hv_get_crash_ctl(hyperv_crash_ctl);
>> -		if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) {
>> -			hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
>> -			if (hv_panic_page) {
>> -				ret = kmsg_dump_register(&hv_kmsg_dumper);
>> -				if (ret) {
>> -					pr_err("Hyper-V: kmsg dump register "
>> -						"error 0x%x\n", ret);
>> -					hv_free_hyperv_page(
>> -					    (unsigned long)hv_panic_page);
>> -					hv_panic_page = NULL;
>> -				}
>> -			} else
>> -				pr_err("Hyper-V: panic message page memory "
>> -					"allocation failed");
>> -		}
>> +		if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG)
>> +			hv_kmsg_dump_register();
>>
>>   		register_die_notifier(&hyperv_die_block);
>>   	}
>> --
>> 2.28.0
>