Re: [Xen-devel] xenfs: race condition on xenstore watch

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.05.13 at 14:55, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
> On Wed, May 15, 2013 at 05:51:06PM +0100, Jonathan Davies wrote:
>> Normally, the userspace process gets an "OK" from xenfs and then the
>> watch fires immediately after. Occasionally, this happens the other way
>> around: the watch fires before the driver sends "OK", which confuses
>> the xenstore-watch client. It seems to me that the client is within its
>> rights to expect the "OK" first.

Now that I look at this another time, I wonder how this behavior
can be guaranteed even with your patch: xenbus_write_watch()
sends the reply by calling queue_reply() followed by wake_up(),
so the reply getting delivered and the watch firing appear to be
inherently asynchronous anyway, and your patch just reduces
the race window. Am I overlooking something here?

>> Take the (non-reentrant) reply_mutex before calling
>> register_xenbus_watch to prevent the watch_fired callback from writing
>> anything until the "OK" has been sent.
> 
> Should that perhaps be done inside the msg_type == XS_WATCH code (with a
> bool that would determine whether an reply_mutex has been taken?)
> 
> As in, is there no need to take this mutex if msg_type != XS_WATCH?

As said in an earlier reply, I also think that it would be preferable
to shrink the locked region as much as possible. But there clearly
is a need to also acquire the mutex for msg_type != XS_WATCH,
just not before the loop in the else branch.

Jan


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