dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Dave Airlie <airlied@redhat.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Gustavo Padovan <gustavo.padovan@collabora.com>,
	CQ Tang <cq.tang@intel.com>, stable <stable@vger.kernel.org>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH 1/3] drm: Restore driver.preclose() for all to use
Date: Mon, 27 Jul 2020 21:32:45 +0200	[thread overview]
Message-ID: <CAKMK7uFt5ViekqBPqdBbJWN4FhfxvF57K58VW8hAZGZwjRDz0w@mail.gmail.com> (raw)
In-Reply-To: <20200723172119.17649-1-chris@chris-wilson.co.uk>

On Thu, Jul 23, 2020 at 7:21 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> An unfortunate sequence of events, but it turns out there is a valid
> usecase for being able to free/decouple the driver objects before they
> are freed by the DRM core. In particular, if we have a pointer into a
> drm core object from inside a driver object, that pointer needs to be
> nerfed *before* it is freed so that concurrent access (e.g. debugfs)
> does not following the dangling pointer.
>
> The legacy marker was adding in the code movement from drp_fops.c to
> drm_file.c

I might fumble a lot, but not this one:

commit 45c3d213a400c952ab7119f394c5293bb6877e6b
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon May 8 10:26:33 2017 +0200

    drm: Nerf the preclose callback for modern drivers

Also looking at the debugfs hook that has some rather adventurous
stuff going on I think, feels a bit like a kitchensink with batteries
included. If that's really all needed I'd say iterate the contexts by
first going over files, then the ctx (which arent shared anyway) and
the problem should also be gone.
-Daniel

> References: 9acdac68bcdc ("drm: rename drm_fops.c to drm_file.c")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Gustavo Padovan <gustavo.padovan@collabora.com>
> Cc: CQ Tang <cq.tang@intel.com>
> Cc: <stable@vger.kernel.org> # v4.12+
> ---
>  drivers/gpu/drm/drm_file.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 0ac4566ae3f4..7b4258d6f7cc 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -258,8 +258,7 @@ void drm_file_free(struct drm_file *file)
>                   (long)old_encode_dev(file->minor->kdev->devt),
>                   atomic_read(&dev->open_count));
>
> -       if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
> -           dev->driver->preclose)
> +       if (dev->driver->preclose)
>                 dev->driver->preclose(dev, file);
>
>         if (drm_core_check_feature(dev, DRIVER_LEGACY))
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2020-07-27 19:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23 17:21 [PATCH 1/3] drm: Restore driver.preclose() for all to use Chris Wilson
2020-07-23 17:21 ` [PATCH 2/3] drm/i915/gem: Move context decoupling from postclose to preclose Chris Wilson
2020-07-23 17:44   ` Tang, CQ
2020-07-23 17:48     ` Chris Wilson
2020-07-23 17:21 ` [PATCH 3/3] drm/i915/gem: Serialise debugfs i915_gem_objects with ctx->mutex Chris Wilson
2020-07-27 21:24   ` Sasha Levin
2020-09-14 16:45   ` [Intel-gfx] " Tvrtko Ursulin
2020-09-16  7:42     ` Daniel Vetter
2020-09-16  8:27       ` Tvrtko Ursulin
2020-07-27 19:32 ` Daniel Vetter [this message]
2020-07-27 20:11   ` [PATCH 1/3] drm: Restore driver.preclose() for all to use Tang, CQ
2020-07-28 16:27   ` Chris Wilson
2020-07-29 15:09     ` Tang, CQ
2020-07-27 21:24 ` Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKMK7uFt5ViekqBPqdBbJWN4FhfxvF57K58VW8hAZGZwjRDz0w@mail.gmail.com \
    --to=daniel@ffwll.ch \
    --cc=airlied@redhat.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=cq.tang@intel.com \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gustavo.padovan@collabora.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).