Eric Dumazet wrote: > Gregory Haskins a écrit : >> Gregory Haskins wrote: >>> Eric Dumazet wrote: >>>> Michael S. Tsirkin a écrit : >>>> using rcu_dereference() and mutex_lock() at the same time seems wrong, I suspect >>>> that your use of RCU is not correct. >>>> >>>> 1) rcu_dereference() should be done inside a read_rcu_lock() section, and >>>> we are not allowed to sleep in such a section. >>>> (Quoting Documentation/RCU/whatisRCU.txt : >>>> It is illegal to block while in an RCU read-side critical section, ) >>>> >>>> 2) mutex_lock() can sleep (ie block) >>>> >>> Michael, >>> I warned you that this needed better documentation ;) >>> >>> Eric, >>> I think I flagged this once before, but Michael convinced me that it >>> was indeed "ok", if but perhaps a bit unconventional. I will try to >>> find the thread. >>> >>> Kind Regards, >>> -Greg >>> >> Here it is: >> >> http://lkml.org/lkml/2009/8/12/173 >> > > Yes, this doesnt convince me at all, and could be a precedent for a wrong RCU use. > People wanting to use RCU do a grep on kernel sources to find how to correctly > use RCU. > > Michael, please use existing locking/barrier mechanisms, and not pretend to use RCU. Yes, I would tend to agree with you. In fact, I think I suggested that a normal barrier should be used instead of abusing rcu_dereference(). But as far as his code is concerned, I think it technically works properly, and that was my main point. Also note that the usage rcu_dereference+mutex_lock() are not necessarily broken, per se: it could be an srcu-based critical section created by the caller, for instance. It would be perfectly legal to sleep on the mutex if that were the case. To me, the bigger issue is that the rcu_dereference() without any apparent hint of a corresponding RSCS is simply confusing as a reviewer. smp_rmb() (or whatever is proper in this case) is probably more appropriate. Kind Regards, -Greg
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/virtualization
