Netback vif reference count mismatching in latest 3.11 kernels

Netback vif reference count mismatching in latest 3.11 kernels

[Tomasz Wroblewski]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

Hi,

After update of our network backend vm kernel to 3.11.9 I'm seeing trouble with netback vif close which seem related to the recent changes 
which separated vif disconnect and free; It seems that now multiple disconnect/connect cycles can happen without freeing and reallocing the 
netdev in the processes, which confuses the vif refcount.

vif refcount is initialized to 1 in xenvif_alloc. Then first xenvif_disconnect brings it back to 0, instead of 1 which would seem more 
reasonable (since its initialized to 1 in xenvif_alloc i would expect it to not be dropped to 0 until xenvif_free). Second xenvif_disconnect 
brings it to -1 and hangs. For us (xenclient XT) this happens when we hibernate linux guest, since linux hibernate is a complex beast which 
transitions the drivers to between close/connected states multiple times (i.e. first it suspends/closes the drivers to take memory snapshot, 
then resumes/reconnects the drivers to the actual writing of hibernate image to disk, then finally it closes them again to shutdown the system)

I've hacked the attached patch which fixes it (for us), is the approach taken there correct/upstreamable/reasonable? It does the following

     * reset tx_irq to 0 after unbinding the irqs on disconnect - because xenvif_connect tests for it being 0 and will not reconnect if it's 
not reset
     * reacquire one reference to vif in disconnect(). This is because the reference
       vif should be 1, as initialized in xenvif_alloc(), until the vif is freed. Otherwise multiple disconne
       and cause a hang. I imagine alternate way of fixing this could be to use "0" as the default
       refcnt in xenvif_alloc()

I believe we didn't experience this issue on previous kernel because vif disconnect was also freeing the vif and netdev, hence it was not 
possible to get xenvif_connect/xenvif_disconnect called multiple times between vif alloc/free.







[-- Attachment #2: netback-refcnt-disconnect-fix --]
[-- Type: text/plain, Size: 725 bytes --]

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 68d5102..ccb46c4 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -420,6 +420,8 @@ void xenvif_disconnect(struct xenvif *vif)
 
 	atomic_dec(&vif->refcnt);
 	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
+	/* reacquire reference since it should be 1 until freed */
+	xenvif_get(vif);
 
 	if (vif->tx_irq) {
 		if (vif->tx_irq == vif->rx_irq)
@@ -428,6 +430,7 @@ void xenvif_disconnect(struct xenvif *vif)
 			unbind_from_irqhandler(vif->tx_irq, vif);
 			unbind_from_irqhandler(vif->rx_irq, vif);
 		}
+		vif->tx_irq = 0;
 	}
 
 	xen_netbk_unmap_frontend_rings(vif);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Wei Liu]

On Wed, Nov 27, 2013 at 03:07:09PM +0100, Tomasz Wroblewski wrote:
> Hi,
> 
> After update of our network backend vm kernel to 3.11.9 I'm seeing
> trouble with netback vif close which seem related to the recent
> changes which separated vif disconnect and free; It seems that now
> multiple disconnect/connect cycles can happen without freeing and
> reallocing the netdev in the processes, which confuses the vif
> refcount.
> 
> vif refcount is initialized to 1 in xenvif_alloc. Then first
> xenvif_disconnect brings it back to 0, instead of 1 which would seem
> more reasonable (since its initialized to 1 in xenvif_alloc i would
> expect it to not be dropped to 0 until xenvif_free). Second
> xenvif_disconnect brings it to -1 and hangs. For us (xenclient XT)
> this happens when we hibernate linux guest, since linux hibernate is
> a complex beast which transitions the drivers to between
> close/connected states multiple times (i.e. first it suspends/closes
> the drivers to take memory snapshot, then resumes/reconnects the
> drivers to the actual writing of hibernate image to disk, then
> finally it closes them again to shutdown the system)
> 

