On Tue, 2020-03-10 at 21:01 -0700, Bart Van Assche wrote: > ... > If any thread modifies any of these existing bitfields concurrently with > scsi_prep_state_check(), one of the two modifications will be lost. That > is because compilers implement bitfield changes as follows: > > new_value = (old_value & ~(1 << ...)) | (1 << ...); > > If two such assignment statements are executed concurrently, both start > from the same 'old_value' and only one of the two changes will happen. > I'm concerned that this patch introduces a maintenance problem in the > long term. Has it been considered to make 'offline_already' a 32-bits > integer variable or a boolean instead of a bitfield? I think such > variables can be modified without discarding values written from another > thread. > > Thanks, > > Bart. > I understand your concern about long-term maintenance, and introducing a pattern that could be used by other code that needs more accurate state. (Although, it is likely that a lock or an atomic operation might be required in other cases). I'll post a v2 changing this to a boolean. -Ewan
