From owner-freebsd-bugs@FreeBSD.ORG  Tue Oct 14 12:00:32 2003
Return-Path: <owner-freebsd-bugs@FreeBSD.ORG>
Delivered-To: freebsd-bugs@hub.freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id EAF4B16A4C0
	for <freebsd-bugs@hub.freebsd.org>;
	Tue, 14 Oct 2003 12:00:32 -0700 (PDT)
Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21])
	by mx1.FreeBSD.org (Postfix) with ESMTP id D9F9843F3F
	for <freebsd-bugs@hub.freebsd.org>;
	Tue, 14 Oct 2003 12:00:31 -0700 (PDT)
	(envelope-from gnats@FreeBSD.org)
Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1])
	by freefall.freebsd.org (8.12.9/8.12.9) with ESMTP id h9EJ0VFY065060
	for <freebsd-bugs@freefall.freebsd.org>;
	Tue, 14 Oct 2003 12:00:31 -0700 (PDT)
	(envelope-from gnats@freefall.freebsd.org)
Received: (from gnats@localhost)
	by freefall.freebsd.org (8.12.9/8.12.9/Submit) id h9EJ0V4e065059;
	Tue, 14 Oct 2003 12:00:31 -0700 (PDT)
	(envelope-from gnats)
Date: Tue, 14 Oct 2003 12:00:31 -0700 (PDT)
Message-Id: <200310141900.h9EJ0V4e065059@freefall.freebsd.org>
To: freebsd-bugs@FreeBSD.org
From: Bruce Evans <bde@zeta.org.au>
Subject: Re: kern/57945: [patch] Add locking to kqueue to make it MP-safe
X-BeenThere: freebsd-bugs@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
Reply-To: Bruce Evans <bde@zeta.org.au>
List-Id: Bug reports <freebsd-bugs.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-bugs>,
	<mailto:freebsd-bugs-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-bugs>
List-Post: <mailto:freebsd-bugs@freebsd.org>
List-Help: <mailto:freebsd-bugs-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-bugs>,
	<mailto:freebsd-bugs-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 14 Oct 2003 19:00:33 -0000

The following reply was made to PR kern/57945; it has been noted by GNATS.

From: Bruce Evans <bde@zeta.org.au>
To: Stefan Farfeleder <stefan@fafoe.narf.at>
Cc: freebsd-gnats-submit@freebsd.org
Subject: Re: kern/57945: [patch] Add locking to kqueue to make it MP-safe
Date: Wed, 15 Oct 2003 04:50:03 +1000 (EST)

 On Mon, 13 Oct 2003, Stefan Farfeleder wrote:
 
 >  On Tue, Oct 14, 2003 at 03:56:19AM +1000, Bruce Evans wrote:
 
 >  > > -			error = tsleep(kq, PSOCK | PCATCH, "kqread", timeout);
 >  > > +			error = msleep(kq, &kq->kq_mtx, PSOCK | PCATCH | PDROP,
 >  > > +			    "kqread", timeout);
 >  > >  		}
 >  > > -		splx(s);
 >  >
 >  > Not copying the spl locking seems to give a bug here.  msleep() returns with
 >  > the mutex held.
 >
 >  The PDROP flag should prevent msleep() from reacquiring the lock.
 
 Oops.
 
 >  > > +		kq->kq_count--;
 >  > > +		mtx_unlock(&kq->kq_mtx);
 >  > >  		if (kn->kn_status & KN_DISABLED) {
 >  > >  			kn->kn_status &= ~KN_QUEUED;
 >  > > -			kq->kq_count--;
 >  > > +			mtx_lock(&kq->kq_mtx);
 >  > >  			continue;
 >  > >  		}
 >  >
 >  > I don't see any reason to unlock/lock here.  There were no splx()/splhigh()'s.
 >  > splhigh() is very global but kq_mtx is very local so keeping the
 >  > critical sections short should be much less needed than before.
 >
 >  Ah, right.  So it should look like this?
 >
 >  +		kq->kq_count--;
 >   		if (kn->kn_status & KN_DISABLED) {
 >   			kn->kn_status &= ~KN_QUEUED;
 >  -			kq->kq_count--;
 >   			continue;
 >   		}
 >  +		mtx_unlock(&kq->kq_mtx);
 
 OK.
 
 >  > BTW, what locks kn?
 >
 >  Nothing at the moment, I hoped it wouldn't be necessary.  Do you have a
 >  specific race in mind?
 
 Nothing specific.  It must be locked by something.  Giant I guess.
 
 Bruce