On Wed, Aug 3, 2011 at 4:35 PM, NeilBrown <neilb@xxxxxxx> wrote: > On Wed, 3 Aug 2011 15:47:47 -0700 Dan Williams <dan.j.williams@xxxxxxxxx> > wrote: > >> On Wed, Jul 20, 2011 at 7:32 PM, NeilBrown <neilb@xxxxxxx> wrote: >> > sh->lock is now mainly used to ensure that two threads aren't running >> > in the locked part of handle_stripe[56] at the same time. >> > >> > That can more neatly be achieved with an 'active' flag which we set >> > while running handle_stripe. If we find the flag is set, we simply >> > requeue the stripe for later by setting STRIPE_HANDLE. >> > >> > For safety we take ->device_lock while examining the state of the >> > stripe and creating a summary in 'stripe_head_state / r6_state'. >> > This possibly isn't needed but as shared fields like ->toread, >> > ->towrite are checked it is safer for now at least. >> > >> > We leave the label after the old 'unlock' called "unlock" because it >> > will disappear in a few patches, so renaming seems pointless. >> > >> > This leaves the stripe 'locked' for longer as we clear STRIPE_ACTIVE >> > later, but that is not a problem. >> > >> >> This removal reminds me of one thing I have wondered about, but to >> date have not found the time to measure (maybe someone might beat me >> to it if the idea is out there), is what is the overhead of all the >> atomic operations that raid5.c generates? If we can guarantee that >> certain updates only happen under sh->lock (now STRIPE_ACTIVE) can we >> downgrade set_bit and clear_bit to their non-atomic __set_bit and >> __clear_bit versions and recover some cpu cycles? >> > > You can only used the unlocked version if you know that no other CPU will > change any of the bits in the 'unsigned long'. As STRIPE_HANDLE can be set > at any time, I think all accesses to sh->state must be atomic. > > However 'struct stripe_head_state' is thread-local so setting/clearing flags > in ops_request can probably benefit from non-atomic ops. I was particularly eyeing 'flags' in struct r5dev... -- 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
