Re: [PATCH v11 01/10] drm/ttm/tests: Fix a warning in ttm_bo_unreserve_bulk

["Christian König" <christian.koenig@amd.com>]

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);