On 24 November 2014 at 14:50, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote: > Yanbo Li <yanbol@xxxxxxxxxxxxxxxx> writes: [...] >> + spin_lock_bh(&ar->data_lock); >> + reg_addr = ar->debug.reg_addr; >> + spin_unlock_bh(&ar->data_lock); > > [...] > >> + spin_lock_bh(&ar->data_lock); >> + ar->debug.reg_addr = reg_addr; >> + spin_unlock_bh(&ar->data_lock); > > [...] > >> + spin_lock_bh(&ar->data_lock); >> + reg_addr = ar->debug.reg_addr; >> + spin_unlock_bh(&ar->data_lock); > > [...] > >> + spin_lock_bh(&ar->data_lock); >> + reg_addr = ar->debug.reg_addr; >> + spin_unlock_bh(&ar->data_lock); > > I admit that I'm far from a locking expert, but does that make any > difference in the functionality? I imagine reg_addr could end up needing 2 instructions to be set on some weird architecture+compiler combination in which case locking is desired. With multiple access to write reg_addr you could end up reading an address that either isn't the old nor the new one. Should be harmless though and you could argue that you shouldn't access the reg_addr debugfs file simultaneously in the first place. Maybe I'm just over-exaggerating? :-) Michał -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
