On Tue 05-12-23 21:22:59, Yury Norov wrote: > On Mon, Dec 04, 2023 at 07:51:01PM +0100, Jan Kara wrote: > > > This series is a result of discussion [1]. All find_bit() functions imply > > > exclusive access to the bitmaps. However, KCSAN reports quite a number > > > of warnings related to find_bit() API. Some of them are not pointing > > > to real bugs because in many situations people intentionally allow > > > concurrent bitmap operations. > > > > > > If so, find_bit() can be annotated such that KCSAN will ignore it: > > > > > > bit = data_race(find_first_bit(bitmap, nbits)); > > > > No, this is not a correct thing to do. If concurrent bitmap changes can > > happen, find_first_bit() as it is currently implemented isn't ever a safe > > choice because it can call __ffs(0) which is dangerous as you properly note > > above. I proposed adding READ_ONCE() into find_first_bit() / find_next_bit() > > implementation to fix this issue but you disliked that. So other option we > > have is adding find_first_bit() and find_next_bit() variants that take > > volatile 'addr' and we have to use these in code like xas_find_chunk() > > which cannot be converted to your new helpers. > > Here is some examples when concurrent operations with plain find_bit() > are acceptable: > > - two threads running find_*_bit(): safe wrt ffs(0) and returns correct > value, because underlying bitmap is unchanged; > - find_next_bit() in parallel with set or clear_bit(), when modifying > a bit prior to the start bit to search: safe and correct; > - find_first_bit() in parallel with set_bit(): safe, but may return wrong > bit number; > - find_first_zero_bit() in parallel with clear_bit(): same as above. > > In last 2 cases find_bit() may not return a correct bit number, but > it may be OK if caller requires any (not exactly first) set or clear > bit, correspondingly. > > In such cases, KCSAN may be safely silenced. True - but these are special cases. In particular the case in xas_find_chunk() is not any of these special cases. It is using find_next_bit() which is can be racing with clear_bit(). So what are your plans for such usecase? Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