Can you illustrate a graph of the whole process? I'm not very clear of
the whole cycle.

There's a xenvif_get in xenvif_connect, which increases refcnt by 1,
that should corresponds to the atomic_dec in xenvif_disconnect, right?

> I've hacked the attached patch which fixes it (for us), is the approach taken there correct/upstreamable/reasonable? It does the following
> 
>     * reset tx_irq to 0 after unbinding the irqs on disconnect -
> because xenvif_connect tests for it being 0 and will not reconnect
> if it's not reset
>     * reacquire one reference to vif in disconnect(). This is because the reference
>       vif should be 1, as initialized in xenvif_alloc(), until the vif is freed. Otherwise multiple disconne
>       and cause a hang. I imagine alternate way of fixing this could be to use "0" as the default
>       refcnt in xenvif_alloc()
> 

You mean the numbers of connect's and disconnect's don't match? Even
after you reset tx_irq to 0?

Wei.

> I believe we didn't experience this issue on previous kernel because
> vif disconnect was also freeing the vif and netdev, hence it was not
> possible to get xenvif_connect/xenvif_disconnect called multiple
> times between vif alloc/free.
> 
> 
> 
> 
> 
> 

> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 68d5102..ccb46c4 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -420,6 +420,8 @@ void xenvif_disconnect(struct xenvif *vif)
>  
>  	atomic_dec(&vif->refcnt);
>  	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
> +	/* reacquire reference since it should be 1 until freed */
> +	xenvif_get(vif);
>  
>  	if (vif->tx_irq) {
>  		if (vif->tx_irq == vif->rx_irq)
> @@ -428,6 +430,7 @@ void xenvif_disconnect(struct xenvif *vif)
>  			unbind_from_irqhandler(vif->tx_irq, vif);
>  			unbind_from_irqhandler(vif->rx_irq, vif);
>  		}
> +		vif->tx_irq = 0;
>  	}
>  
>  	xen_netbk_unmap_frontend_rings(vif);

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Paul Durrant]

> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Wei Liu
> Sent: 27 November 2013 14:41
> To: Tomasz Wroblewski
> Cc: xen-devel; Paul Durrant; Wei Liu; David Vrabel
> Subject: Re: [Xen-devel] Netback vif reference count mismatching in latest
> 3.11 kernels
> 
> On Wed, Nov 27, 2013 at 03:07:09PM +0100, Tomasz Wroblewski wrote:
> > Hi,
> >
> > After update of our network backend vm kernel to 3.11.9 I'm seeing
> > trouble with netback vif close which seem related to the recent
> > changes which separated vif disconnect and free; It seems that now
> > multiple disconnect/connect cycles can happen without freeing and
> > reallocing the netdev in the processes, which confuses the vif
> > refcount.
> >
> > vif refcount is initialized to 1 in xenvif_alloc. Then first
> > xenvif_disconnect brings it back to 0, instead of 1 which would seem
> > more reasonable (since its initialized to 1 in xenvif_alloc i would
> > expect it to not be dropped to 0 until xenvif_free). Second
> > xenvif_disconnect brings it to -1 and hangs. For us (xenclient XT)
> > this happens when we hibernate linux guest, since linux hibernate is
> > a complex beast which transitions the drivers to between
> > close/connected states multiple times (i.e. first it suspends/closes
> > the drivers to take memory snapshot, then resumes/reconnects the
> > drivers to the actual writing of hibernate image to disk, then
> > finally it closes them again to shutdown the system)
> >
> 
> Can you illustrate a graph of the whole process? I'm not very clear of
> the whole cycle.
> 
> There's a xenvif_get in xenvif_connect, which increases refcnt by 1,
> that should corresponds to the atomic_dec in xenvif_disconnect, right?
> 

ISTR this happening with some frontends that did not drive the state model entirely correctly. Commit ea732dff5cfa10789007bf4a5b935388a0bb2a8f (Handle backend state transitions in a more robust way ) should fix this.

  Paul

