Date: Thu, 04 Mar 2010 11:20:17 +0200 From: Alexander Motin <mav@FreeBSD.org> To: Attilio Rao <attilio@freebsd.org> Cc: freebsd-scsi@freebsd.org, "Justin T. Gibbs" <gibbs@freebsd.org>, Matthew Jacob <mj@feral.com> Subject: Re: How is supposed to be protected the units list? Message-ID: <4B8F7B51.6050403@FreeBSD.org> In-Reply-To: <1267554183.00225132.1267543801@10.7.7.3> References: <1267417389.00224447.1267407002@10.7.7.3> <1267554183.00225132.1267543801@10.7.7.3>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi. Attilio Rao wrote: > 2010/3/1 Attilio Rao <attilio@freebsd.org>: >> I have a question that I've been unable to reply reading the code. >> Someone could point me to documentation explaining how the unit tailq >> (within a struct periph_driver) is supposed to be locked? >> I'm not sure how it is assured consistency of accesses to the list and >> more important how is ensured that the periphs composing it doesn't go >> away as I don't see any reference bump for objects inserted there. I am not sure that existing implementation is complete, but at least in some places it is protected by xpt_lock_buses(), which is wrapper for mtx_lock(&xsoftc.xpt_topo_lock). camperiphfree() called under the lock, same as many (all?) traverses. > I don't think the lists are protected at all so I made this simple > patch taking advantage by a global lock: > http://www.freebsd.org/~attilio/Sandvine/pdrv/pdrv_lock.diff > > The patch is simple enough but I just test-compiled it (will need some > time to run in a debugging kernel, hope to do tonight) and maybe you > can already give your opinions here. It is not obvious to me, how your new lock expected to interoperate with existing xpt_topo_lock. I have feeling that it duplicates one in some places. If you believe that second lock is needed there, then probably first one should be partially removed. But then we should be careful to not create LORs between them, as they are not natively nested. Changes in cam_xpt.c break splbreaknum meaning of temporary dropping xpt_topo_lock. I have doubts whether it is required now at all now, but leaving dead code it definitely not good. -- Alexander Motin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B8F7B51.6050403>