Hi, On Wed, 8 Jul 2020 at 16:13, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > On Wed, Jul 8, 2020 at 4:57 PM Christian König <christian.koenig@xxxxxxx> wrote: > > Could we merge this controlled by a separate config option? > > > > This way we could have the checks upstream without having to fix all the > > stuff before we do this? > > Since it's fully opt-in annotations nothing blows up if we don't merge > any annotations. So we could start merging the first 3 patches. After > that the fun starts ... > > My rough idea was that first I'd try to tackle display, thus far > there's 2 actual issues in drivers: > - amdgpu has some dma_resv_lock in commit_tail, plus a kmalloc. I > think those should be fairly easy to fix (I'd try a stab at them even) > - vmwgfx has a full on locking inversion with dma_resv_lock in > commit_tail, and that one is functional. Not just reading something > which we can safely assume to be invariant anyway (like the tmz flag > for amdgpu, or whatever it was). > > I've done a pile more annotations patches for other atomic drivers > now, so hopefully that flushes out any remaining offenders here. Since > some of the annotations are in helper code worst case we might need a > dev->mode_config.broken_atomic_commit flag to disable them. At least > for now I have 0 plans to merge any of these while there's known > unsolved issues. Maybe if some drivers take forever to get fixed we > can then apply some duct-tape for the atomic helper annotation patch. > Instead of a flag we can also copypasta the atomic_commit_tail hook, > leaving the annotations out and adding a huge warning about that. How about an opt-in drm_driver DRIVER_DEADLOCK_HAPPY flag? At first this could just disable the annotations and nothing else, but as we see the annotations gaining real-world testing and maturity, we could eventually make it taint the kernel. Cheers, Daniel