> > I've hacked the attached patch which fixes it (for us), is the approach taken
> there correct/upstreamable/reasonable? It does the following
> >
> >     * reset tx_irq to 0 after unbinding the irqs on disconnect -
> > because xenvif_connect tests for it being 0 and will not reconnect
> > if it's not reset
> >     * reacquire one reference to vif in disconnect(). This is because the
> reference
> >       vif should be 1, as initialized in xenvif_alloc(), until the vif is freed.
> Otherwise multiple disconne
> >       and cause a hang. I imagine alternate way of fixing this could be to use
> "0" as the default
> >       refcnt in xenvif_alloc()
> >
> 
> You mean the numbers of connect's and disconnect's don't match? Even
> after you reset tx_irq to 0?
> 
> Wei.
> 
> > I believe we didn't experience this issue on previous kernel because
> > vif disconnect was also freeing the vif and netdev, hence it was not
> > possible to get xenvif_connect/xenvif_disconnect called multiple
> > times between vif alloc/free.
> >
> >
> >
> >
> >
> >
> 
> > diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> netback/interface.c
> > index 68d5102..ccb46c4 100644
> > --- a/drivers/net/xen-netback/interface.c
> > +++ b/drivers/net/xen-netback/interface.c
> > @@ -420,6 +420,8 @@ void xenvif_disconnect(struct xenvif *vif)
> >
> >  	atomic_dec(&vif->refcnt);
> >  	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
> > +	/* reacquire reference since it should be 1 until freed */
> > +	xenvif_get(vif);
> >
> >  	if (vif->tx_irq) {
> >  		if (vif->tx_irq == vif->rx_irq)
> > @@ -428,6 +430,7 @@ void xenvif_disconnect(struct xenvif *vif)
> >  			unbind_from_irqhandler(vif->tx_irq, vif);
> >  			unbind_from_irqhandler(vif->rx_irq, vif);
> >  		}
> > +		vif->tx_irq = 0;
> >  	}
> >
> >  	xen_netbk_unmap_frontend_rings(vif);
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Wei Liu]

On Wed, Nov 27, 2013 at 03:17:20PM +0000, Paul Durrant wrote:
> > -----Original Message-----
> > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> > bounces@lists.xen.org] On Behalf Of Wei Liu
> > Sent: 27 November 2013 14:41
> > To: Tomasz Wroblewski
> > Cc: xen-devel; Paul Durrant; Wei Liu; David Vrabel
> > Subject: Re: [Xen-devel] Netback vif reference count mismatching in latest
> > 3.11 kernels
> > 
> > On Wed, Nov 27, 2013 at 03:07:09PM +0100, Tomasz Wroblewski wrote:
> > > Hi,
> > >
> > > After update of our network backend vm kernel to 3.11.9 I'm seeing
> > > trouble with netback vif close which seem related to the recent
> > > changes which separated vif disconnect and free; It seems that now
> > > multiple disconnect/connect cycles can happen without freeing and
> > > reallocing the netdev in the processes, which confuses the vif
> > > refcount.
> > >
> > > vif refcount is initialized to 1 in xenvif_alloc. Then first
> > > xenvif_disconnect brings it back to 0, instead of 1 which would seem
> > > more reasonable (since its initialized to 1 in xenvif_alloc i would
> > > expect it to not be dropped to 0 until xenvif_free). Second
> > > xenvif_disconnect brings it to -1 and hangs. For us (xenclient XT)
> > > this happens when we hibernate linux guest, since linux hibernate is
> > > a complex beast which transitions the drivers to between
> > > close/connected states multiple times (i.e. first it suspends/closes
> > > the drivers to take memory snapshot, then resumes/reconnects the
> > > drivers to the actual writing of hibernate image to disk, then
> > > finally it closes them again to shutdown the system)
> > >
> > 
> > Can you illustrate a graph of the whole process? I'm not very clear of
> > the whole cycle.
> > 
> > There's a xenvif_get in xenvif_connect, which increases refcnt by 1,
> > that should corresponds to the atomic_dec in xenvif_disconnect, right?
> > 
> 
> ISTR this happening with some frontends that did not drive the state model entirely correctly. Commit ea732dff5cfa10789007bf4a5b935388a0bb2a8f (Handle backend state transitions in a more robust way ) should fix this.
> 

FWIW 3.11.9 already has that patch.

Wei.

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Tomasz Wroblewski]

On 11/27/2013 03:41 PM, Wei Liu wrote:
> On Wed, Nov 27, 2013 at 03:07:09PM +0100, Tomasz Wroblewski wrote:
>> Hi,
>>
>> After update of our network backend vm kernel to 3.11.9 I'm seeing
>> trouble with netback vif close which seem related to the recent
>> changes which separated vif disconnect and free; It seems that now
>> multiple disconnect/connect cycles can happen without freeing and
>> reallocing the netdev in the processes, which confuses the vif
>> refcount.
>>
>> vif refcount is initialized to 1 in xenvif_alloc. Then first
>> xenvif_disconnect brings it back to 0, instead of 1 which would seem
>> more reasonable (since its initialized to 1 in xenvif_alloc i would
>> expect it to not be dropped to 0 until xenvif_free). Second
>> xenvif_disconnect brings it to -1 and hangs. For us (xenclient XT)
>> this happens when we hibernate linux guest, since linux hibernate is
>> a complex beast which transitions the drivers to between
>> close/connected states multiple times (i.e. first it suspends/closes
>> the drivers to take memory snapshot, then resumes/reconnects the
>> drivers to the actual writing of hibernate image to disk, then
>> finally it closes them again to shutdown the system)
>>
>
> Can you illustrate a graph of the whole process? I'm not very clear of
> the whole cycle.
>
> There's a xenvif_get in xenvif_connect, which increases refcnt by 1,
> that should corresponds to the atomic_dec in xenvif_disconnect, right?
>

Not if I'm reading the code right. I think the atomic_dec in xenvif_disconnect corresponds to atomic_set(..., 1) in xenvif_alloc, and 
xenvif_get() in xenvif_connect corresponds to xenvif_put in xenvif_carrier_off() (which is called at top of xenvif_disconnect).

Therefore xenvif_disconnect() decrements the refcnt twice whilst xenvif_connect() increments it once, which results in negative refcnt after 
one cycle. The graph I'm seeing looks like this:

      | guest vm boots
      |
      v
netback:xenvif_alloc() refcnt after=1
      |
      v
netback:xenvif_connect() refcnt after=2
      |
      v
    ..... stuff
      |
      | guest hibernate starts and suspends netfront (netfront goes to Closed xenbus state)
      |
      v
netback:xenvif_disconnect() refcnt after=0
      |
      | hibernate takes memory snapshot and resumes netfront afterwards (which will go thru initialising, connected etc states)
      |
      v
netback:xenvif_connect() refcnt after=1
      |
      | hibernate suspends netfront again (netfront goes to Closed xenbus state)
      |
      v
netback:xenvif_disconnect() refcnt=-1
      |
      v
   netback stuck
      |
      v
netback:xenvif_free() never happens

>> I've hacked the attached patch which fixes it (for us), is the approach taken there correct/upstreamable/reasonable? It does the following
>>
>>      * reset tx_irq to 0 after unbinding the irqs on disconnect -
>> because xenvif_connect tests for it being 0 and will not reconnect
>> if it's not reset
>>      * reacquire one reference to vif in disconnect(). This is because the reference
>>        vif should be 1, as initialized in xenvif_alloc(), until the vif is freed. Otherwise multiple disconne
>>        and cause a hang. I imagine alternate way of fixing this could be to use "0" as the default
>>        refcnt in xenvif_alloc()
>>
>
> You mean the numbers of connect's and disconnect's don't match? Even
> after you reset tx_irq to 0?
>
The number of connects and disconnect matches, but the amounts of refcnt changes do not seem to be symmetric in these functions. And yeah, 
the kernel I'm using already has Paul's "robustness" fix and subsequent zombie fix by David Vrabel.

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Wei Liu]

