[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] fix race condition between libvirtd event handling and libxl fd deregister
Ian Campbell wrote: > On Tue, 2012-11-20 at 07:16 +0000, Bamvor Jian Zhang wrote: > >> the race condition may be encounted at the following senaro: >> (1), xenlight will remove fd handle just after the transfer is done >> according to >> the buffer pointer. This action will first mark fd handle deleted in libvirtd >> then remove fd handler from list in libxl. To mark the fd deleted in >> libvirtd, >> the libvirt event loop mutex must be acquired. >> >> (2), meanwhile in the libvirt event dispatch process, libvirt will check the >> fd >> deleted flag while holding the event loop mutex. At this time, "(1)" may be >> blocked from marking the deleted flag. Then libvirt release its mutex >> temperary >> to run the dispatcher callback. But this callback need xenlight >> lock(CTX_LOCK) >> which is held by xenlight fd deregister function. So, libvirtd will continue >> to >> run this callback after fd deregister exit which means xenlight has been >> marked >> deleted flag, removed this fd handler and set ev->fd as -1. after >> libxl__ev_fd_deregister exit, it is time for callback running. but >> unfortunately, this callback has been removed as I mentioned above. >> >> reference the following graph: >> libvirt event dispatch xenlight transfer done >> | enter libxl__ev_fd_deregister >> | CTX_LOCK >> | | >> | | >> | enter osevent_fd_deregister >> | | >> | enter virEventRemoveHandle >> | waiting virMutexLock >> check handler delete flag | >> virMutexUnlock | >> | virMutexLock >> enter libxl_osevent_occurred_fd | >> waiting CTX_LOCK mark delete flag >> | virMutexUnlock >> | | >> | exit virEventRemoveHandle >> | exit osevent_fd_deregister >> | | >> | remove fd handler from list >> | set ev->fd as -1 >> | CTX_UNLOCK >> CTX_LOCK >> assert(fd==ev->fd) //lead to crash >> call back in libxl >> CTX_UNLOCK >> exit libxl_osevent_occurred_fd >> > > This all seems pretty plausible to me, but I'd like to have an Ack from > Ian J before I commit. > FYI, I hit the race again today on a test machine without this patch, and no longer encountered the race after applying the patch. So Tested-by: Jim Fehlig <jfehlig@xxxxxxxx> If ACK'ed by Ian J., can this also be added to 4.2-testing? Thanks! Jim > >> at the same time, i found the times of file handler register is less >> than the times of file handler deregister. is that right? seems that >> it will be better if the register and deregister is paired. >> > > How many extra file handles are we talking about? > > Presumably some long lived file handles will stay around until you call > libxl_ctx_free for the ctx. Or are you doing this and saying you are > seeing leaked fd's? > > >> Signed-off-by: Bamvor Jian Zhang <bjzhang@xxxxxxxx> >> > > >> diff -r 321f8487379b -r 0451e6041bdd tools/libxl/libxl_event.c >> --- a/tools/libxl/libxl_event.c Thu Nov 15 10:25:29 2012 +0000 >> +++ b/tools/libxl/libxl_event.c Tue Nov 20 14:56:04 2012 +0800 >> @@ -1018,11 +1018,15 @@ void libxl_osevent_occurred_fd(libxl_ctx >> CTX_LOCK; >> assert(!CTX->osevent_in_hook); >> >> + if (!libxl__ev_fd_isregistered(ev)) { >> + DBG("ev_fd=%p deregister unregistered",ev); >> + goto out; >> + } >> assert(fd == ev->fd); >> revents &= ev->events; >> if (revents) >> ev->func(egc, ev, fd, ev->events, revents); >> - >> +out: >> CTX_UNLOCK; >> EGC_FREE; >> } >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@xxxxxxxxxxxxx >> http://lists.xen.org/xen-devel >> > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |