Date: Sun, 21 Aug 2005 16:10:16 -0600 From: Scott Long <scottl@samsco.org> To: Nate Lawson <nate@root.org> Cc: Perforce Change Reviews <perforce@FreeBSD.org>, Scott Long <scottl@FreeBSD.org> Subject: Re: PERFORCE change 82377 for review Message-ID: <4308FBC8.3030305@samsco.org> In-Reply-To: <4308C06E.8040301@root.org> References: <200508211713.j7LHDWnZ056624@repoman.freebsd.org> <4308C06E.8040301@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4308FBC8.3030305>