On Fri, Jul 12, 2024 at 12:33 AM Bart Van Assche <bvanassche@xxxxxxx> wrote: > > On 7/11/24 6:39 AM, Ming Lei wrote: > > There are only two WRITE on 'cleared': > > > > - xchg(&map->cleared, 0) in sbitmap_deferred_clear() > > > > - set_bit() in sbitmap_deferred_clear_bit() > > > > xchg() supposes to provide such protection already. > > Hi Ming, > > The comment above 'swap_lock' in this patch is as follows: > > /** > * @swap_lock: Held while swapping word <-> cleared > */ > > In other words, 'swap_lock' is used to serialize *code*. Using > synchronization objects to serialize code is known as an anti-pattern, > something that shouldn't be done. > Synchronization objects should be used > to serialize access to data. It can be thought of serializing UPDATE on both ->word and ->cleared, instead of code. > Hence my question whether it would be > appropriate to protect all 'cleared' changes with the newly introduced > spinlock. Wrt. ->cleared only update, xchag() is enough. Thanks, Ming
