Messages in this thread | | | Subject | Re: [PATCH] xen: fix frontend driver disconnected from xenbus on removal | From | Boris Ostrovsky <> | Date | Thu, 1 Feb 2018 16:09:25 -0500 |
| |
On 02/01/2018 03:24 PM, Oleksandr Andrushchenko wrote: > > > On 02/01/2018 10:08 PM, Boris Ostrovsky wrote: >> On 02/01/2018 03:57 AM, Oleksandr Andrushchenko wrote: >>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> >>> Current xenbus frontend driver removal flow first disconnects >>> the driver from xenbus and then calls driver's remove callback. >>> This makes it impossible for the driver to listen to backend's >>> state changes and synchronize the removal procedure. >>> >>> Fix this by removing other end XenBus watches after the >>> driver's remove callback is called. >>> >>> Signed-off-by: Oleksandr Andrushchenko >>> <oleksandr_andrushchenko@epam.com> >>> --- >>> drivers/xen/xenbus/xenbus_probe.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/xen/xenbus/xenbus_probe.c >>> b/drivers/xen/xenbus/xenbus_probe.c >>> index 74888cacd0b0..9c63cd3f416b 100644 >>> --- a/drivers/xen/xenbus/xenbus_probe.c >>> +++ b/drivers/xen/xenbus/xenbus_probe.c >>> @@ -258,11 +258,11 @@ int xenbus_dev_remove(struct device *_dev) >>> DPRINTK("%s", dev->nodename); >>> - free_otherend_watch(dev); >>> - >>> if (drv->remove) >>> drv->remove(dev); >> >> Is it possible for the watch to fire here? > Indeed. Yes, It is possible, so we have to somehow protect the removed > driver from being called, e.g. the driver cleans up in its .remove, > but watch may still trigger .otherend_changed callback. > Is this what you mean?
(-David who is not at Citrix anymore)
Exactly.
That's why otherend cleanup is split into free_otherend_watch() and free_otherend_details().
> If so, do you have something neat on your mind how to solve this?
Not necessarily "neat" but perhaps you can use xenbus_read_otherend_details() in both front and back ends. After all, IIUIC you are doing something synchronously so you don't really need a watch.
-boris
> >> -boris >> >>> + free_otherend_watch(dev); >>> + >>> free_otherend_details(dev); >>> xenbus_switch_state(dev, XenbusStateClosed); > Thank you, > Oleksandr
| |