1. 26 Mar, 2020 10 commits
    • Daniel Vetter's avatar
      drm/cirrus: Use drmm_add_final_kfree · b6097727
      Daniel Vetter authored
      With this we can drop the final kfree from the release function.
      
      I also noticed that cirrus forgot to call drm_dev_fini().
      
      v2: Don't call kfree(cirrus) after we've handed overship of that to
      drm_device and the drmm_ stuff.
      Acked-by: default avatarGerd Hoffmann <kraxel@redhat.com>
      Acked-by: default avatarSam Ravnborg <sam@ravnborg.org>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Cc: Dave Airlie <airlied@redhat.com>
      Cc: Gerd Hoffmann <kraxel@redhat.com>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: "Noralf Trønnes" <noralf@tronnes.org>
      Cc: Linus Walleij <linus.walleij@linaro.org>
      Cc: Sam Ravnborg <sam@ravnborg.org>
      Cc: Thomas Zimmermann <tzimmermann@suse.de>
      Cc: virtualization@lists.linux-foundation.org
      Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-10-daniel.vetter@ffwll.ch
      b6097727
    • Daniel Vetter's avatar
      drm/i915: Use drmm_add_final_kfree · 7fb81e9d
      Daniel Vetter authored
      With this we can drop the final kfree from the release function.
      
      The mock device in the selftests needed it's pci_device split
      up from the drm_device. In the future we could simplify this again
      by allocating the pci_device as a managed allocation too.
      
      v2: I overlooked that i915_driver_destroy is also called in the
      unwind code of the error path. There we need a drm_dev_put.
      Similar for the mock object.
      
      Now the problem with that is that the drm_driver->release callbacks
      for both the real driver and the mock one assume everything has been
      set up. Hence going through that path for a partially set up driver
      will result in issues. Quickest fix is to disable the ->release() hook
      until the driver is fully initialized, and keep the onion unwinding.
      Long term would be cleanest to move everything over to drmm_ release
      actions, but that's a lot of work for a big driver like i915. Plus
      more core work needed first anyway.
      
      v3: Fix i915_drm pointer wrangling in mock_gem_device. Also switch
      over to start using drm_dev_put() to clean up even on the error path.
      Aside I think the current error path is leaking the allocation.
      
      v4: more fixes for intel-gfx-ci, some if it damage from v3 :-/
      Reviewed-by: default avatarJani Nikula <jani.nikula@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Cc: Jani Nikula <jani.nikula@linux.intel.com>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
      Cc: Matthew Auld <matthew.auld@intel.com>
      Cc: Andi Shyti <andi.shyti@intel.com>
      Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
      Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
      Cc: intel-gfx@lists.freedesktop.org
      Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-9-daniel.vetter@ffwll.ch
      7fb81e9d
    • Maarten Lankhorst's avatar
      Merge remote-tracking branch 'drm/drm-next' into drm-misc-next · c1b164a5
      Maarten Lankhorst authored
      Requested for getting some i915 fixes back into drm-misc-next by danvet.
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      c1b164a5
    • Daniel Vetter's avatar
      drm/qxl: Use drmm_add_final_kfree · 873863b6
      Daniel Vetter authored
      With this we can drop the final kfree from the release function.
      Acked-by: default avatarGerd Hoffmann <kraxel@redhat.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Cc: Dave Airlie <airlied@redhat.com>
      Cc: Gerd Hoffmann <kraxel@redhat.com>
      Cc: virtualization@lists.linux-foundation.org
      Cc: spice-devel@lists.freedesktop.org
      Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-8-daniel.vetter@ffwll.ch
      873863b6
    • Daniel Vetter's avatar
      drm/udl: Use drmm_add_final_kfree · d0c116ad
      Daniel Vetter authored
      With this we can drop the final kfree from the release function.
      
      v2: We need drm_dev_put to unroll the driver creation (once
      drm_dev_init and drmm_add_final_kfree suceeded), otherwise
      the drmm_ magic doesn't happen.
      
      v3: Actually squash in the fixup (Laurent).
      Acked-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
      Acked-by: default avatarSam Ravnborg <sam@ravnborg.org>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
      Cc: Dave Airlie <airlied@redhat.com>
      Cc: Sean Paul <sean@poorly.run>
      Cc: Thomas Zimmermann <tzimmermann@suse.de>
      Cc: Emil Velikov <emil.l.velikov@gmail.com>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: "Noralf Trønnes" <noralf@tronnes.org>
      Cc: Thomas Gleixner <tglx@linutronix.de>
      Cc: Sam Ravnborg <sam@ravnborg.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-7-daniel.vetter@ffwll.ch
      d0c116ad
    • Daniel Vetter's avatar
      drm/mipi_dbi: Use drmm_add_final_kfree in all drivers · f5ad671b
      Daniel Vetter authored
      They all share mipi_dbi_release so we need to switch them all
      together. With this we can drop the final kfree from the release
      function.
      
      Aside, I think we could perhaps have a tiny additional helper for
      these mipi_dbi drivers, the first few lines around devm_drm_dev_init
      are all the same (except for the drm_driver pointer).
      Acked-by: default avatarSam Ravnborg <sam@ravnborg.org>
      Reviewed-by: default avatarNoralf Trønnes <noralf@tronnes.org>
      Tested-by: default avatarNoralf Trønnes <noralf@tronnes.org>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Maxime Ripard <mripard@kernel.org>
      Cc: Thomas Zimmermann <tzimmermann@suse.de>
      Cc: David Airlie <airlied@linux.ie>
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Cc: Eric Anholt <eric@anholt.net>
      Cc: David Lechner <david@lechnology.com>
      Cc: Kamlesh Gurudasani <kamlesh.gurudasani@gmail.com>
      Cc: "Noralf Trønnes" <noralf@tronnes.org>
      Cc: Sam Ravnborg <sam@ravnborg.org>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-6-daniel.vetter@ffwll.ch
      f5ad671b
    • Daniel Vetter's avatar
      drm: Set final_kfree in drm_dev_alloc · 6f365e56
      Daniel Vetter authored
      I also did a full review of all callers, and only the xen driver
      forgot to call drm_dev_put in the failure path. Fix that up too.
      
      v2: I noticed that xen has a drm_driver.release hook, and uses
      drm_dev_alloc(). We need to remove the kfree from
      xen_drm_drv_release().
      
      bochs also has a release hook, but leaked the drm_device ever since
      
      commit 0a6659bd
      Author: Gerd Hoffmann <kraxel@redhat.com>
      Date:   Tue Dec 17 18:04:46 2013 +0100
      
          drm/bochs: new driver
      
      This patch here fixes that leak.
      
      Same for virtio, started leaking with
      
      commit b1df3a2b
      Author: Gerd Hoffmann <kraxel@redhat.com>
      Date:   Tue Feb 11 14:58:04 2020 +0100
      
          drm/virtio: add drm_driver.release callback.
      Acked-by: default avatarGerd Hoffmann <kraxel@redhat.com>
      Acked-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
      Acked-by: default avatarSam Ravnborg <sam@ravnborg.org>
      Reviewed-by: default avatarOleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
      Cc: Gerd Hoffmann <kraxel@redhat.com>
      Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
      Cc: xen-devel@lists.xenproject.org
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Maxime Ripard <mripard@kernel.org>
      Cc: Thomas Zimmermann <tzimmermann@suse.de>
      Cc: David Airlie <airlied@linux.ie>
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
      Cc: xen-devel@lists.xenproject.org
      Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-5-daniel.vetter@ffwll.ch
      6f365e56
    • Daniel Vetter's avatar
      drm: add managed resources tied to drm_device · c6603c74
      Daniel Vetter authored
      We have lots of these. And the cleanup code tends to be of dubious
      quality. The biggest wrong pattern is that developers use devm_, which
      ties the release action to the underlying struct device, whereas
      all the userspace visible stuff attached to a drm_device can long
      outlive that one (e.g. after a hotunplug while userspace has open
      files and mmap'ed buffers). Give people what they want, but with more
      correctness.
      
      Mostly copied from devres.c, with types adjusted to fit drm_device and
      a few simplifications - I didn't (yet) copy over everything. Since
      the types don't match code sharing looked like a hopeless endeavour.
      
      For now it's only super simplified, no groups, you can't remove
      actions (but kfree exists, we'll need that soon). Plus all specific to
      drm_device ofc, including the logging. Which I didn't bother to make
      compile-time optional, since none of the other drm logging is compile
      time optional either.
      
      One tricky bit here is the chicken&egg between allocating your
      drm_device structure and initiliazing it with drm_dev_init. For
      perfect onion unwinding we'd need to have the action to kfree the
      allocation registered before drm_dev_init registers any of its own
      release handlers. But drm_dev_init doesn't know where exactly the
      drm_device is emebedded into the overall structure, and by the time it
      returns it'll all be too late. And forcing drivers to be able clean up
      everything except the one kzalloc is silly.
      
      Work around this by having a very special final_kfree pointer. This
      also avoids troubles with the list head possibly disappearing from
      underneath us when we release all resources attached to the
      drm_device.
      
      v2: Do all the kerneldoc at the end, to avoid lots of fairly pointless
      shuffling while getting everything into shape.
      
      v3: Add static to add/del_dr (Neil)
      Move typo fix to the right patch (Neil)
      
      v4: Enforce contract for drmm_add_final_kfree:
      
      Use ksize() to check that the drm_device is indeed contained somewhere
      in the final kfree(). Because we need that or the entire managed
      release logic blows up in a pile of use-after-frees. Motivated by a
      discussion with Laurent.
      
      v5: Review from Laurent:
      - %zu instead of casting size_t
      - header guards
      - sorting of includes
      - guarding of data assignment if we didn't allocate it for a NULL
        pointer
      - delete spurious newline
      - cast void* data parameter correctly in ->release call, no idea how
        this even worked before
      
      v6: Review from Sam
      - Add the kerneldoc for the managed sub-struct back in, even if it
        doesn't show up in the generated html somehow.
      - Explain why __always_inline.
      - Fix bisectability around the final kfree() in drm_dev_relase(). This
        is just interim code which will disappear again.
      - Some whitespace polish.
      - Add debug output when drmm_add_action or drmm_kmalloc fail.
      
      v7: My bisectability fix wasn't up to par as noticed by smatch.
      
      v8: Remove unecessary {} around if else
      
      v9: Use kstrdup_const, which requires kfree_const and introducing a free_dr()
      helper (Thomas).
      
      v10: kfree_const goes boom on the plain "kmalloc" assignment, somehow
      we need to wrap that in kstrdup_const() too!! Also renumber revision
      log, I somehow reset it midway thruh.
      Reviewed-by: default avatarSam Ravnborg <sam@ravnborg.org>
      Cc: Thomas Zimmermann <tzimmermann@suse.de>
      Cc: Dan Carpenter <dan.carpenter@oracle.com>
      Cc: Sam Ravnborg <sam@ravnborg.org>
      Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
      Cc: Neil Armstrong <narmstrong@baylibre.com
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Cc: "Rafael J. Wysocki" <rafael@kernel.org>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20200324124540.3227396-1-daniel.vetter@ffwll.ch
      c6603c74
    • Daniel Vetter's avatar
      drm/i915: Don't clear drvdata in ->release · 0ce542f7
      Daniel Vetter authored
      For two reasons:
      
      - The driver core clears this already for us after we're unloaded in
        __device_release_driver().
      
      - It's way too late, the drm_device ->release callback might massively
        outlive the underlying physical device, since a drm_device can be
        kept alive by open drm_file or well really anything else userspace
        is still hanging onto. So if we clear this ourselves, we should
        clear it in the pci ->remove callback, not in the drm_device
        ->release callback.
      
      Looking at git history this was fixed in the driver core with
      
      commit 0998d063
      Author: Hans de Goede <hdegoede@redhat.com>
      Date:   Wed May 23 00:09:34 2012 +0200
      
          device-core: Ensure drvdata = NULL when no driver is bound
      
      v2: Cite the core fix in the commit message (Chris).
      
      v3: Fix commit message and unused variable warning (Jani).
      
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Reviewed-by: default avatarJani Nikula <jani.nikula@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-3-daniel.vetter@ffwll.ch
      0ce542f7
    • Daniel Vetter's avatar
      mm/sl[uo]b: export __kmalloc_track(_node)_caller · fd7cb575
      Daniel Vetter authored
      slab does this already, and I want to use this in a memory allocation
      tracker in drm for stuff that's tied to the lifetime of a drm_device,
      not the underlying struct device. Kinda like devres, but for drm.
      Acked-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Cc: Christoph Lameter <cl@linux.com>
      Cc: Pekka Enberg <penberg@kernel.org>
      Cc: David Rientjes <rientjes@google.com>
      Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: linux-mm@kvack.org
      Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-2-daniel.vetter@ffwll.ch
      fd7cb575
  2. 25 Mar, 2020 2 commits
  3. 24 Mar, 2020 7 commits
  4. 23 Mar, 2020 19 commits
  5. 22 Mar, 2020 2 commits