On Tue, Mar 19, 2019 at 08:06:48AM -0700, Bart Van Assche wrote: > On Tue, 2019-03-19 at 16:38 +0800, Ming Lei wrote: > > Inside sbitmap_queue_clear(), once the clear bit is set, it will be > > visiable to allocation path immediately. Meantime READ/WRITE on old > > associated instance(such as request in case of blk-mq) may be > > out-of-order with the setting clear bit, so race with re-allocation > > may be triggered. > > > > Adds one memory barrier for ordering READ/WRITE of the freed associated > > instance with setting clear bit for avoiding race with re-allocation. > > > > The following kernel oops triggerd by block/006 on aarch64 may be fixed: > ^^^ > > Does that mean that it has not been verified whether this patch fixes the > NULL pointer issue mentioned in the patch description? V1 has been verified by Zhang Yi, but V2 isn't done yet. > > > diff --git a/lib/sbitmap.c b/lib/sbitmap.c > > index 5b382c1244ed..941a46495e12 100644 > > --- a/lib/sbitmap.c > > +++ b/lib/sbitmap.c > > @@ -591,6 +591,17 @@ EXPORT_SYMBOL_GPL(sbitmap_queue_wake_up); > > void sbitmap_queue_clear(struct sbitmap_queue *sbq, unsigned int nr, > > unsigned int cpu) > > { > > + /* > > + * Once the clear bit is set, the bit may be allocated out. > ^^^^^^^^^^^^^^^^^^^^^^^^^ > > This comment is confusing. Did you perhaps mean "Once the bit is cleared"? No, please take at look at sbitmap_deferred_clear_bit(). > > > + * > > + * Orders READ/WRITE on the asssociated instance(such as request > > + * of blk_mq) by this bit for avoiding race with re-allocation, > > + * and its pair is the memory barrier implied in __sbitmap_get_word. > > + * > > + * One invarient is that the clear bit has to be zero when the bit > ^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > invariant? I cant' make sense of this. It is really 'clear bit', again please look at sbitmap_deferred_clear_bit(). Thanks, Ming