| Message ID | 20211129134735.628712-10-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Remove short term pins from execbuf. | expand |
On Mon, 29 Nov 2021 at 13:58, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > > Now that we require locking to evict, multiple vmas from the same object > might not be evicted. This is expected and required, because execbuf will > move to short-term pinning by using the lock only. This will cause these > tests to fail, because they create a ton of vma's for the same object. > > Unbind manually to prevent spurious -ENOSPC in those mock tests. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Hmm, do we need this? It looks like we should be able to handle such scenarios, with already locked objects sharing the same dma-resv? Or is something else going on here? > --- > drivers/gpu/drm/i915/selftests/i915_vma.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c > index 1f10fe36619b..5c5809dfe9b2 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_vma.c > +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c > @@ -691,7 +691,11 @@ static int igt_vma_rotate_remap(void *arg) > } > > i915_vma_unpin(vma); > - > + err = i915_vma_unbind(vma); > + if (err) { > + pr_err("Unbinding returned %i\n", err); > + goto out_object; > + } > cond_resched(); > } > } > @@ -848,6 +852,11 @@ static int igt_vma_partial(void *arg) > > i915_vma_unpin(vma); > nvma++; > + err = i915_vma_unbind(vma); > + if (err) { > + pr_err("Unbinding returned %i\n", err); > + goto out_object; > + } > > cond_resched(); > } > @@ -882,6 +891,12 @@ static int igt_vma_partial(void *arg) > > i915_vma_unpin(vma); > > + err = i915_vma_unbind(vma); > + if (err) { > + pr_err("Unbinding returned %i\n", err); > + goto out_object; > + } > + > count = 0; > list_for_each_entry(vma, &obj->vma.list, obj_link) > count++; > -- > 2.34.0 >
On Wed, 8 Dec 2021 at 11:49, Matthew Auld <matthew.william.auld@gmail.com> wrote: > > On Mon, 29 Nov 2021 at 13:58, Maarten Lankhorst > <maarten.lankhorst@linux.intel.com> wrote: > > > > Now that we require locking to evict, multiple vmas from the same object > > might not be evicted. This is expected and required, because execbuf will > > move to short-term pinning by using the lock only. This will cause these > > tests to fail, because they create a ton of vma's for the same object. > > > > Unbind manually to prevent spurious -ENOSPC in those mock tests. > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Hmm, do we need this? It looks like we should be able to handle such > scenarios, with already locked objects sharing the same dma-resv? Or > is something else going on here? Oh, because trylock still "fails' in such cases? Do the later changes to evict_vm, where it is able to handle already locked objects fix this? Do we not need similar treatment for things like evict_for_node? > > > --- > > drivers/gpu/drm/i915/selftests/i915_vma.c | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c > > index 1f10fe36619b..5c5809dfe9b2 100644 > > --- a/drivers/gpu/drm/i915/selftests/i915_vma.c > > +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c > > @@ -691,7 +691,11 @@ static int igt_vma_rotate_remap(void *arg) > > } > > > > i915_vma_unpin(vma); > > - > > + err = i915_vma_unbind(vma); > > + if (err) { > > + pr_err("Unbinding returned %i\n", err); > > + goto out_object; > > + } > > cond_resched(); > > } > > } > > @@ -848,6 +852,11 @@ static int igt_vma_partial(void *arg) > > > > i915_vma_unpin(vma); > > nvma++; > > + err = i915_vma_unbind(vma); > > + if (err) { > > + pr_err("Unbinding returned %i\n", err); > > + goto out_object; > > + } > > > > cond_resched(); > > } > > @@ -882,6 +891,12 @@ static int igt_vma_partial(void *arg) > > > > i915_vma_unpin(vma); > > > > + err = i915_vma_unbind(vma); > > + if (err) { > > + pr_err("Unbinding returned %i\n", err); > > + goto out_object; > > + } > > + > > count = 0; > > list_for_each_entry(vma, &obj->vma.list, obj_link) > > count++; > > -- > > 2.34.0 > >
diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c index 1f10fe36619b..5c5809dfe9b2 100644 --- a/drivers/gpu/drm/i915/selftests/i915_vma.c +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c @@ -691,7 +691,11 @@ static int igt_vma_rotate_remap(void *arg) } i915_vma_unpin(vma); - + err = i915_vma_unbind(vma); + if (err) { + pr_err("Unbinding returned %i\n", err); + goto out_object; + } cond_resched(); } } @@ -848,6 +852,11 @@ static int igt_vma_partial(void *arg) i915_vma_unpin(vma); nvma++; + err = i915_vma_unbind(vma); + if (err) { + pr_err("Unbinding returned %i\n", err); + goto out_object; + } cond_resched(); } @@ -882,6 +891,12 @@ static int igt_vma_partial(void *arg) i915_vma_unpin(vma); + err = i915_vma_unbind(vma); + if (err) { + pr_err("Unbinding returned %i\n", err); + goto out_object; + } + count = 0; list_for_each_entry(vma, &obj->vma.list, obj_link) count++;
Now that we require locking to evict, multiple vmas from the same object might not be evicted. This is expected and required, because execbuf will move to short-term pinning by using the lock only. This will cause these tests to fail, because they create a ton of vma's for the same object. Unbind manually to prevent spurious -ENOSPC in those mock tests. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/selftests/i915_vma.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)