From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregory Haskins Subject: Re: [KVM PATCH v5 3/4] KVM: Fix races in irqfd using new eventfd_kref_get interface Date: Sun, 28 Jun 2009 15:54:42 -0400 Message-ID: <4A47CA82.4040108@novell.com> References: <20090625132441.26748.641.stgit@dev.haskins.net> <20090625132826.26748.15607.stgit@dev.haskins.net> <20090628114846.GA11764@redhat.com> <4A4767C2.3010503@novell.com> <20090628125612.GA11866@redhat.com> <20090628125730.GB11866@redhat.com> <20090628132035.GD11866@redhat.com> <4A479A23.1010804@novell.com> <20090628190710.GB14136@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig713850D088B4DA425FBD3B46" Cc: kvm@vger.kernel.org, avi@redhat.com To: "Michael S. Tsirkin" Return-path: Received: from victor.provo.novell.com ([137.65.250.26]:47744 "EHLO victor.provo.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751102AbZF1Ty5 (ORCPT ); Sun, 28 Jun 2009 15:54:57 -0400 In-Reply-To: <20090628190710.GB14136@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig713850D088B4DA425FBD3B46 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Michael S. Tsirkin wrote: > On Sun, Jun 28, 2009 at 12:28:19PM -0400, Gregory Haskins wrote: > =20 >> Michael S. Tsirkin wrote: >> =20 >>> On Sun, Jun 28, 2009 at 03:57:30PM +0300, Michael S. Tsirkin wrote: >>> =20 >>> =20 >>>> On Sun, Jun 28, 2009 at 03:56:12PM +0300, Michael S. Tsirkin wrote: >>>> =20 >>>> =20 >>>>> On Sun, Jun 28, 2009 at 08:53:22AM -0400, Gregory Haskins wrote: >>>>> =20 >>>>> =20 >>>>>> Michael S. Tsirkin wrote: >>>>>> =20 >>>>>> =20 >>>>>>> On Thu, Jun 25, 2009 at 09:28:27AM -0400, Gregory Haskins wrote: >>>>>>> =20 >>>>>>> =20 >>>>>>> =20 >>>>>>>> eventfd currently emits a POLLHUP wakeup on f_ops->release() to = generate a >>>>>>>> "release" callback. This lets eventfd clients know if the event= fd is about >>>>>>>> to go away and is very useful particularly for in-kernel clients= =2E However, >>>>>>>> until recently it is not possible to use this feature of eventfd= in a >>>>>>>> race-free way. This patch utilizes a new eventfd interface to r= ectify >>>>>>>> the problem. >>>>>>>> >>>>>>>> Note that one final race is known to exist: the slow-work thread= may race >>>>>>>> with module removal. We are currently working with slow-work up= stream >>>>>>>> to fix this issue as well. Since the code prior to this patch a= lso >>>>>>>> races with module_put(), we are not making anything worse, but r= ather >>>>>>>> shifting the cause of the race. Once the slow-work code is patc= hed we >>>>>>>> will be fixing the last remaining issue. >>>>>>>> =20 >>>>>>>> =20 >>>>>>>> =20 >>>>>>> By the way, why are we using slow-work here? Wouldn't a regular >>>>>>> workqueue do just as well, with less code, and avoid the race? >>>>>>> >>>>>>> =20 >>>>>>> =20 >>>>>>> =20 >>>>>> I believe it will cause a problem if you do a "flush_work()" from = inside >>>>>> a work-item. I could be wrong, of course, but it looks like a rec= ipe to >>>>>> deadlock. >>>>>> >>>>>> -Greg >>>>>> >>>>>> =20 >>>>>> =20 >>>>> Sure, but the idea is to only flush on kvm close, never from work i= tem. >>>>> =20 >>>>> =20 >>>> To clarify, you don't flush slow works from a work-item, >>>> so you shouldn't need to flush workqueue either. >>>> =20 >>>> =20 >>> I guess my question is - why is slow work different? It's still >>> a thread pool underneath ... >>> >>> =20 >>> =20 >> Its not interdependent. Flush-work blocks the thread..if the thread >> happens to be the work-queue thread you may deadlock preventing it fro= m >> processing further jobs like the inject. In reality it shouldnt be >> possible, but its just a bad idea to assume its ok. >> Slow work, on the >> other hand, will just make a new thread. >> >> -Greg >> >> =20 > > But if you create your own workqueue, and all you do there is destroy > irqfds, things are ok I think. Right? > =20 Yep, creating your own queue works too. I picked slow-work as an alternate to generating a dedicated resource, but I agree either method would work fine. Do you have a preference?=20 Regards, -Greg > > > =20 --------------enig713850D088B4DA425FBD3B46 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.11 (Darwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkpHyoUACgkQlOSOBdgZUxmPjACdFOgY/w7DZ+zOIlOYN+2md+a1 gFIAn2PSZfNr5YjchhM0INerjI4gwvrO =+gdR -----END PGP SIGNATURE----- --------------enig713850D088B4DA425FBD3B46--