From mboxrd@z Thu Jan 1 00:00:00 1970 From: 'Eduardo Otubo' Subject: Re: [PATCH] xen-netfront: remove warning when unloading module Date: Mon, 20 Nov 2017 13:56:10 +0100 Message-ID: <20171120125610.GA12153@vader> References: <20171120104109.11585-1-otubo@redhat.com> <593f1c5a3bf24a2dbce56287391e776d@AMSPEX02CL03.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: "xen-devel@lists.xenproject.org" , "netdev@vger.kernel.org" , Wei Liu , "linux-kernel@vger.kernel.org" , "vkuznets@redhat.com" , "cavery@redhat.com" , "cheshi@redhat.com" , "mgamal@redhat.com" To: Paul Durrant Return-path: Content-Disposition: inline In-Reply-To: <593f1c5a3bf24a2dbce56287391e776d@AMSPEX02CL03.citrite.net> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Mon, Nov 20, 2017 at 10:55:55AM +0000, Paul Durrant wrote: > > -----Original Message----- > > From: Eduardo Otubo [mailto:otubo@redhat.com] > > Sent: 20 November 2017 10:41 > > To: xen-devel@lists.xenproject.org > > Cc: netdev@vger.kernel.org; Paul Durrant ; Wei > > Liu ; linux-kernel@vger.kernel.org; > > vkuznets@redhat.com; cavery@redhat.com; cheshi@redhat.com; > > mgamal@redhat.com; Eduardo Otubo > > Subject: [PATCH] xen-netfront: remove warning when unloading module > > > > When unloading module xen_netfront from guest, dmesg would output > > warning messages like below: > > > > [ 105.236836] xen:grant_table: WARNING: g.e. 0x903 still in use! > > [ 105.236839] deferring g.e. 0x903 (pfn 0x35805) > > > > This problem relies on netfront and netback being out of sync. By the time > > netfront revokes the g.e.'s netback didn't have enough time to free all of > > them, hence displaying the warnings on dmesg. > > > > The trick here is to make netfront to wait until netback frees all the g.e.'s > > and only then continue to cleanup for the module removal, and this is done > > by > > manipulating both device states. > > > > Signed-off-by: Eduardo Otubo > > --- > > drivers/net/xen-netfront.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c > > index 8b8689c6d887..b948e2a1ce40 100644 > > --- a/drivers/net/xen-netfront.c > > +++ b/drivers/net/xen-netfront.c > > @@ -2130,6 +2130,17 @@ static int xennet_remove(struct xenbus_device > > *dev) > > > > dev_dbg(&dev->dev, "%s\n", dev->nodename); > > > > + xenbus_switch_state(dev, XenbusStateClosing); > > + while (xenbus_read_driver_state(dev->otherend) != > > XenbusStateClosing){ > > + cpu_relax(); > > + schedule(); > > + } > > + xenbus_switch_state(dev, XenbusStateClosed); > > + while (dev->xenbus_state != XenbusStateClosed){ > > + cpu_relax(); > > + schedule(); > > + } > > + > > Waitiing for closing should be ok but waiting for closed is risky. As soon as a backend is in the closed state then a toolstack can completely remove the backend xenstore area, resulting a state of XenbusStateUnknown, which would cause your second loop to spin forever. > > Paul Well, that's a scenario I didn't foresee. I'll come up with a solution in order avoid this problem. Thanks for the review. > > > xennet_disconnect_backend(info); > > > > unregister_netdev(info->netdev); > > -- > > 2.13.6 > -- Eduardo Otubo