| Message ID | 4b5a19219e4a1313fd438d52431b57cb7b77b34a.1713357042.git.karolina.stolarek@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve test coverage of TTM | expand |
Am 17.04.24 um 15:03 schrieb Karolina Stolarek: > BOs in a bulk move have to share the same reservation object. That is > not the case in the ttm_bo_unreserve_bulk subtest. Share bo2's resv > object with bo1 to fix the issue. > > Fixes: 995279d280d1 ("drm/ttm/tests: Add tests for ttm_bo functions") > Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> > --- > drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c > index 1f8a4f8adc92..632306adc4a1 100644 > --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c > +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c > @@ -339,6 +339,9 @@ static void ttm_bo_unreserve_bulk(struct kunit *test) > bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE); > bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE); > > + /* Share the reservation object in the same bulk move */ > + bo1->base.resv = bo2->base.resv; > + In a real world driver that would be illegal because it is racy for the UAPI. It might work in the test case, but it could leak any fence storage allocate for the bo1 reservation object. We should probably avoid that and I would rather modify ttm_bo_kunit_init() so that it gets the reservation object which should be used for the newly created BO with the default behavior if the parameter is NULL. Regards, Christian. > dma_resv_lock(bo1->base.resv, NULL); > ttm_bo_set_bulk_move(bo1, &lru_bulk_move); > dma_resv_unlock(bo1->base.resv);
On 18.04.2024 10:15, Christian König wrote: > Am 17.04.24 um 15:03 schrieb Karolina Stolarek: >> BOs in a bulk move have to share the same reservation object. That is >> not the case in the ttm_bo_unreserve_bulk subtest. Share bo2's resv >> object with bo1 to fix the issue. >> >> Fixes: 995279d280d1 ("drm/ttm/tests: Add tests for ttm_bo functions") >> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> >> --- >> drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c >> b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c >> index 1f8a4f8adc92..632306adc4a1 100644 >> --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c >> +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c >> @@ -339,6 +339,9 @@ static void ttm_bo_unreserve_bulk(struct kunit *test) >> bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE); >> bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE); >> + /* Share the reservation object in the same bulk move */ >> + bo1->base.resv = bo2->base.resv; >> + > > In a real world driver that would be illegal because it is racy for the > UAPI. It might work in the test case, but it could leak any fence > storage allocate for the bo1 reservation object. > > We should probably avoid that and I would rather modify > ttm_bo_kunit_init() so that it gets the reservation object which should > be used for the newly created BO with the default behavior if the > parameter is NULL. Thanks for providing the context, makes sense. I'll extend that helper. All the best, Karolina
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c index 1f8a4f8adc92..632306adc4a1 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c @@ -339,6 +339,9 @@ static void ttm_bo_unreserve_bulk(struct kunit *test) bo1 = ttm_bo_kunit_init(test, test->priv, BO_SIZE); bo2 = ttm_bo_kunit_init(test, test->priv, BO_SIZE); + /* Share the reservation object in the same bulk move */ + bo1->base.resv = bo2->base.resv; + dma_resv_lock(bo1->base.resv, NULL); ttm_bo_set_bulk_move(bo1, &lru_bulk_move); dma_resv_unlock(bo1->base.resv);
BOs in a bulk move have to share the same reservation object. That is not the case in the ttm_bo_unreserve_bulk subtest. Share bo2's resv object with bo1 to fix the issue. Fixes: 995279d280d1 ("drm/ttm/tests: Add tests for ttm_bo functions") Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com> --- drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 3 +++ 1 file changed, 3 insertions(+)