Re: VM lockdep warning

["Christian König" <deathsimple@vodafone.de>]

On 21.04.2012 19:30, Jerome Glisse wrote:
> 2012/4/21 Christian König<deathsimple@vodafone.de>:
>> On 21.04.2012 17:57, Dave Airlie wrote:
>>> 2012/4/21 Jerome Glisse<j.glisse@gmail.com>:
>>>> 2012/4/21 Christian König<deathsimple@vodafone.de>:
>>>>> On 21.04.2012 16:08, Jerome Glisse wrote:
>>>>>> 2012/4/21 Christian König<deathsimple@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.