lkml.org 
[lkml]   [2013]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [Xen-devel] [PATCH 4/4] xen/xenbus: Avoid synchronous wait on XenBus stalling shutdown/restart.
On Thu, Nov 21, 2013 at 05:52:28PM +0000, David Vrabel wrote:
> On 08/11/13 17:38, Konrad Rzeszutek Wilk wrote:
> > The 'read_reply' works with 'process_msg' to read of a reply in XenBus.
> > 'process_msg' is running from within the 'xenbus' thread. Whenever
> > a message shows up in XenBus it is put on a xs_state.reply_list list
> > and 'read_reply' picks it up.
> >
> > The problem is if the backend domain or the xenstored process is killed.
> > In which case 'xenbus' is still awaiting - and 'read_reply' if called -
> > stuck forever waiting for the reply_list to have some contents.
> >
> > This is normally not a problem - as the backend domain can come back
> > or the xenstored process can be restarted. However if the domain
> > is in process of being powered off/restarted/halted - there is no
> > point of waiting on it coming back - as we are effectively being
> > terminated and should not impede the progress.
> >
> > This patch solves this problem by checking the 'system_state' value
> > to see if we are in heading towards death. We also make the wait
> > mechanism a bit more asynchronous.
>
> This seems to be checking the wrong thing conceptually. We should abort
> the wait if xenstored is dead not if our domain is dying.
>
> I think you can consider xenstored as dead if:
>
> a) it's local and we're dying.

OK. Not sure exactly how to do that but that should be possible.

> b) it's remote and the remote domain is dead.

OK, any idea how to do that? As in check if a remote domain is dead?

>
> > Fixes-Bug: http://bugs.xenproject.org/xen/bug/8
>
> This bug link has no useful information in it.
>
> > --- a/drivers/xen/xenbus/xenbus_xs.c
> > +++ b/drivers/xen/xenbus/xenbus_xs.c
> > @@ -148,9 +148,24 @@ static void *read_reply(enum xsd_sockmsg_type *type, unsigned int *len)
> >
> > while (list_empty(&xs_state.reply_list)) {
> > spin_unlock(&xs_state.reply_lock);
> > - /* XXX FIXME: Avoid synchronous wait for response here. */
> > - wait_event(xs_state.reply_waitq,
> > - !list_empty(&xs_state.reply_list));
> > + wait_event_timeout(xs_state.reply_waitq,
> > + !list_empty(&xs_state.reply_list),
> > + msecs_to_jiffies(500));
>
> This is still a synchronous wait. Is the removal of the FIXME comment
> correct?

I thought that the comment was meant in terms of it blocking forever.
But perhaps that was not the intent of the comment?
>
> > +
> > + /*
> > + * If we are in the process of being shut-down there is
> > + * no point of trying to contact XenBus - it is either
> > + * killed (xenstored application) or the other domain
> > + * has been killed or is unreachable.
>
> Not necessarily, xenstore could just be slow.

That is true. Your suggestion would help in evaluating when XenBus
end point is kaput.
>
> David


\
 
 \ /
  Last update: 2013-11-26 18:41    [W:0.254 / U:0.600 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site