Re: [PATCH] vc4_bo.c: always set bo->resv

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/02/2017 11:48 PM, Eric Anholt wrote:
Hans Verkuil <hverkuil@xxxxxxxxx> writes:

The bo->resv pointer could be NULL, leading to kernel oopses like the one below.
It looks like .fb_probe -> drm_fbdev_cma_create() -> drm_gem_cma_create() would
end up not initializing resv, because we're doing that in vc4_bo_create(), not
vc4_create_object().

This patch ensures that bo->resv is always set in vc4_create_object
ensuring that it is never NULL.

Thanks to Eric Anholt for pointing to the correct solution.

Folks that know dma-buf better: Will having two reservations around for
the lifetime of the BO in the case of CMA dma-buf imports be a bad
thing?  I'm assuming reservations are cheap, given that we're
unconditionally allocating them per BO for un-shared BOs already.

Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
---
   drivers/gpu/drm/vc4/vc4_bo.c | 12 ++++--------
   1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 80b2f9e55c5c..2231ee76cd8d 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -91,8 +91,7 @@ static void vc4_bo_destroy(struct vc4_bo *bo)
   	vc4->bo_stats.num_allocated--;
   	vc4->bo_stats.size_allocated -= obj->size;

-	if (bo->resv == &bo->_resv)
-		reservation_object_fini(bo->resv);
+	reservation_object_fini(bo->resv);

This one would need to be "reservation_object_fini(&bo->_resv);" -- we
need to free the reservation that we created (and left unused in the
case of an import), not the one that was imported from someone else.

Of course, fixed. I'll wait a bit to see if anyone replies to your query
above, and will post a v2 some time next week.

Regards,

	Hans


   	drm_gem_cma_free_object(obj);
   }
@@ -212,6 +211,8 @@ struct drm_gem_object *vc4_create_object(struct drm_device *dev, size_t size)
   	vc4->bo_stats.num_allocated++;
   	vc4->bo_stats.size_allocated += size;
   	mutex_unlock(&vc4->bo_lock);
+	bo->resv = &bo->_resv;
+	reservation_object_init(bo->resv);

   	return &bo->base.base;
   }
@@ -250,12 +251,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size,
   			return ERR_PTR(-ENOMEM);
   		}
   	}
-	bo = to_vc4_bo(&cma_obj->base);
-
-	bo->resv = &bo->_resv;
-	reservation_object_init(bo->resv);
-
-	return bo;
+	return to_vc4_bo(&cma_obj->base);
   }

   int vc4_dumb_create(struct drm_file *file_priv,
--
2.11.0


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux