Date: Sat, 05 Jun 2010 08:34:26 -0700 From: Matthew Jacob <mj@feral.com> To: Alexander Motin <mav@FreeBSD.org> Cc: freebsd-scsi@FreeBSD.org Subject: Re: report luns (plus some CAM_DEBUG changes) Message-ID: <4C0A6E82.4050006@feral.com> In-Reply-To: <4C09FD65.9010406@FreeBSD.org> References: <mailpost.1275699021.5610248.61078.mailing.freebsd.scsi@FreeBSD.cs.nctu.edu.tw> <4C09FD65.9010406@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C0A6E82.4050006>