From owner-p4-projects@FreeBSD.ORG  Sun Aug 21 17:57:09 2005
Return-Path: <owner-p4-projects@FreeBSD.ORG>
X-Original-To: p4-projects@freebsd.org
Delivered-To: p4-projects@freebsd.org
Received: by hub.freebsd.org (Postfix, from userid 32767)
	id 3A7CE16A421; Sun, 21 Aug 2005 17:57:09 +0000 (GMT)
X-Original-To: perforce@FreeBSD.org
Delivered-To: perforce@FreeBSD.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id F06B116A41F;
	Sun, 21 Aug 2005 17:57:08 +0000 (GMT) (envelope-from nate@root.org)
Received: from www.cryptography.com (li-22.members.linode.com [64.5.53.22])
	by mx1.FreeBSD.org (Postfix) with ESMTP id A4C8F43D45;
	Sun, 21 Aug 2005 17:57:08 +0000 (GMT) (envelope-from nate@root.org)
Received: from [10.0.0.250] (adsl-64-171-187-4.dsl.snfc21.pacbell.net
	[64.171.187.4])
	by www.cryptography.com (8.12.8/8.12.8) with ESMTP id j7LHv8o5010430
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT);
	Sun, 21 Aug 2005 10:57:11 -0700
Message-ID: <4308C06E.8040301@root.org>
Date: Sun, 21 Aug 2005 10:57:02 -0700
From: Nate Lawson <nate@root.org>
User-Agent: Mozilla Thunderbird 1.0.6 (Windows/20050716)
X-Accept-Language: en-us, en
MIME-Version: 1.0
To: Scott Long <scottl@FreeBSD.org>
References: <200508211713.j7LHDWnZ056624@repoman.freebsd.org>
In-Reply-To: <200508211713.j7LHDWnZ056624@repoman.freebsd.org>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Perforce Change Reviews <perforce@FreeBSD.org>
Subject: Re: PERFORCE change 82377 for review
X-BeenThere: p4-projects@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: p4 projects tree changes <p4-projects.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/p4-projects>,
	<mailto:p4-projects-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/p4-projects>
List-Post: <mailto:p4-projects@freebsd.org>
List-Help: <mailto:p4-projects-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/p4-projects>,
	<mailto:p4-projects-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sun, 21 Aug 2005 17:57:10 -0000

Scott Long wrote:
> http://perforce.freebsd.org/chv.cgi?CH=82377
> 
> Change 82377 by scottl@scottl-junior on 2005/08/21 17:13:18
> 
> 	Introduce the topology lock.  It covers the reference counting of
> 	path components so that peripherals and sims can use path objects
> 	without having to lock them.
>
> @@ -4808,6 +4839,7 @@
>  					  sim->c_handle);
>  				sim->flags &= ~CAM_SIM_REL_TIMEOUT_PENDING;
>  			}
> +			mtx_lock(&cam_topo_lock);
>  			bus = xpt_find_bus(sim->path_id);
>  			splx(s);
>  
> @@ -4815,9 +4847,12 @@
>  				/*
>  				 * Now that we are unfrozen run the send queue.
>  				 */
> +				mtx_unlock(&cam_topo_lock);
>  				xpt_run_dev_sendq(bus);
> +				mtx_lock(&cam_topo_lock);
>  			}
>  			xpt_release_bus(bus);
> +			mtx_unlock(&cam_topo_lock);
>  		} else
>  			splx(s);
>  	} else

I've heard of some performance problems from unlocking and relocking in 
the xpt_start() path for each ccb.  I do that in scsi_target.  Since 
this is only once per bus, this may be fine here.

There is also a dangling splx() above and a few others left that were 
obsoleted by your lock.

-- 
Nate