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

From	"Alex Ng (LIS)" <alexng@microsoft.com>
Newsgroups	linux.kernel
Subject	RE: [PATCH v2 RESEND 2/4] Drivers: hv: balloon: account for gaps in hot add regions
Date	2016-08-16 23:50 +0200
Message-ID	<s6Upb-8vn-7@gated-at.bofh.it> (permalink)
References	<s4XAS-3T5-5@gated-at.bofh.it> <s4XAS-3T5-35@gated-at.bofh.it> <s6yI1-3bM-1@gated-at.bofh.it> <s6Iem-Nx-11@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

> >>  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

Thread

[PATCH v2 RESEND 0/4] Drivers: hv: balloon: fix WS2012 memory hotplug issues and do some cleanup Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-08-11 14:50 +0200
  [PATCH v2 RESEND 3/4] Drivers: hv: balloon: don't wait for ol_waitevent when memhp_auto_online is enabled Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-08-11 14:50 +0200
  [PATCH v2 RESEND 2/4] Drivers: hv: balloon: account for gaps in hot add regions Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-08-11 14:50 +0200
    RE: [PATCH v2 RESEND 2/4] Drivers: hv: balloon: account for gaps in  hot add regions "Alex Ng (LIS)" <alexng@microsoft.com> - 2016-08-16 00:40 +0200
      Re: [PATCH v2 RESEND 2/4] Drivers: hv: balloon: account for gaps in hot add regions Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-08-16 10:50 +0200
        RE: [PATCH v2 RESEND 2/4] Drivers: hv: balloon: account for gaps in  hot add regions "Alex Ng (LIS)" <alexng@microsoft.com> - 2016-08-16 23:50 +0200
  [PATCH v2 RESEND 4/4] Drivers: hv: balloon: replace ha_region_mutex with spinlock Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-08-11 14:50 +0200
  [PATCH v2 RESEND 1/4] Drivers: hv: balloon: keep track of where ha_region starts Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-08-11 15:00 +0200

csiph-web