linux-kernel - Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <65772742-c18d-7e8b-93f5-74236409d46b@axentia.se>
Date:   Thu, 6 Jul 2017 08:18:02 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Daniel Vetter <daniel@...ll.ch>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Daniel Vetter <daniel.vetter@...el.com>
Subject: Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in
 terms of crtc .gamma_set

On 2017-07-06 07:55, Daniel Vetter wrote:
> On Wed, Jul 5, 2017 at 7:50 PM, Peter Rosin <peda@...ntia.se> wrote:
>>>> +retry:
>>>> +    ret = drm_modeset_lock_all_ctx(dev, &ctx);
>>>
>>> With atomic you don't need to grab locks, this is done behind the scenes
>>> (as long as you handle the retry/backoff correctly). See the kerneldoc for
>>> the various drm_atomic_get_*_state functions.
>>
>> It doesn't work if I remove it. What is the disconnect?
> 
> Good question,

Duh, for symmetry I also removed the dropping of the locks. So the next
call of course had no chance of getting access. How silly of me...

>                didn't spot this at first, but your backoff/retry logic
> is proken. When typing drm_modeset_lock locking code please make sure
> you've enabled both CONFIG_PROVE_LOCKING and
> CONFIG_DEBUG_WW_MUTEX_SLOWPATH. Without these two it's really easy to
> get this wrong. Please also read
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#kms-locking
> carefully plus all the kernel-doc of the various hooks. This stuff is
> a really tricky locking scheme, it takes a while to understand it and
> implement it correctly. Which is why all the locking magic is in
> shared code and for normal drivers no need think about it. For the
> fundamental algorithm, you can also check out the docs for w/w mutexes
> at https://www.kernel.org/doc/Documentation/locking/ww-mutex-design.txt
> 
> Might also help to read a bunch of the other locking paths again, with
> my patches there's a few just in drm_fbdev_helper.c. I'll leave you
> with these snippets here since I think this is fun to learn, but when
> you're stuck I'm happy to help learn.

I'll take a long look at this before I send a cleaned up v4. Thanks for
the pointers...

Cheers,
peda