From owner-freebsd-scsi@FreeBSD.ORG Sat Jun 5 15:34:32 2010 Return-Path: Delivered-To: freebsd-scsi@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5B8B41065670; Sat, 5 Jun 2010 15:34:32 +0000 (UTC) (envelope-from mj@feral.com) Received: from ns1.feral.com (ns1.feral.com [192.67.166.1]) by mx1.freebsd.org (Postfix) with ESMTP id 356DF8FC08; Sat, 5 Jun 2010 15:34:31 +0000 (UTC) Received: from [172.16.135.100] (lportal.in1.lcl [172.16.1.9]) by ns1.feral.com (8.14.3/8.14.3) with ESMTP id o55FYVrj042346 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Sat, 5 Jun 2010 08:34:31 -0700 (PDT) (envelope-from mj@feral.com) Message-ID: <4C0A6E82.4050006@feral.com> Date: Sat, 05 Jun 2010 08:34:26 -0700 From: Matthew Jacob Organization: Feral Software User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9) Gecko/20100317 Thunderbird/3.0.4 MIME-Version: 1.0 To: Alexander Motin References: <4C09FD65.9010406@FreeBSD.org> In-Reply-To: <4C09FD65.9010406@FreeBSD.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.3 (ns1.feral.com [192.168.221.1]); Sat, 05 Jun 2010 08:34:31 -0700 (PDT) Cc: freebsd-scsi@FreeBSD.org Subject: Re: report luns (plus some CAM_DEBUG changes) 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: Sat, 05 Jun 2010 15:34:32 -0000 Thank you! Since I'm old and cannot think well any more, reviews are very helpful. I've never been comfortable with the refcount fiddling, and you've forced me to rethink that. New patches shortly. > Matthew Jacob wrote: > >> I'm ready to push this I think. Comments before I do? >> >> See http://people.freebsd.org/~mjacob/active_patches >> > Some comments in order of appearance: > - removing blank line from xpt_acquire_device() violates style(9). > - wouldn't "debug" sounded better the "dflags" in sysctl? > - is there reason to check CAM_DEV_INQUIRY_DATA_VALID in PROBE_REPORT_LUNS? > - in PROBE_REPORT_LUNS you are incrementing target->refcount. But who > will decrement it back, if XPT_SCAN_LUN was called directly, without > XPT_SCAN_BUS/TGT? > - while target is probably also counted by scan request and is not > going to disappear, do you think direct manipulation with > target->refcount (especially decrement) is a good policy? > - if xpt_create_path() or something else fails, I think you may leak > target->refcount. > >