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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux