On Thu, 2019-05-09 at 16:51 -0700, Jae Hyun Yoo wrote: > On 5/9/2019 4:19 PM, Benjamin Herrenschmidt wrote: > > On Thu, 2019-05-09 at 10:19 -0700, Jae Hyun Yoo wrote: > > > > > > Okay. Probably I need to add one another patch in this series to > > > address > > > what you pointed out. > > > > > > I have one question. I reviewed again all bitops in this driver > > > and > > > checked that some bitops are protected by a spinlock and some > > > others > > > are not. In this case, can I mix use atomic and non-atomic bitops > > > depend on each bitop's protection state by the spinlock? Or, > > > would it be > > > better to change all of them to bool in this case? > > > > No, if some aren't protected by a lock and some are, then they need > > to > > remain atomic. > > > > The question then becomes whether the unprotected ones are actually > > correct or just exposing more races. > > Got it. Not sure yet but I think the protected bitops could be moved > out > from the spinlock scope then all bitops could be kept as atomic. Which is very likely to be extremely racy... (and gratuitously more costly) :-) > I'll > look at and test this driver code more deeply again, and will submit > v2 > soon. > > Again, thanks a lot for your review. > > Regards, > Jae