On Mon, Apr 30, 2018 at 09:20:42AM +0200, Paolo Bonzini wrote: > 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)
Thanks Paolo for the input. I'll temporarily keep this patch in my next post. We can further discuss it there if we have better alternatives. Regards, -- Peter Xu