Re: [PATCH v5] Drivers: hv: Cosmetic changes for hv.c and balloon.c

[Wei Liu]

On Wed, May 29, 2024 at 04:29:17PM +0000, Michael Kelley wrote:
> From: Aditya Nagesh <adityanagesh@xxxxxxxxxxxxxxxxxxx> Sent: Wednesday, May 29, 2024 9:05 AM
> > 
> > Fix issues reported by checkpatch.pl script in hv.c and
> > balloon.c
> >  - Remove unnecessary parentheses
> >  - Remove extra newlines
> >  - Remove extra spaces
> >  - Add spaces between comparison operators
> >  - Remove comparison with NULL in if statements
> > 
> > No functional changes intended
> > 
> > Signed-off-by: Aditya Nagesh <adityanagesh@xxxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
> > ---
> > [V5]
> > Rebase to hyperv-fixes
> > 
> > [V4]
> > Fix Alignment issue and revert a line since 100 characters are allowed in a line
> > 
> > [V3]
> > Fix alignment issues in multiline function parameters.
> > 
> > [V2]
> > Change Subject from "Drivers: hv: Fix Issues reported by checkpatch.pl script"
> >  to "Drivers: hv: Cosmetic changes for hv.c and balloon.c"
> >  drivers/hv/hv.c         |  37 +++++++-------
> >  drivers/hv/hv_balloon.c | 105 ++++++++++++++--------------------------
> >  2 files changed, 53 insertions(+), 89 deletions(-)
> > 
> 
> [snip]
> 
> > @@ -999,21 +984,14 @@ static void hot_add_req(struct work_struct *dummy)
> >  	rg_start = dm->ha_wrk.ha_region_range.finfo.start_page;
> >  	rg_sz = dm->ha_wrk.ha_region_range.finfo.page_cnt;
> > 
> > -	if ((rg_start == 0) && (!dm->host_specified_ha_region)) {
> > +	if (rg_start == 0 && !dm->host_specified_ha_region) {
> >  		/*
> > -		 * The host has not specified the hot-add region.
> >  		 * Based on the hot-add page range being specified,
> > -		 * compute a hot-add region that can cover the pages
> > -		 * that need to be hot-added while ensuring the alignment
> > -		 * and size requirements of Linux as it relates to hot-add.
> > -		 */
> > -		rg_start = ALIGN_DOWN(pg_start, ha_pages_in_chunk);
> > -		rg_sz = ALIGN(pfn_cnt, ha_pages_in_chunk);
> 
> Hmmm.  The above is not a cosmetic change.  Looks like this
> delta was erroneously introduced in the v5 version.  It wasn't
> there in v4.

This also breaks the build, since now the comment is not terminated.

Please fix this and resubmit.

In general, you should always build test your code before submission.

Thanks,
Wei.

> 
> Everything else LGTM.
> 
> Michael