All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Track number of pending freed objects
Date: Mon, 19 Feb 2018 18:17:05 +0000	[thread overview]
Message-ID: <151906422571.2041.15820844697013597653@mail.alporthouse.com> (raw)
In-Reply-To: <127b299d-d5ce-f1c9-0291-02914ff7d4e4@linux.intel.com>

Quoting Tvrtko Ursulin (2018-02-19 18:13:13)
> 
> On 19/02/2018 16:19, Chris Wilson wrote:
> > During igt, we frequently call into the driver to reset both HW and
> > driver state (idling the device, waiting for it to become idle and
> > freeing off old objects) to ensure that we start each test/subtest/pass
> > from known state. This process incurs an RCU barrier or two to ensure
> > that any such pending frees are indeed flushed before we return.
> > However, unconditionally waiting on the RCU barrier adds needless delay
> > to many callers, which adds up to several seconds when repeated thousands
> > of times. We can skip the rcu_barrier() if by tracking how many outstanding
> > frees we have, we know there are none.
> 
> To be pedantic it is not skipping the rcu_barrier, but skipping the 
> drain altogether.
> 
> So theoretically there is a tiny difference in behaviour where today 
> drain would wait for all frees currently executing, where after the 
> patch it will ignore these and only process the ones which got to the 
> end of the function.
> 
> Perhaps it atomic_inc was at the very top of i915_gem_free_object it 
> would be closer to today. But such suggestions feel extremely iffy.

That's a smallish window. And it exists even today, with a race after
the RCU grace period (if you let userspace race with itself). I think
it's fair to say that we are dependent upon single-threaded client
operation here (either igt or suspend) for defined behaviour.

> Nosing around the code base suggest the change is completely fine. Only 
> potentially relevant site which might care about the subtle difference 
> is i915_gem_freeze_late, which actually doesnt care since everything has 
> been frozen at that point. So all frees have presumably exited and 
> incremented the new counter.

That's the idea at least :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2018-02-19 18:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-19 16:19 [PATCH] drm/i915: Track number of pending freed objects Chris Wilson
2018-02-19 16:46 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-02-19 18:13 ` [PATCH] " Tvrtko Ursulin
2018-02-19 18:17   ` Chris Wilson [this message]
2018-02-19 21:31 ` ✗ Fi.CI.IGT: warning for " Patchwork
2018-02-19 21:44 ` [PATCH] " Chris Wilson
2018-02-19 22:06 ` [PATCH v2] " Chris Wilson
2018-02-19 22:43 ` ✓ Fi.CI.BAT: success for drm/i915: Track number of pending freed objects (rev2) Patchwork
2018-02-20  1:40 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-02-20  9:12   ` Chris Wilson

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=151906422571.2041.15820844697013597653@mail.alporthouse.com \
    --to=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=tvrtko.ursulin@linux.intel.com \
    /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.