Re: [PATCH v5 01/21] gpu: host1x: Use different lock classes for each client

Dmitry Osipenko
2021-03-23T13:25:47Z

23.03.2021 13:20, Thierry Reding пишет:
> On Mon, Mar 22, 2021 at 07:01:34PM +0300, Dmitry Osipenko wrote:
>> 22.03.2021 18:19, Mikko Perttunen пишет:
>>> On 22.3.2021 16.48, Dmitry Osipenko wrote:
>>>> 22.03.2021 17:46, Thierry Reding пишет:
>>>>> On Mon, Jan 11, 2021 at 02:59:59PM +0200, Mikko Perttunen wrote:
>>>>>> To avoid false lockdep warnings, give each client lock a different
>>>>>> lock class, passed from the initialization site by macro.
>>>>>>
>>>>>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>>>>>> ---
>>>>>>   drivers/gpu/host1x/bus.c | 7 ++++---
>>>>>>   include/linux/host1x.h   | 9 ++++++++-
>>>>>>   2 files changed, 12 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
>>>>>> index 347fb962b6c9..8fc79e9cb652 100644
>>>>>> --- a/drivers/gpu/host1x/bus.c
>>>>>> +++ b/drivers/gpu/host1x/bus.c
>>>>>> @@ -715,13 +715,14 @@ EXPORT_SYMBOL(host1x_driver_unregister);
>>>>>>    * device and call host1x_device_init(), which will in turn call
>>>>>> each client's
>>>>>>    * &host1x_client_ops.init implementation.
>>>>>>    */
>>>>>> -int host1x_client_register(struct host1x_client *client)
>>>>>> +int __host1x_client_register(struct host1x_client *client,
>>>>>> +               struct lock_class_key *key)
>>>>>
>>>>> I've seen the kbuild robot warn about this because the kerneldoc is now
>>>>> out of date.
>>>>>
>>>>>>   {
>>>>>>       struct host1x *host1x;
>>>>>>       int err;
>>>>>>         INIT_LIST_HEAD(&client->list);
>>>>>> -    mutex_init(&client->lock);
>>>>>> +    __mutex_init(&client->lock, "host1x client lock", key);
>>>>>
>>>>> Should we maybe attempt to make this unique? Could we use something like
>>>>> dev_name(client->dev) for this?
>>>>
>>>> I'm curious who the lockdep warning could be triggered at all, I don't
>>>> recall ever seeing it. Mikko, could you please clarify how to reproduce
>>>> the warning?
>>>>
>>>
>>> This is pretty difficult to read but I guess it's some interaction
>>> related to the delayed initialization of host1x clients? In any case, I
>>> consistently get it at boot (though it may be triggered by vic probe
>>> instead of nvdec).
>>>
>>> I'll fix the kbuild robot warnings and see if I can add a
>>> client-specific lock name for v6.
>>
>> Thank you for the clarification! We now actually have a similar problem on Tegra20 after fixing the coupling of display controllers using the dc1_client->parent=dc0_client and I see the same warning when DC1 is enabled.
>>
...
> Sounds like we should decouple this from the series and fast-track this
> for v5.13, or perhaps even v5.12 along with the DC coupling fix?

Agree that the patch should be decoupled since it fixes a standalone
problem that isn't related to the rest of the patches.

It also should be good to have it backported, although this is optional.
If there are no merge conflicts with stable kernels for this patch, then
I'd add a stable tag to it.

Mikko, please update this patch and send it separately.