On Tuesday March 4, maan@xxxxxxxxxxxxxxx wrote: > On 17:08, Neil Brown wrote: > > > Do we really need to take the spin lock in the common case where > > > conf->pending_bio_list.head is NULL? If not, the above could be > > > optimized to the slightly faster and better readable > > > > > > struct bio *bio; > > > > > > if (!conf->pending_bio_list.head) > > > return 0; > > > spin_lock_irq(&conf->device_lock); > > > bio = bio_list_get(&conf->pending_bio_list); > > > ... > > > spin_unlock_irq(&conf->device_lock); > > > return 1; > > > > Maybe... If I write a memory location inside a spinlock, then after > > the spinlock is dropped, I read that location on a different CPU, > > am I always guaranteed to see the new value? or do I need some sort of > > memory barrier? > > Are you worried about another CPU setting conf->pending_bio_list.head > to != NULL after the if statement? If that's an issue I think also > the original patch is problematic because the same might happen after > the final spin_unlock_irq() but but before flush_pending_writes() > returns zero. No. I'm worried that another CPU might set conf->pending_bio_list.head *before* the if statement, but it isn't seen by this CPU because of the lack of memory barriers. The spinlock ensures that the memory state is consistent. It is possible that I am being overcautious. But I think that is better than the alternative. Thanks, NeilBrown -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
