From owner-freebsd-scsi@FreeBSD.ORG Fri Jun 1 04:43:12 2007 Return-Path: X-Original-To: freebsd-scsi@freebsd.org Delivered-To: freebsd-scsi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id BA9B916A41F for ; Fri, 1 Jun 2007 04:43:12 +0000 (UTC) (envelope-from freebsd@gm.nunu.org) Received: from py-out-1112.google.com (py-out-1112.google.com [64.233.166.178]) by mx1.freebsd.org (Postfix) with ESMTP id 84DAB13C44B for ; Fri, 1 Jun 2007 04:43:12 +0000 (UTC) (envelope-from freebsd@gm.nunu.org) Received: by py-out-1112.google.com with SMTP id a29so740542pyi for ; Thu, 31 May 2007 21:43:12 -0700 (PDT) Received: by 10.35.88.17 with SMTP id q17mr2295296pyl.1180672568057; Thu, 31 May 2007 21:36:08 -0700 (PDT) Received: by 10.35.79.1 with HTTP; Thu, 31 May 2007 21:36:07 -0700 (PDT) Message-ID: <626eb4530705312136n6aa78a96t45aa19a0c9912863@mail.gmail.com> Date: Fri, 1 Jun 2007 13:36:07 +0900 From: "Hidetoshi Shimokawa" Sender: freebsd@gm.nunu.org To: "Scott Long" In-Reply-To: <465F90A9.7080608@samsco.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <86wsyopf0t.wl%simokawa@FreeBSD.ORG> <465F90A9.7080608@samsco.org> X-Google-Sender-Auth: 1d598d044e7ed63a Cc: freebsd-scsi@freebsd.org Subject: Re: CAM locking X-BeenThere: freebsd-scsi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SCSI subsystem List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Jun 2007 04:43:12 -0000 On 6/1/07, Scott Long wrote: > Hidetoshi Shimokawa wrote: > > Hi Scott, > > > > Thank you for your work on CAM. > > I have some questions about lock assertion. > > > > 1. Now, xpt_done() should be called with a sim lock for QUEUED ccbs. > > Why don't you add an assertion in xpt_done()? > > Accessing the sim_lock from the CCB requires enough dereferences that if > the lock isn't already held, you'll probably run into problems just > doing the derefs in the assertion. Adding a sim_lock field to the CCB > has crossed my mind, but I don't want to pollute the CCB with more > kernel-only fields if I can. I need to think some more on it. If the lock isn't already held, we have a problem sooner or later. The assertion doesn't add extra dereference except for mtx itself. I don't see any problem with adding this assertion, though it may be not perfect. I think it useful for detecting programming error and describing interface. > > 2. CAMDEBUG seems useless with WITNESS because xpt_path_path_id() requires > > a sim lock. Can we safely drop the assertion? > > Yes, this is an area that I didn't clean up very well. A lot of code in > periphs and SIMs wants to be able to use CAMDEBUG functions in random > places without the lock held, and I don't know of a good way to safely > fix this without rewriting a lot of code. I don't know if it's better > to just drop the assertion or to audit all of the code provide locked > and unlocked versions of the CAMDEBUG functions. Yes, it's very complicated to lock all debug codes. Can you document somewhere that CAMDEBUG is not usable with INVARIANTS(or WINTNESS??). -- /\ Hidetoshi Shimokawa \/ simokawa@FreeBSD.ORG