On Wed, Nov 27, 2013 at 05:21:56PM +0100, Tomasz Wroblewski wrote:
> On 11/27/2013 03:41 PM, Wei Liu wrote:
> >On Wed, Nov 27, 2013 at 03:07:09PM +0100, Tomasz Wroblewski wrote:
> >>Hi,
> >>
> >>After update of our network backend vm kernel to 3.11.9 I'm seeing
> >>trouble with netback vif close which seem related to the recent
> >>changes which separated vif disconnect and free; It seems that now
> >>multiple disconnect/connect cycles can happen without freeing and
> >>reallocing the netdev in the processes, which confuses the vif
> >>refcount.
> >>
> >>vif refcount is initialized to 1 in xenvif_alloc. Then first
> >>xenvif_disconnect brings it back to 0, instead of 1 which would seem
> >>more reasonable (since its initialized to 1 in xenvif_alloc i would
> >>expect it to not be dropped to 0 until xenvif_free). Second
> >>xenvif_disconnect brings it to -1 and hangs. For us (xenclient XT)
> >>this happens when we hibernate linux guest, since linux hibernate is
> >>a complex beast which transitions the drivers to between
> >>close/connected states multiple times (i.e. first it suspends/closes
> >>the drivers to take memory snapshot, then resumes/reconnects the
> >>drivers to the actual writing of hibernate image to disk, then
> >>finally it closes them again to shutdown the system)
> >>
> >
> >Can you illustrate a graph of the whole process? I'm not very clear of
> >the whole cycle.
> >
> >There's a xenvif_get in xenvif_connect, which increases refcnt by 1,
> >that should corresponds to the atomic_dec in xenvif_disconnect, right?
> >
> 
> Not if I'm reading the code right. I think the atomic_dec in
> xenvif_disconnect corresponds to atomic_set(..., 1) in xenvif_alloc,
> and xenvif_get() in xenvif_connect corresponds to xenvif_put in
> xenvif_carrier_off() (which is called at top of xenvif_disconnect).
> 
> Therefore xenvif_disconnect() decrements the refcnt twice whilst
> xenvif_connect() increments it once, which results in negative
> refcnt after one cycle. The graph I'm seeing looks like this:
> 

OK, I get what you mean. The separation of vif disconnect and free is
indeed causing problem.

I think the right fix would be to make things symmetric again.

Does the following *untested* patch work for you? If so I will submit a
proper patch to netdev.

Wei.

---8<---
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index d28324a..342d4e5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -418,9 +418,6 @@ void xenvif_disconnect(struct xenvif *vif)
 	if (netif_carrier_ok(vif->dev))
 		xenvif_carrier_off(vif);
 
-	atomic_dec(&vif->refcnt);
-	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
-
 	if (vif->tx_irq) {
 		if (vif->tx_irq == vif->rx_irq)
 			unbind_from_irqhandler(vif->tx_irq, vif);
@@ -428,6 +425,7 @@ void xenvif_disconnect(struct xenvif *vif)
 			unbind_from_irqhandler(vif->tx_irq, vif);
 			unbind_from_irqhandler(vif->rx_irq, vif);
 		}
+		vif->tx_irq = 0;
 	}
 
 	xen_netbk_unmap_frontend_rings(vif);
@@ -435,6 +433,9 @@ void xenvif_disconnect(struct xenvif *vif)
 
 void xenvif_free(struct xenvif *vif)
 {
+	atomic_dec(&vif->refcnt);
+	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
+
 	unregister_netdev(vif->dev);
 
 	free_netdev(vif->dev);

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Tomasz Wroblewski]

