dri-devel Archive mirror
 help / color / mirror / Atom feed
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: "Christian König" <christian.koenig@amd.com>,
	dri-devel@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: drm_exec changes for eviction
Date: Tue, 07 May 2024 16:41:31 +0200	[thread overview]
Message-ID: <953c157bf69df12d831a781f0f638d93717bb044.camel@linux.intel.com> (raw)
In-Reply-To: <e54f1ffc-854f-4dc0-b1b5-257cbe5d9772@amd.com>

On Tue, 2024-05-07 at 15:36 +0200, Christian König wrote:
> Am 06.05.24 um 11:46 schrieb Thomas Hellström:
> > Hi Christian, Others.
> > 
> > In order to support exhaustive eviction there are some changes that
> > I
> > think needs to be made to drm_exec:
> > 
> > 1) Trylock support
> > (This is for ttm_bo_vm, ttm_buffer_object_init_reserved, and also
> > for
> > the eviction path where I think we want to make a trylock pass
> > before a
> > sleeping lock pass).
> 
> Not sure why we need this for ttm_bo_vm, but in general that sounds
> like 
> the right approach to me as well.

In current ttm_bo_vm, if the trylock fails, the mmap lock is typically
dropped while we wait for the lock to become available, and then the
fault is retried, using the fault retry mechanism. (This is only to
avoid blocking the mmap lock unnecessarily- the comment around that
code is obsolete).

> 
> > In essence this means we can't leave any contending lock unlocked
> > until
> > the next sleeping lock, but rather need to relock it on
> > drm_exec_retry_on_contention(), meaning that that macro also gets
> > passed and returns an error code to handle -EINTR.
> 
> Hui what? I can't really follow what you mean here.

Currently drm_exec keeps a reference to the contended lock, which is
not locked. And it is locked on the *next* drm_exec_lock_obj() after a
restart. So the question arises what to do if the *next* lock is a
trylock. We shouldn't really do a sleeping lock of the contended object
at that point. So the suggestion here was to do it during
drm_exec_retry_on_contention() instead. But then we face the problem
that the locking of the contended object might fail with -EINTR.

> 
> > 
> > 2) Snapshot unlock
> > After successfully obtaining backing store, we want to unlock all
> > evicted objects. So snapshot the drm_exec state when eviction
> > begins,
> > and unlock everything down to the snapshot after successful backing
> > store allocation. Essentially the snapshot contains the number of
> > locked objects and a pointer to the prelocked object.
> 
> Interesting idea, never though about that. Not sure if that makes the
> situation better or worse.
> 
> One goal of drm_exec was to be able to keep evicted BOs locked until
> the 
> whole submission is completed and that obviously contradicts that.

We've tried both approaches on i915 downstream (non-ttm). IIRC the
snapshot idea was originally brought up by Sima. Couldn't see any real
difference, but I think snapshotting should be sufficient to avoid
starvation.

/Thomas


> 
> Regards,
> Christian.
> 
> > 
> > Any concerns?
> > Thanks,
> > Thomas
> > 
> 


      reply	other threads:[~2024-05-07 14:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-06  9:46 drm_exec changes for eviction Thomas Hellström
2024-05-07 13:36 ` Christian König
2024-05-07 14:41   ` Thomas Hellström [this message]

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=953c157bf69df12d831a781f0f638d93717bb044.camel@linux.intel.com \
    --to=thomas.hellstrom@linux.intel.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-xe@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 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 read-only IMAP folder(s) and NNTP newsgroup(s).