From owner-svn-src-all@FreeBSD.ORG  Wed Jul 29 07:02:28 2009
Return-Path: <owner-svn-src-all@FreeBSD.ORG>
Delivered-To: svn-src-all@FreeBSD.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 91AF6106566B;
	Wed, 29 Jul 2009 07:02:28 +0000 (UTC)
	(envelope-from rwatson@FreeBSD.org)
Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42])
	by mx1.freebsd.org (Postfix) with ESMTP id 6AB2A8FC1D;
	Wed, 29 Jul 2009 07:02:28 +0000 (UTC)
	(envelope-from rwatson@FreeBSD.org)
Received: from fledge.watson.org (fledge.watson.org [65.122.17.41])
	by cyrus.watson.org (Postfix) with ESMTPS id 0A96046B0C;
	Wed, 29 Jul 2009 03:02:28 -0400 (EDT)
Date: Wed, 29 Jul 2009 08:02:27 +0100 (BST)
From: Robert Watson <rwatson@FreeBSD.org>
X-X-Sender: robert@fledge.watson.org
To: Randall Stewart <rrs@lakerest.net>
In-Reply-To: <354E0657-DC37-4493-8E17-D09B257B5A28@lakerest.net>
Message-ID: <alpine.BSF.2.00.0907290735170.17344@fledge.watson.org>
References: <200907281409.n6SE971u034585@svn.freebsd.org>
	<20090729051016.GB3550@garage.freebsd.pl>
	<354E0657-DC37-4493-8E17-D09B257B5A28@lakerest.net>
User-Agent: Alpine 2.00 (BSF 1167 2008-08-23)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org,
	src-committers@FreeBSD.org, Pawel Jakub Dawidek <pjd@FreeBSD.org>
Subject: Re: svn commit: r195918 - head/sys/netinet
X-BeenThere: svn-src-all@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: "SVN commit messages for the entire src tree \(except for &quot;
	user&quot; and &quot; projects&quot; \)" <svn-src-all.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-all>,
	<mailto:svn-src-all-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-all>
List-Post: <mailto:svn-src-all@freebsd.org>
List-Help: <mailto:svn-src-all-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-all>,
	<mailto:svn-src-all-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 29 Jul 2009 07:02:29 -0000


On Wed, 29 Jul 2009, Randall Stewart wrote:

>> Instead of using additional argument to the sctp_add_to_readq() function, 
>> wouldn't it be sufficient to just check with mtx_owned(9) if the lock is 
>> already held?
>
> Hmm... I suppose one could go that way... but traditionally upper code as 
> told the lower code that it holds/does not hold the lock. This is true in 
> quite a few other functions...

Structures of the form:

 	if (mtx_owned(&mtx))
 		mtx_unlock(&mtx);

Strike me as less robust than code with either fixed assertions about lock 
state from the caller, or code that accepts a flag that in effect leads to two 
variants each with fixed state that can be asserted.  I.e.,

 	void
 	foo(void *obj)
 	{

 		OBJ_LOCK_ASSERT(obj);
 		...
 	}

or:

 	void
 	foo(void *obj, int arg_locked)
 	{

 		if (arg_locked)
 			OBJ_LOCK_ASSERT(obj);
 		else
 			OBJ_LOCK(obj);
 		...
 		if (!arg_locked)
 			OBJ_UNLOCK(obj);
 	}

I guess I'm sort of OK with structure but it smacks of poor code design:

 	void
 	foo(void *obj)
 	{
 		int locked;

 		if (OBJ_LOCK_OWNED(obj)) {
 			locked = 1;
 			OBJ_LOCK(obj);
 		} else
 			locked = 0;
 		...
 		if (locked)
 			OBJ_UNLOCK(obj);
 	}

However, this structure doesn't lend itself to moving to lock types that can't 
cheaply support mtx_owned()-like operations, such as read acquisitions of 
reader-writer locks.

Robert N M Watson
Computer Laboratory
University of Cambridge