Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Oct 2003 12:00:31 -0700 (PDT)
From:      Bruce Evans <bde@zeta.org.au>
To:        freebsd-bugs@FreeBSD.org
Subject:   Re: kern/57945: [patch] Add locking to kqueue to make it MP-safe
Message-ID:  <200310141900.h9EJ0V4e065059@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200310141900.h9EJ0V4e065059>