> OK, I get what you mean. The separation of vif disconnect and free is
> indeed causing problem.
>
> I think the right fix would be to make things symmetric again.
>
> Does the following *untested* patch work for you? If so I will submit a
> proper patch to netdev.
>
Thanks, indeed your patch works fine after a quick linux hibernate tests. But I will do more comprehensive test tomorrow morning (such as 
with windows guests as well, and things like rebooting backend network vm while front stays up) and get back to you with the result, so 
probably best to hold off a little :) thanks!

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Tomasz Wroblewski]

On 11/27/2013 06:02 PM, Tomasz Wroblewski wrote:
>
>> OK, I get what you mean. The separation of vif disconnect and free is
>> indeed causing problem.
>>
>> I think the right fix would be to make things symmetric again.
>>
>> Does the following *untested* patch work for you? If so I will submit a
>> proper patch to netdev.
>>
> Thanks, indeed your patch works fine after a quick linux hibernate tests. But I will do more comprehensive test tomorrow morning (such as
> with windows guests as well, and things like rebooting backend network vm while front stays up) and get back to you with the result, so
> probably best to hold off a little :) thanks!
>
Wei, gave your patch more testing now, as stated, and haven't noticed any trouble.

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Wei Liu]

On Thu, Nov 28, 2013 at 10:32 AM, Tomasz Wroblewski
<tomasz.wroblewski@citrix.com> wrote:
> On 11/27/2013 06:02 PM, Tomasz Wroblewski wrote:
>>
>>
>>> OK, I get what you mean. The separation of vif disconnect and free is
>>> indeed causing problem.
>>>
>>> I think the right fix would be to make things symmetric again.
>>>
>>> Does the following *untested* patch work for you? If so I will submit a
>>> proper patch to netdev.
>>>
>> Thanks, indeed your patch works fine after a quick linux hibernate tests.
>> But I will do more comprehensive test tomorrow morning (such as
>> with windows guests as well, and things like rebooting backend network vm
>> while front stays up) and get back to you with the result, so
>> probably best to hold off a little :) thanks!
>>
> Wei, gave your patch more testing now, as stated, and haven't noticed any
> trouble.
>

Thanks, please give me final confirmation if everything passes.

Wei.

>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Tomasz Wroblewski]

On 11/28/2013 12:45 PM, Wei Liu wrote:
> On Thu, Nov 28, 2013 at 10:32 AM, Tomasz Wroblewski
> <tomasz.wroblewski@citrix.com> wrote:
>> On 11/27/2013 06:02 PM, Tomasz Wroblewski wrote:
>>>
>>>
>>>> OK, I get what you mean. The separation of vif disconnect and free is
>>>> indeed causing problem.
>>>>
>>>> I think the right fix would be to make things symmetric again.
>>>>
>>>> Does the following *untested* patch work for you? If so I will submit a
>>>> proper patch to netdev.
>>>>
>>> Thanks, indeed your patch works fine after a quick linux hibernate tests.
>>> But I will do more comprehensive test tomorrow morning (such as
>>> with windows guests as well, and things like rebooting backend network vm
>>> while front stays up) and get back to you with the result, so
>>> probably best to hold off a little :) thanks!
>>>
>> Wei, gave your patch more testing now, as stated, and haven't noticed any
>> trouble.
>>
>
> Thanks, please give me final confirmation if everything passes.
>
it did, not planning to do more testing as of now (I tested S3/S4 in linuxes/windowses, reboots of network vm whilst frontend stays up and 
general lifecycle ops)

> Wei.
>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Wei Liu]

