From owner-freebsd-current@FreeBSD.ORG Mon Jul 18 22:18:39 2005 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id AA82416A41C for ; Mon, 18 Jul 2005 22:18:39 +0000 (GMT) (envelope-from jmg@hydrogen.funkthat.com) Received: from hydrogen.funkthat.com (gate.funkthat.com [69.17.45.168]) by mx1.FreeBSD.org (Postfix) with ESMTP id 465B143D48 for ; Mon, 18 Jul 2005 22:18:39 +0000 (GMT) (envelope-from jmg@hydrogen.funkthat.com) Received: from hydrogen.funkthat.com (localhost.funkthat.com [127.0.0.1]) by hydrogen.funkthat.com (8.13.3/8.13.3) with ESMTP id j6IMIQVg026876; Mon, 18 Jul 2005 15:18:26 -0700 (PDT) (envelope-from jmg@hydrogen.funkthat.com) Received: (from jmg@localhost) by hydrogen.funkthat.com (8.13.3/8.13.3/Submit) id j6IMINWh026875; Mon, 18 Jul 2005 15:18:23 -0700 (PDT) (envelope-from jmg) Date: Mon, 18 Jul 2005 15:18:23 -0700 From: John-Mark Gurney To: Craig Rodrigues Message-ID: <20050718221822.GB97979@funkthat.com> Mail-Followup-To: Craig Rodrigues , "Wojciech A. Koszek" , freebsd-current@freebsd.org References: <20050716224442.GA87607@freebsd.czest.pl> <20050717153936.GA2218@crodrigues.org> <20050717165945.GA1017@crodrigues.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20050717165945.GA1017@crodrigues.org> User-Agent: Mutt/1.4.2.1i X-Operating-System: FreeBSD 5.4-RELEASE-p1 i386 X-PGP-Fingerprint: B7 EC EF F8 AE ED A7 31 96 7A 22 B3 D8 56 36 F4 X-Files: The truth is out there X-URL: http://resnet.uoregon.edu/~gurney_j/ X-Resume: http://resnet.uoregon.edu/~gurney_j/resume.html Cc: "Wojciech A. Koszek" , freebsd-current@freebsd.org Subject: Re: [LOR] kqueue() X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: John-Mark Gurney List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jul 2005 22:18:39 -0000 Craig Rodrigues wrote this message on Sun, Jul 17, 2005 at 12:59 -0400: > On Sun, Jul 17, 2005 at 11:39:36AM -0400, Craig Rodrigues wrote: > > On Sat, Jul 16, 2005 at 10:44:43PM +0000, Wojciech A. Koszek wrote: > > > http://FreeBSD.czest.pl/dunstan/kqueuetest.c > > > > I can reproduce this LOR: > > > > It looks like on line 1545 of kern_event.c, we have a KQ_LOCK(kq);, > > and then on line 1039, the KQ_NOTOWNED(kq); assertion fails, because > > the lock is never released. > > Can you try this patch: > > --- /usr/src/sys/kern/kern_event.c.orig Sun Jul 17 12:32:58 2005 > +++ /usr/src/sys/kern/kern_event.c Sun Jul 17 12:41:54 2005 > @@ -410,7 +410,15 @@ > kev.fflags = kn->kn_sfflags; > kev.data = kn->kn_id; /* parent */ > kev.udata = kn->kn_kevent.udata; /* preserve udata */ > + > + if (kn->kn_status & KN_HASKQLOCK) > + KQ_UNLOCK(kn->kn_kq); > + > error = kqueue_register(kn->kn_kq, &kev, NULL, 0); > + > + if (kn->kn_status & KN_HASKQLOCK) > + KQ_LOCK(kn->kn_kq); > + > if (error) > kn->kn_fflags |= NOTE_TRACKERR; > } This is not a correct fix.. This will cause the error message to go away, but will open a race... You can only unlock the kq while holding a reference to the knote when the knote is INFLUX.... If it's not INFLUX, the knote could disappear out from under you by another thread deleting it and you'd have a pointer to stale memory.... It maybe possible to exploit the fact that the knote has the KN_HASKQLOCK set in it's kn_status, but looking at kqueue_register, this would be difficult, since we might sleep and need to unlock the kqueue... the most correct solution would be to attach a work item to the process and then have another thread (possibly even the kqueue thread) process the event add request out of band where we won't have a problem with the locks... The only issue with this is that there may not be the ability to immediately note the error if the knote is unable to be attached.... > I don't get the crash any more, but sometimes I get this LOR > in dmesg: > > lock order reversal > 1st 0xc1b0aaa4 process lock (process lock) @ /usr/src/sys/kern/kern_fork.c:690 > 2nd 0xc092dba0 allproc (allproc) @ /usr/src/sys/kern/kern_proc.c:229 Which is another race opened by the patch that can get you into trouble... -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."