On 28/04/2018 04:24, Peter Xu wrote: >>>> 2) Can we just reuse qemu BQL here? >>> I would prefer not. As I mentioned, at least I have spent too much >>> time on fighting BQL already. I really hope we can start to use >>> isolated locks when capable. BQL is always the worst choice to me. >> Just a thought, using BQL may greatly simplify the code actually (consider >> we don't plan to remove BQL now). > Frankly speaking I don't understand why using BQL may greatly simplify > the code... :( IMHO the lock here is really not a complicated one. > > Note that IMO BQL is mostly helpful when we really want something to > be run sequentially with some other things _already_ protected by BQL. > In this case, all the stuff is inside VT-d code itself (or other > IOMMUs), why bother taking the BQL to make our life harder? > > So, even if we want to provide a general lock for the translation > procedure, I would prefer we add a per AddressSpace lock but not BQL. > However still that will need some extra flag showing that whether we > need the protection of not. For example, we may need to expliclitly > turn that off for Power and s390. Would that really worth it? > > So my final preference is still current patch - we solve thread-safety > problems in VT-d and IOMMU code. Again, we really should make sure > all IOMMUs work with multithreads. >
I agree. In particular, using BQL is _worse_ because it has very strict lock ordering requirements. Using fine-grained locks is greatly preferred as long as: 1) they are leaves in the lock ordering 2) they are not kept across calls to external callbacks (or there are no external callbacks involved) Paolo