Hi Christian we call reservation_object_reserve_shared() at very first time, and later we call reservation_object_add_shared_fence(), which will call reservation_object_add_shared_replace() since fobj is not NULL by this time.
in reservation_object_get_list(), @old is NULL, @fobj is not NULL, so @fence will init the @fobj->shared[] and @fobj->shared_count set to 1, and later the @obj->fence will be set to @fobj (through RCU_INIT_POINTER) and this is the part obj->fence related with obj->staged... At next time calls on reservation_object_reserve_shared(), since current logic will kfree @obj->staged, which let above @obj->fence points to a wild pointer, and lead to weird bugs, in fact we pass stress test on vulkan CTS and vk_example after this patch applied ... /Monk -----Original Message----- From: Christian König [mailto:[email protected]] Sent: 2018年2月26日 17:42 To: Liu, Monk <[email protected]>; [email protected]; Chris Wilson <[email protected]> Subject: Re: [PATCH 21/22] dma-buf/reservation: shouldn't kfree staged when slot available Well first of all you need to send that to dri-devel and even lkml, so that Chris and others can take a look as well. Second that patch doesn't looks correct to me, obj->staged should never be related to obj->fence. Regards, Christian. Am 26.02.2018 um 06:34 schrieb Monk Liu: > issue: > kernel oops or vmc page fault occured during vk_example/vk_cts test. > > root cause: > previously reservation object would kfree the staged when slot checked > available during reserve_shared(), which is incorrect becasue this way > reservation_object->fence will be a wild pointer referenced by > following reservation_object_add_shared_fence, > and lead to lot of abnormal cases depends on luck. > > fix: > don't call kfree on staged in reserve_shared and there won't be > memleak introduced because reservation's finish routine would kfree > both staged and fence > > Change-Id: If7c01f1b4be3d3d8a81efa90216841f79ab1fc1c > Signed-off-by: Monk Liu <[email protected]> > --- > drivers/dma-buf/reservation.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c > b/drivers/dma-buf/reservation.c index 314eb10..bc01e0d 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -74,12 +74,9 @@ int reservation_object_reserve_shared(struct > reservation_object *obj) > old = reservation_object_get_list(obj); > > if (old && old->shared_max) { > - if (old->shared_count < old->shared_max) { > - /* perform an in-place update */ > - kfree(obj->staged); > - obj->staged = NULL; > + if (old->shared_count < old->shared_max) > return 0; > - } else > + else > max = old->shared_max * 2; > } else > max = 4; _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
