From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
Daniel Vetter <daniel@ffwll.ch>,
intel-gfx@lists.freedesktop.org,
Ben Widawsky <benjamin.widawsky@intel.com>
Subject: Re: [PATCH] drm/i915: Prevent signals from interrupting close()
Date: Wed, 9 Apr 2014 18:50:03 +0200 [thread overview]
Message-ID: <20140409165003.GA9262@phenom.ffwll.local> (raw)
In-Reply-To: <20140409150323.GA14192@nuc-i3427.alporthouse.com>
On Wed, Apr 09, 2014 at 04:03:23PM +0100, Chris Wilson wrote:
> On Wed, Apr 09, 2014 at 04:49:02PM +0200, Daniel Vetter wrote:
> > On Wed, Apr 09, 2014 at 08:03:39AM +0100, Chris Wilson wrote:
> > > We neither report any unfinished operations during releasing GEM objects
> > > associated with the file, and even if we did, it is bad form to report
> > > -EINTR from a close().
> > >
> > > The root cause of the bug that first showed itself during close is that
> > > we do not do proper live tracking of vma and contexts under full-ppgtt,
> > > but this is useful piece of defensive programming enforcing our
> > > userspace API contract.
> > >
> > > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > I'd really prefer something annoying and loud like we've done when nuking
> > the deferred free list in
> >
> > commit 1488fc08c1706288616c602416654fd38c773deb
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date: Tue Apr 24 15:47:31 2012 +0100
> >
> > drm/i915: Remove the deferred-free list
> >
> > where we've added a WARN_ON in gem_free_object if any unbind was failing
> > due to interrupts. This patch here disables that imo useful safety check.
>
> It doesn't classify as a safety check if kernel/userspace explodes. The
> trick would be to somehow get the error code back. And in this case we
> cannot accept any error anyway. So yes, I would like a nice big warning
> to catch bugs, but that is icing on the cake compared to preventing bugs
> from destroying data.
Hm ... double-checking: This is just for full ppgtt, right? I think in
that case I prefer if people working on it just carry this around locally.
Otherwise I need to freak out ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
next prev parent reply other threads:[~2014-04-09 16:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-09 7:03 [PATCH] drm/i915: Prevent signals from interrupting close() Chris Wilson
2014-04-09 14:49 ` Daniel Vetter
2014-04-09 15:03 ` Chris Wilson
2014-04-09 16:50 ` Daniel Vetter [this message]
2014-04-09 16:57 ` Chris Wilson
2014-04-09 17:43 ` Ben Widawsky
2014-04-09 17:58 ` Chris Wilson
2014-04-11 6:44 ` Ben Widawsky
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=20140409165003.GA9262@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=benjamin.widawsky@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.