From owner-p4-projects@FreeBSD.ORG  Sun Aug 21 22:10:22 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 120C616A421; Sun, 21 Aug 2005 22:10:22 +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 DD85716A41F;
	Sun, 21 Aug 2005 22:10:21 +0000 (GMT)
	(envelope-from scottl@samsco.org)
Received: from pooker.samsco.org (pooker.samsco.org [168.103.85.57])
	by mx1.FreeBSD.org (Postfix) with ESMTP id 5D2C643D48;
	Sun, 21 Aug 2005 22:10:18 +0000 (GMT)
	(envelope-from scottl@samsco.org)
Received: from [192.168.254.14] (imini.samsco.home [192.168.254.14])
	(authenticated bits=0)
	by pooker.samsco.org (8.13.3/8.13.3) with ESMTP id j7LMKSg7086780;
	Sun, 21 Aug 2005 16:20:28 -0600 (MDT)
	(envelope-from scottl@samsco.org)
Message-ID: <4308FBC8.3030305@samsco.org>
Date: Sun, 21 Aug 2005 16:10:16 -0600
From: Scott Long <scottl@samsco.org>
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US;
	rv:1.7.7) Gecko/20050416
X-Accept-Language: en-us, en
MIME-Version: 1.0
To: Nate Lawson <nate@root.org>
References: <200508211713.j7LHDWnZ056624@repoman.freebsd.org>
	<4308C06E.8040301@root.org>
In-Reply-To: <4308C06E.8040301@root.org>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
X-Spam-Status: No, score=-2.8 required=3.8 tests=ALL_TRUSTED autolearn=failed 
	version=3.0.2
X-Spam-Checker-Version: SpamAssassin 3.0.2 (2004-11-16) on pooker.samsco.org
Cc: Perforce Change Reviews <perforce@FreeBSD.org>,
	Scott Long <scottl@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 22:10:22 -0000

Nate Lawson wrote:

> 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.

I don't like what I did in here either.  I need to think about it more.

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

No.  The sendq is still only protected by Giant, so the spls are still
valid markers.

Scott