Re: [Xen-devel] [PATCH 3/3] libxl: fd events: Suppress spurious fd events

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2015-04-16 at 19:23 +0100, Ian Jackson wrote:
> Always recheck with poll() right before making the callback.
> 
> All sorts of things may have happened since poll() originally signaled
> the fd.  We would like the main functional libxl code not to have to
> worry about spurious wakeups.

This works because the recheck and the callback are both done within the
same CTX lock critical section. The major assumption here is that
anything which could invalidate the result of the previous poll must
also happen with the lock held, otherwise you have a race between the
recheck and the callback.

The new call of fd_occurs here is locked, as is the one from
afterpoll_internal (refactored in a previous patch), so good.

Are all of the fd's which are going to be involved in this path owned by
libxl? I believe that is the case (if not then the application would
need to take care). The application may have visibility onto the fds
(via the result of libxl_osevent_beforepoll or the use of the
fd_register hook) but I think it is pretty obvious that the application
should never do anything with those fds other than poll or remember them
to give to libxl later.

Likewise is all of the code in libxl which touches such an fd locked? I
expect this is usually trivially the case because the fd is touched in a
callback, and if it is not the case elsewhere that would be a bug to be
fixed.

(a bit longer than it needed to be, but I figured that having reasoned
it out in writing to keep my thoughts straight I may as well post it)

> In particular, this fixes a bug in the save/restore callout: the save
> helper message reader operates with the fd in blocking mode.  In a
> multithreaded program one thread might have eaten all the messages out
> of the fd while another one is busy returning from poll and reaquiring

"reacquiring"

> the libxl lock, possibly resulting in a deadlock.
> 
> (Also, we abolish the anomalous direct caller of efd->func.)
> 
> Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Reported-by: Jim Fehlig <jfehlig@xxxxxxxx>
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Jim Fehlig <jfehlig@xxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>



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