RE: [PATCH v2 RESEND 2/4] Drivers: hv: balloon: account for gaps in hot add regions

[Alex Ng (LIS)]

> >>  static unsigned long handle_pg_range(unsigned long pg_start, @@ -
> 834,13
> >> +881,19 @@ static unsigned long process_hot_add(unsigned long
> pg_start,
> >>  					unsigned long rg_size)
> >>  {
> >>  	struct hv_hotadd_state *ha_region = NULL;
> >> +	int covered;
> >>
> >>  	if (pfn_cnt == 0)
> >>  		return 0;
> >>
> >> -	if (!dm_device.host_specified_ha_region)
> >> -		if (pfn_covered(pg_start, pfn_cnt))
> >> +	if (!dm_device.host_specified_ha_region) {
> >> +		covered = pfn_covered(pg_start, pfn_cnt);
> >> +		if (covered < 0)
> >> +			return 0;
> >
> > If the hot-add pages aren't covered by any region, then shouldn't it fall
> through instead of returning?
> > That way the new ha_region can be added to the list and we hot-add the
> > pages accordingly.
> 
> I was under an impression this is impossible:
> hot_add_req()/process_hot_add() will create a new region in this
> case. 'covered < 0' was added to handle one particular error: failure to
> allocate memory to record gap (struct hv_hotadd_gap) and I don't have a
> better idea how to handle this: if we can't remember the gap we'll crash
> later on onlining...
> 

You are correct. I misread your patch thinking "covered < 0" means the page range is not covered; when it is actually handling an error case.

> --
>   Vitaly