On Thu, Nov 28, 2013 at 01:07:15PM +0100, Tomasz Wroblewski wrote:
> On 11/28/2013 12:45 PM, Wei Liu wrote:
> >On Thu, Nov 28, 2013 at 10:32 AM, Tomasz Wroblewski
> ><tomasz.wroblewski@citrix.com> wrote:
> >>On 11/27/2013 06:02 PM, Tomasz Wroblewski wrote:
> >>>
> >>>
> >>>>OK, I get what you mean. The separation of vif disconnect and free is
> >>>>indeed causing problem.
> >>>>
> >>>>I think the right fix would be to make things symmetric again.
> >>>>
> >>>>Does the following *untested* patch work for you? If so I will submit a
> >>>>proper patch to netdev.
> >>>>
> >>>Thanks, indeed your patch works fine after a quick linux hibernate tests.
> >>>But I will do more comprehensive test tomorrow morning (such as
> >>>with windows guests as well, and things like rebooting backend network vm
> >>>while front stays up) and get back to you with the result, so
> >>>probably best to hold off a little :) thanks!
> >>>
> >>Wei, gave your patch more testing now, as stated, and haven't noticed any
> >>trouble.
> >>
> >
> >Thanks, please give me final confirmation if everything passes.
> >
> it did, not planning to do more testing as of now (I tested S3/S4 in
> linuxes/windowses, reboots of network vm whilst frontend stays up
> and general lifecycle ops)
> 

Thanks for the confirmation, I will add a tag

  reported-and-tested-by: Tomasz Wroblewski <tomasz.wroblewski@citrix.com>

to the final patch.

Wei.

> >Wei.
> >
> >>
> >>_______________________________________________
> >>Xen-devel mailing list
> >>Xen-devel@lists.xen.org
> >>http://lists.xen.org/xen-devel

Re: Netback vif reference count mismatching in latest 3.11 kernels

[Wei Liu]

On Wed, Nov 27, 2013 at 03:07:09PM +0100, Tomasz Wroblewski wrote:
> Hi,
> 
> After update of our network backend vm kernel to 3.11.9 I'm seeing
> trouble with netback vif close which seem related to the recent
> changes which separated vif disconnect and free; It seems that now
> multiple disconnect/connect cycles can happen without freeing and
> reallocing the netdev in the processes, which confuses the vif
> refcount.
> 
> vif refcount is initialized to 1 in xenvif_alloc. Then first
> xenvif_disconnect brings it back to 0, instead of 1 which would seem
> more reasonable (since its initialized to 1 in xenvif_alloc i would
> expect it to not be dropped to 0 until xenvif_free). Second
> xenvif_disconnect brings it to -1 and hangs. For us (xenclient XT)
> this happens when we hibernate linux guest, since linux hibernate is
> a complex beast which transitions the drivers to between
> close/connected states multiple times (i.e. first it suspends/closes
> the drivers to take memory snapshot, then resumes/reconnects the
> drivers to the actual writing of hibernate image to disk, then
> finally it closes them again to shutdown the system)
> 
> I've hacked the attached patch which fixes it (for us), is the approach taken there correct/upstreamable/reasonable? It does the following
> 
>     * reset tx_irq to 0 after unbinding the irqs on disconnect -
> because xenvif_connect tests for it being 0 and will not reconnect
> if it's not reset
>     * reacquire one reference to vif in disconnect(). This is because the reference
>       vif should be 1, as initialized in xenvif_alloc(), until the vif is freed. Otherwise multiple disconne
>       and cause a hang. I imagine alternate way of fixing this could be to use "0" as the default
>       refcnt in xenvif_alloc()
> 
> I believe we didn't experience this issue on previous kernel because
> vif disconnect was also freeing the vif and netdev, hence it was not
> possible to get xenvif_connect/xenvif_disconnect called multiple
> times between vif alloc/free.
> 
> 

For the record this bug only manifests in stable trees prior 3.12. A
patch for 1:1 model went in 3.12 and removed a bunch of ref counting
stuffs. On the other hand stable trees picked up Paul's state machine
patch which separated disconnect and free functions, not handling those
ref counting bits correctly.

Wei.