Date: Sun, 21 Mar 2010 17:22:21 +0100 From: Attilio Rao <attilio@freebsd.org> To: Scott Long <scottl@samsco.org> Cc: freebsd-scsi@freebsd.org, Alexander Motin <mav@freebsd.org>, "Justin T. Gibbs" <gibbs@freebsd.org>, mj@feral.com, Ed Maste <emaste@sandvine.com> Subject: Re: How is supposed to be protected the units list? Message-ID: <3bbf2fe11003210922w7f9a9fb7t2d3b4f69b9251484@mail.gmail.com> In-Reply-To: <F5071E1F-3847-49E6-82E9-96980E7D1FAC@samsco.org> References: <3bbf2fe11002281655i61a5f0a0if3f381ad0c4a1ef8@mail.gmail.com> <3bbf2fe11003031607wa3727b5ke89bc2a909d4d6a6@mail.gmail.com> <4B901419.8060800@feral.com> <3bbf2fe11003041737p30690522ya81e1b8f4bd6bbf9@mail.gmail.com> <3bbf2fe11003120601y3c403a1ct50f9fc6c1f0903bf@mail.gmail.com> <4B9A91DA.7030107@FreeBSD.org> <3bbf2fe11003200523t60895bfv1fa73d04e58a7838@mail.gmail.com> <4BA5C746.7060203@FreeBSD.org> <3bbf2fe11003210902g2db270bdp45b1f4dfe2996b11@mail.gmail.com> <F5071E1F-3847-49E6-82E9-96980E7D1FAC@samsco.org>
next in thread | previous in thread | raw e-mail | index | archive | help
2010/3/21 Scott Long <scottl@samsco.org>: > > On Mar 21, 2010, at 10:02 AM, Attilio Rao wrote: > >> 2010/3/21 Alexander Motin <mav@freebsd.org>: >>> Attilio Rao wrote: >>>> So I made this new patch using the bus lock: >>>> http://www.freebsd.org/~attilio/Sandvine/pdrv/xpt_lock.diff >>> >>> OK. I've looked on both and I think both have race window between unit >>> number allocation and insertion into the list. I've changed last patch >>> to not drop the lock in meantime. What do you think about this: >>> http://people.freebsd.org/~mav/unit_lock.patch >>> ? >>> >>> Part about scsi_da.c I don't like in both cases, as I am not sure that >>> locks can't be recursed there in case of some errors. I don't see how >>> adding second lock could solve it. >> >> I think that we should protect there in anyway. >> Probabilly we may cache the list and refcount the periphs? (then >> unlock the lock and just do the lockless operation in order to avoid >> recursion?) >> The race handling for units allocation is fine, thanks. >> > > Please give me a few more days to review and comment on this. =C2=A0Alexa= nder's change is very invasive in that it re-orders operations, and I'd lik= e to review it to ensure that that doesn't break existing ordering assumpti= ons. > Anyways, we should not undervalue the shutdown case for scsi_da, it is easilly race prone as Matt has proven. If you could comment on it, also, I would appreciate. Thanks, Attilio --=20 Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe11003210922w7f9a9fb7t2d3b4f69b9251484>