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;
Date:   Sun, 29 Nov 2020 21:43:28 -0800
From:   Stephen Hemminger <stephen@networkplumber.org>
To:     "Michael Kelley" <mikelley@microsoft.com>
Cc:     "Matheus Castello" <matheus@castello.eng.br>, <wei.liu@kernel.org>,
        <joe@perches.com>, "KY Srinivasan" <kys@microsoft.com>,
        "Haiyang Zhang" <haiyangz@microsoft.com>,
        "Stephen Hemminger" <sthemmin@microsoft.com>, <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-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 4/6] drivers: hv: vmbus: Fix checkpatch SPLIT_STRING
Message-ID: <20201129214328.254372dd@hermes.local>
In-Reply-To: <MW2PR2101MB105245D2BF9C9B5995D0188ED7F61@MW2PR2101MB1052.namprd21.prod.outlook.com>
References: <20201125032926.17002-1-matheus@castello.eng.br>
        <MW2PR2101MB105245D2BF9C9B5995D0188ED7F61@MW2PR2101MB1052.namprd21.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sun, 29 Nov 2020 08:51:47 -0800
"Michael Kelley" <mikelley@microsoft.com> wrote:

> From: Matheus Castello <matheus@castello.eng.br> Sent: Tuesday, November
> 24, 2020 7:29 PM
> > 
> > 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>
> > ---
> > This is the V3 of patch 4 of the series "Add improvements suggested by
> > checkpatch for vmbus_drv" with the changes suggested by Michael Kelley  
> and
> > Joe Perches. Thanks!
> > ---
> >  drivers/hv/vmbus_drv.c | 35 ++++++++++++++++++++---------------
> >  1 file changed, 20 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 61d28c743263..edcc419ba328 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -1387,6 +1387,24 @@ 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();

I know you just copy/pasted the original code, but one other thing.

Doesn't it already return void *?
arch/x86/include/asm/mshyperv.h:void *hv_alloc_hyperv_zeroed_page(void);

> > +	if (!hv_panic_page) {
> > +		pr_err("Hyper-V: panic message page memory allocation  
> failed\n");
> > +		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;
> > +	}
> > +}
> > +
> >  static struct ctl_table_header *hv_ctl_table_hdr;
> > 
> >  /*
> > @@ -1477,21 +1495,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.29.2  
> 
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>