On 21.04.2012 19:30, Jerome Glisse wrote: > 2012/4/21 Christian K?nig<deathsimple at vodafone.de>: >> On 21.04.2012 17:57, Dave Airlie wrote: >>> 2012/4/21 Jerome Glisse<j.glisse at gmail.com>: >>>> 2012/4/21 Christian K?nig<deathsimple at vodafone.de>: >>>>> On 21.04.2012 16:08, Jerome Glisse wrote: >>>>>> 2012/4/21 Christian K?nig<deathsimple at vodafone.de>: >>>>>>> Interesting, I'm pretty sure that I haven't touched the locking order >>>>>>> of >>>>>>> the >>>>>>> cs_mutex vs. vm_mutex. >>>>>>> >>>>>>> Maybe it is just some kind of side effect, going to locking into it >>>>>>> anyway. >>>>>>> >>>>>>> Christian. >>>>>>> >>>>>> It's the using, init path take lock in different order than cs path >>>>> Well, could you explain to me why the vm code takes cs mutex in the >>>>> first >>>>> place? >>>>> >>>>> It clearly has it's own mutex and it doesn't looks like that it deals >>>>> with >>>>> any cs related data anyway. >>>>> >>>>> Christian. >>>> Lock simplification is on my todo. The issue is that vm manager is >>>> protected by >>>> cs_mutex The vm.mutex is specific to each vm it doesn't protect the >>>> global vm >>>> management. I didn't wanted to introduce a new global vm mutex as vm >>>> activity >>>> is mostly trigger on behalf of cs so i dediced to use the cs mutex. >>>> >>>> That's why non cs path of vm need to take the cs mutex. >>> So if one app is adding a bo, and another doing CS, isn't deadlock a >>> real possibility? >> Yeah, I think so. > No it's not. Look at the code. > >>> I expect the VM code need to take CS mutex earlier then. > No it does not. The idea is that when adding a bo we only need to take the > cs mutex if we need to resize the vm size (and even that can be worked > around). > > So we will need to take the cs ioctl in very few case (suspend, increasing vm > size). > >> I would strongly suggest to give the vm code their own global mutex and >> remove the per vm mutex, cause the later is pretty superfluous if the >> cs_mutex is also taken most of the time. >> >> The attached patch is against drm-fixes and does exactly that. >> >> Christian. > NAK with your change there will be lock contention if one app is in cs and > another try to create bo. Currently there is allmost never contention. Once > i ironed out the DP->VGA i will work on something to remove the cs mutex > from vm path (ie remove it from bo creation/del path).
Ok, sounds like I don't understand the code deeply enough to fix this. So I'm just going to wait for your fix. By the way: If you are talking about the NUTMEG DP->VGA problem, I have two systems with that sitting directly beside me. So if you got any patches just leave me a note and I can try them. Christian.
