On Mon, Oct 31, 2011 at 10:33 AM, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > On Fri, 2011-10-21 at 18:57 +0200, Bart Van Assche wrote: > > Register transport attributes after the attribute array has been > > set up instead of before. The current code is racy because there > > is no guarantee that the CPU examining the attribute container > > will see all values written to the container. > > I don't agree with this change log. As far as the kernel is concerned, > nothing happens until that function returns because the only way to use > anything is to get a match to succeed, and they all check for > ->transportt which will be NULL. > > I can accept that it's best practise to initialise something before > registering it, because the reverse excites everyone's bogosity sensors, > it's just not a bug in this case. It's very well possible that I have missed something, but it's not clear to me what's wrong with the patch description ? At least with ib_srp device registration happens from another context than module initialization so it can potentially run on another CPU. I don't see any kind of memory barrier after the initialization of the SRP transport attribute array. So at least in theory and on an architecture with a sufficiently weak memory model (e.g. the POWER architecture) there is no guarantee that these store operations will be observed before an SRP device gets registered. Even more, there is no guarantee that the CPU on which device registration happens will observe these store operations in the same order as these were performed on the CPU that ran srp_attach_transport(). Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
