From owner-cvs-all@FreeBSD.ORG Thu Jun 10 02:41:41 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 680) id B870916A4D0; Thu, 10 Jun 2004 02:41:41 +0000 (GMT) Date: Thu, 10 Jun 2004 02:41:41 +0000 From: Darren Reed To: Brian Feldman Message-ID: <20040610024141.GA59816@hub.freebsd.org> References: <55929.1086798000@critter.freebsd.dk> <20040609092423.N85944@root.org> <20040609164538.GB15285@green.homeunix.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20040609164538.GB15285@green.homeunix.org> User-Agent: Mutt/1.4.1i cc: src-committers@FreeBSD.org cc: cvs-src@FreeBSD.org cc: cvs-all@FreeBSD.org cc: Poul-Henning Kamp cc: Nate Lawson cc: "M. Warner Losh" Subject: Re: cvs commit: src/sys/kern kern_proc.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Jun 2004 02:41:41 -0000 On Wed, Jun 09, 2004 at 12:45:38PM -0400, Brian Feldman wrote: > > That's not a way to handle that case. The way to handle that case > in general is to make it impossible to find a reference the object > when the refcount hits zero. > > LOCK(foo_list) > > > LOCK(foo->lock) > > > i = --foo->refcount; > if (i == 0) > remove(foo_list, foo); > > > UNLOCK(foo->lock) > UNLOCK(foo_list) > > > if (i == 0) > > > destroy(foo); Just to add my $0.02 on this thread, the above code is correct. you could put the destroy in the critical section but those parts of the code are meant to be small and tight. In my experience, the way to do this for performance is to use read-write locks for locking on the list. So, for example, you would have: new() { create and initialise new foo foo->refcount = 2 WRITE_LOCK(list) add_to_list(foo) UNLOCK(list) return foo } search() { READ_LOCK(list) find foo if (foo is found) LOCK(foo->lock) foo->refcount++; UNLOCK(foo->lock) UNLOCK(list) return foo; } deref(foo) { LOCK(foo->lock) foo->refcount-- if (foo->refcount == 0) foo->refcount++ UNLOCK(foo->lock) delete(foo) else UNLOCK(foo->lock) } delete(foo) { WRITE_LOCK(list) if (foo_on_list) remove_from_list(foo) LOCK(foo->lock) foo->refcount-- if (foo->refcount == 0) destroy(foo) else UNLOCK(foo->lock) UNLOCK(list) } Why use read/write locks? It allows concurrent searching and use of the object foo with little penalty - it does, however, make new/delete operations slower but what occurs more - search or new/delete ? Why is there a "refcount++" after checking "refcount == 0" ? Because you need to give up foo->lock and in this code, you don't have a lock on the list to stop another finding it and thereby doing a refcount++. This is a more complex code model because the locking is more detached from the operations. It also distinguishes between deref() and delete(), although delete() can be used without calling deref(). It allows the object "foo" to be used safely without a lock on list or foo. The race between search() incrementing the refcount and the delete() free'ing the object is eliminated by the list lock. Technically you shouldn't need to have foo->lock for remove_from_list because the list modifications should all be exclusive anyway (in this case, the write lock provides it.) You should only ever have code entering both deref() and delete() simultaneously if refcount == 2. Why does new() set refcount to 2 ? Because it returns a pointer to the object for the calling code to use and there's a reference to it on the list. When the calling code finishes using it, it must call deref(). If you need to destroy the mutex in a special way then this is done with destroy(). Questions/comments ? Darren