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: <CAKMK7uE=krvHYWEjOxuVdzwGVwYfyr5pEpbHiyt3e5YYkH+n_g@mail.gmail.com>
Date:   Thu, 6 Jul 2017 07:55:35 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Peter Rosin <peda@...ntia.se>
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 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, 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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch