Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 27da44dd authored by Lynus Vaz's avatar Lynus Vaz
Browse files

msm: kgsl: Fix a race condition when destroying fence events 



Make sure the fence event structure is valid when cancelling it,
and that only one of the callback and cancel functions can run. The
function that runs cleans up the associated structures.

CRs-fixed: 2077454
Change-Id: I7c250868ae4fd193aef0424c7d66283a3e7f8121
Signed-off-by: default avatarLynus Vaz <lvaz@codeaurora.org>
parent 106f4889

drivers/gpu/msm/kgsl.c

+2 −1
Original line number Diff line number Diff line
@@ -1925,12 +1925,13 @@ static long gpuobj_free_on_timestamp(struct kgsl_device_private *dev_priv, 
	return ret;

}



static void gpuobj_free_fence_func(void *priv)

static bool gpuobj_free_fence_func(void *priv)

{

	struct kgsl_mem_entry *entry = priv;



	INIT_WORK(&entry->work, _deferred_put);

	queue_work(kgsl_driver.mem_workqueue, &entry->work);

	return true;

}



static long gpuobj_free_on_fence(struct kgsl_device_private *dev_priv,

drivers/gpu/msm/kgsl_drawobj.c

+30 −15
Original line number Diff line number Diff line
@@ -158,10 +158,13 @@ static void syncobj_timer(unsigned long data) 
}



/*

 * a generic function to retire a pending sync event and (possibly)

 * kick the dispatcher

 * a generic function to retire a pending sync event and (possibly) kick the

 * dispatcher.

 * Returns false if the event was already marked for cancellation in another

 * thread. This function should return true if this thread is responsible for

 * freeing up the memory, and the event will not be cancelled.

 */

static void drawobj_sync_expire(struct kgsl_device *device,

static bool drawobj_sync_expire(struct kgsl_device *device,

	struct kgsl_drawobj_sync_event *event)

{

	struct kgsl_drawobj_sync *syncobj = event->syncobj;
@@ -170,7 +173,7 @@ static void drawobj_sync_expire(struct kgsl_device *device, 
	 * leave without doing anything useful

	 */

	if (!test_and_clear_bit(event->id, &syncobj->pending))

		return;

		return false;



	/*

	 * If no more pending events, delete the timer and schedule the command
@@ -183,6 +186,7 @@ static void drawobj_sync_expire(struct kgsl_device *device, 
			device->ftbl->drawctxt_sched(device,

				event->syncobj->base.context);

	}

	return true;

}



/*
@@ -228,18 +232,23 @@ static void drawobj_destroy_sparse(struct kgsl_drawobj *drawobj) 
static void drawobj_destroy_sync(struct kgsl_drawobj *drawobj)

{

	struct kgsl_drawobj_sync *syncobj = SYNCOBJ(drawobj);

	unsigned long pending;

	unsigned long pending = 0;

	unsigned int i;



	/* Zap the canary timer */

	del_timer_sync(&syncobj->timer);



	/*

	 * Copy off the pending list and clear all pending events - this will

	 * render any subsequent asynchronous callback harmless

	 * Copy off the pending list and clear each pending event atomically -

	 * this will render any subsequent asynchronous callback harmless.

	 * This marks each event for deletion. If any pending fence callbacks

	 * run between now and the actual cancel, the associated structures

	 * are kfreed only in the cancel call.

	 */

	bitmap_copy(&pending, &syncobj->pending, KGSL_MAX_SYNCPOINTS);

	bitmap_zero(&syncobj->pending, KGSL_MAX_SYNCPOINTS);

	for_each_set_bit(i, &syncobj->pending, KGSL_MAX_SYNCPOINTS) {

		if (test_and_clear_bit(i, &syncobj->pending))

			__set_bit(i, &pending);

	}



	/*

	 * Clear all pending events - this will render any subsequent async
@@ -259,7 +268,7 @@ static void drawobj_destroy_sync(struct kgsl_drawobj *drawobj) 
				drawobj_sync_func, event);

			break;

		case KGSL_CMD_SYNCPOINT_TYPE_FENCE:

			if (kgsl_sync_fence_async_cancel(event->handle))

			kgsl_sync_fence_async_cancel(event->handle);

			kgsl_drawobj_put(drawobj);

			break;

		}
@@ -320,15 +329,21 @@ void kgsl_drawobj_destroy(struct kgsl_drawobj *drawobj) 
}

EXPORT_SYMBOL(kgsl_drawobj_destroy);



static void drawobj_sync_fence_func(void *priv)

static bool drawobj_sync_fence_func(void *priv)

{

	struct kgsl_drawobj_sync_event *event = priv;



	trace_syncpoint_fence_expire(event->syncobj, event->fence_name);



	drawobj_sync_expire(event->device, event);



	/*

	 * Only call kgsl_drawobj_put() if it's not marked for cancellation

	 * in another thread.

	 */

	if (drawobj_sync_expire(event->device, event)) {

		kgsl_drawobj_put(&event->syncobj->base);

		return true;

	}

	return false;

}



/* drawobj_add_sync_fence() - Add a new sync fence syncpoint

drivers/gpu/msm/kgsl_sync.c

+24 −12
Original line number Diff line number Diff line
@@ -427,10 +427,15 @@ static void kgsl_sync_fence_callback(struct fence *fence, struct fence_cb *cb) 
{

	struct kgsl_sync_fence_cb *kcb = (struct kgsl_sync_fence_cb *)cb;



	kcb->func(kcb->priv);

	/*

	 * If the callback is marked for cancellation in a separate thread,

	 * let the other thread do the cleanup.

	 */

	if (kcb->func(kcb->priv)) {

		fence_put(kcb->fence);

		kfree(kcb);

	}

}



static void kgsl_get_fence_name(struct fence *fence,

	char *fence_name, int name_len)
@@ -452,7 +457,7 @@ static void kgsl_get_fence_name(struct fence *fence, 
}



struct kgsl_sync_fence_cb *kgsl_sync_fence_async_wait(int fd,

	void (*func)(void *priv), void *priv, char *fence_name, int name_len)

	bool (*func)(void *priv), void *priv, char *fence_name, int name_len)

{

	struct kgsl_sync_fence_cb *kcb;

	struct fence *fence;
@@ -492,17 +497,24 @@ struct kgsl_sync_fence_cb *kgsl_sync_fence_async_wait(int fd, 
	return kcb;

}



int kgsl_sync_fence_async_cancel(struct kgsl_sync_fence_cb *kcb)

/*

 * Cancel the fence async callback and do the cleanup. The caller must make

 * sure that the callback (if run before cancelling) returns false, so that

 * no other thread frees the pointer.

 */

void kgsl_sync_fence_async_cancel(struct kgsl_sync_fence_cb *kcb)

{

	if (kcb == NULL)

		return 0;

		return;



	if (fence_remove_callback(kcb->fence, &kcb->fence_cb)) {

	/*

	 * After fence_remove_callback() returns, the fence callback is

	 * either not called at all, or completed without freeing kcb.

	 * This thread can then put the fence refcount and free kcb.

	 */

	fence_remove_callback(kcb->fence, &kcb->fence_cb);

	fence_put(kcb->fence);

	kfree(kcb);

		return 1;

	}

	return 0;

}



struct kgsl_syncsource {

drivers/gpu/msm/kgsl_sync.h

+6 −6
Original line number Diff line number Diff line
@@ -68,13 +68,14 @@ struct kgsl_sync_fence { 
 * fence_cb: Fence callback struct

 * fence: Pointer to the fence for which the callback is done

 * priv: Private data for the callback

 * func: Pointer to the kgsl function to call

 * func: Pointer to the kgsl function to call. This function should return

 * false if the sync callback is marked for cancellation in a separate thread.

 */

struct kgsl_sync_fence_cb {

	struct fence_cb fence_cb;

	struct fence *fence;

	void *priv;

	void (*func)(void *priv);

	bool (*func)(void *priv);

};



struct kgsl_syncsource;
@@ -91,10 +92,10 @@ void kgsl_sync_timeline_destroy(struct kgsl_context *context); 
void kgsl_sync_timeline_put(struct kgsl_sync_timeline *ktimeline);



struct kgsl_sync_fence_cb *kgsl_sync_fence_async_wait(int fd,

					void (*func)(void *priv), void *priv,

					bool (*func)(void *priv), void *priv,

					char *fence_name, int name_len);



int kgsl_sync_fence_async_cancel(struct kgsl_sync_fence_cb *kcb);

void kgsl_sync_fence_async_cancel(struct kgsl_sync_fence_cb *kcb);



long kgsl_ioctl_syncsource_create(struct kgsl_device_private *dev_priv,

					unsigned int cmd, void *data);
@@ -143,10 +144,9 @@ struct kgsl_sync_fence_cb *kgsl_sync_fence_async_wait(int fd, 
	return NULL;

}



static inline int

static inline void

kgsl_sync_fence_async_cancel(struct kgsl_sync_fence_cb *kcb)

{

	return 1;

}



static inline long