On Tue, 2008-07-29 at 13:32 +0400, Vladislav Bolkhovitin wrote: > Nope, taking only one that hunk from this patch isn't sufficient. > Around > dpc_thread there is pretty simple and classical race. You can't do > > if (x != NULL) > y = *x; > > without any protection, if x can be set to NULL by another thread. It > can happen exactly between "if" and "*x" and hence lead to a crash, > correct? No. Today we go to reasonable lengths to avoid additional locking. Spinlocks are nasty in most boxes because of the potential for introducing hot points and cacheline bouncing. The first question you ask is how hot is the variable ... as in how constantly changing is it? This one is set at start of day and cleared when the thread is killed in shutdown. Basically there's a single not NULL -> NULL transition. Once NULL, it never goes back to not NULL. The point is that in this particular circumstance, the lock is overkill. You can either use other indicators to show that the driver is being removed (and the thread is dying) or you can simply use some type of refcounting to ensure the thread isn't killed until it's no longer needed. Even if it were a constantly changing variable, and therefore a more ideal candidate for locking, we might still look into atomics and bitops before adding a lock. James -- 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
