[v2,3/3] dma-buf/selftests: Add locking selftests for sw_sync

Commit Message

Chris Wilson July 14, 2020, 9:24 p.m. UTC

While sw_sync is purely a debug facility for userspace to create fences
and timelines it can control, nevertheless it has some tricky locking
semantics of its own. In particular, Bas Nieuwenhuizen reported that we
had reintroduced a deadlock if a signal callback attempted to destroy
the fence. So let's add a few trivial selftests to make sure that once
fixed again, it stays fixed.

Signed-off-by: Chris Wilson <[e-mail]>
Cc: Bas Nieuwenhuizen <[e-mail]>
Reviewed-by: Bas Nieuwenhuizen <[e-mail]>
---
 drivers/dma-buf/Makefile     |   3 +-
 drivers/dma-buf/selftests.h  |   1 +
 drivers/dma-buf/st-sw_sync.c | 279 +++++++++++++++++++++++++++++++++++
 drivers/dma-buf/sw_sync.c    |  39 +++++
 drivers/dma-buf/sync_debug.h |   8 +
 5 files changed, 329 insertions(+), 1 deletion(-)
 create mode 100644 drivers/dma-buf/st-sw_sync.c

Patch

diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 995e05f609ff..9be4d4611609 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -10,6 +10,7 @@  obj-$(CONFIG_UDMABUF)		+= udmabuf.o
 dmabuf_selftests-y := \
 	selftest.o \
 	st-dma-fence.o \
-	st-dma-fence-chain.o
+	st-dma-fence-chain.o \
+	st-sw_sync.o
 
 obj-$(CONFIG_DMABUF_SELFTESTS)	+= dmabuf_selftests.o
diff --git a/drivers/dma-buf/selftests.h b/drivers/dma-buf/selftests.h
index bc8cea67bf1e..232499a24872 100644
--- a/drivers/dma-buf/selftests.h
+++ b/drivers/dma-buf/selftests.h
@@ -12,3 +12,4 @@ 
 selftest(sanitycheck, __sanitycheck__) /* keep first (igt selfcheck) */
 selftest(dma_fence, dma_fence)
 selftest(dma_fence_chain, dma_fence_chain)
+selftest(sw_sync, sw_sync)
diff --git a/drivers/dma-buf/st-sw_sync.c b/drivers/dma-buf/st-sw_sync.c
new file mode 100644
index 000000000000..145fd330f1c6
--- /dev/null
+++ b/drivers/dma-buf/st-sw_sync.c
@@ -0,0 +1,279 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include <linux/delay.h>
+#include <linux/dma-fence.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/sched/signal.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "sync_debug.h"
+#include "selftest.h"
+
+static int sanitycheck(void *arg)
+{
+	struct sync_timeline *tl;
+	struct dma_fence *f;
+	int err = -ENOMEM;
+
+	/* Quick check we can create the timeline and syncpt */
+
+	tl = st_sync_timeline_create("mock");
+	if (!tl)
+		return -ENOMEM;
+
+	f = st_sync_pt_create(tl, 1);
+	if (!f)
+		goto out;
+
+	dma_fence_signal(f);
+	dma_fence_put(f);
+
+	err = 0;
+out:
+	st_sync_timeline_put(tl);
+	return err;
+}
+
+static int signal(void *arg)
+{
+	struct sync_timeline *tl;
+	struct dma_fence *f;
+	int err = -EINVAL;
+
+	/* Check that the syncpt fence is signaled when the timeline advances */
+
+	tl = st_sync_timeline_create("mock");
+	if (!tl)
+		return -ENOMEM;
+
+	f = st_sync_pt_create(tl, 1);
+	if (!f) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	if (dma_fence_is_signaled(f)) {
+		pr_err("syncpt:%lld signaled too early\n", f->seqno);
+		goto out_fence;
+	}
+
+	st_sync_timeline_signal(tl, 1);
+
+	if (!dma_fence_is_signaled(f)) {
+		pr_err("syncpt:%lld not signaled after increment\n", f->seqno);
+		goto out_fence;
+	}
+
+	err = 0;
+out_fence:
+	dma_fence_signal(f);
+	dma_fence_put(f);
+out:
+	st_sync_timeline_put(tl);
+	return err;
+}
+
+struct cb_destroy {
+	struct dma_fence_cb cb;
+	struct dma_fence *f;
+};
+
+static void cb_destroy(struct dma_fence *fence, struct dma_fence_cb *_cb)
+{
+	struct cb_destroy *cb = container_of(_cb, typeof(*cb), cb);
+
+	pr_info("syncpt:%llx destroying syncpt:%llx\n",
+		fence->seqno, cb->f->seqno);
+	dma_fence_put(cb->f);
+	cb->f = NULL;
+}
+
+static int cb_autodestroy(void *arg)
+{
+	struct sync_timeline *tl;
+	struct cb_destroy cb;
+	int err = -EINVAL;
+
+	/* Check that we can drop the final syncpt reference from a callback */
+
+	tl = st_sync_timeline_create("mock");
+	if (!tl)
+		return -ENOMEM;
+
+	cb.f = st_sync_pt_create(tl, 1);
+	if (!cb.f) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	if (dma_fence_add_callback(cb.f, &cb.cb, cb_destroy)) {
+		pr_err("syncpt:%lld signaled before increment\n", cb.f->seqno);
+		goto out;
+	}
+
+	st_sync_timeline_signal(tl, 1);
+	if (cb.f) {
+		pr_err("syncpt:%lld callback not run\n", cb.f->seqno);
+		dma_fence_put(cb.f);
+		goto out;
+	}
+
+	err = 0;
+out:
+	st_sync_timeline_put(tl);
+	return err;
+}
+
+static int cb_destroy_12(void *arg)
+{
+	struct sync_timeline *tl;
+	struct cb_destroy cb;
+	struct dma_fence *f;
+	int err = -EINVAL;
+
+	/* Check that we can drop some other syncpt reference from a callback */
+
+	tl = st_sync_timeline_create("mock");
+	if (!tl)
+		return -ENOMEM;
+
+	f = st_sync_pt_create(tl, 1);
+	if (!f) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	cb.f = st_sync_pt_create(tl, 2);
+	if (!cb.f) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	if (dma_fence_add_callback(f, &cb.cb, cb_destroy)) {
+		pr_err("syncpt:%lld signaled before increment\n", f->seqno);
+		goto out;
+	}
+
+	st_sync_timeline_signal(tl, 1);
+	if (cb.f) {
+		pr_err("syncpt:%lld callback not run\n", f->seqno);
+		dma_fence_put(cb.f);
+		goto out_fence;
+	}
+
+	err = 0;
+out_fence:
+	dma_fence_put(f);
+out:
+	st_sync_timeline_put(tl);
+	return err;
+}
+
+static int cb_destroy_21(void *arg)
+{
+	struct sync_timeline *tl;
+	struct cb_destroy cb;
+	struct dma_fence *f;
+	int err = -EINVAL;
+
+	/* Check that we can drop an earlier syncpt reference from a callback */
+
+	tl = st_sync_timeline_create("mock");
+	if (!tl)
+		return -ENOMEM;
+
+	cb.f = st_sync_pt_create(tl, 1);
+	if (!cb.f) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	f = st_sync_pt_create(tl, 2);
+	if (!f) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	if (dma_fence_add_callback(f, &cb.cb, cb_destroy)) {
+		pr_err("syncpt:%lld signaled before increment\n", f->seqno);
+		goto out;
+	}
+
+	st_sync_timeline_signal(tl, 2);
+	if (cb.f) {
+		pr_err("syncpt:%lld callback not run\n", f->seqno);
+		dma_fence_put(cb.f);
+		goto out_fence;
+	}
+
+	err = 0;
+out_fence:
+	dma_fence_put(f);
+out:
+	st_sync_timeline_put(tl);
+	return err;
+}
+
+static int cb_destroy_22(void *arg)
+{
+	struct sync_timeline *tl;
+	struct cb_destroy cb;
+	struct dma_fence *f;
+	int err = -EINVAL;
+
+	/* Check that we can drop the later syncpt reference from a callback */
+
+	tl = st_sync_timeline_create("mock");
+	if (!tl)
+		return -ENOMEM;
+
+	f = st_sync_pt_create(tl, 1);
+	if (!f) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	cb.f = st_sync_pt_create(tl, 2);
+	if (!cb.f) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	if (dma_fence_add_callback(f, &cb.cb, cb_destroy)) {
+		pr_err("syncpt:%lld signaled before increment\n", f->seqno);
+		goto out;
+	}
+
+	st_sync_timeline_signal(tl, 2);
+	if (cb.f) {
+		pr_err("syncpt:%lld callback not run\n", f->seqno);
+		dma_fence_put(cb.f);
+		goto out_fence;
+	}
+
+	err = 0;
+out_fence:
+	dma_fence_put(f);
+out:
+	st_sync_timeline_put(tl);
+	return err;
+}
+
+int sw_sync(void)
+{
+	static const struct subtest tests[] = {
+		SUBTEST(sanitycheck),
+		SUBTEST(signal),
+		SUBTEST(cb_autodestroy),
+		SUBTEST(cb_destroy_12),
+		SUBTEST(cb_destroy_21),
+		SUBTEST(cb_destroy_22),
+	};
+
+	return subtests(tests, NULL);
+}
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 17a5c1a3b7ce..f16b6c476eef 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -428,3 +428,42 @@  const struct file_operations sw_sync_debugfs_fops = {
 	.unlocked_ioctl = sw_sync_ioctl,
 	.compat_ioctl	= compat_ptr_ioctl,
 };
+
+#if IS_ENABLED(CONFIG_DMABUF_SELFTESTS)
+struct sync_timeline *st_sync_timeline_create(const char *name)
+{
+	return sync_timeline_create(name);
+}
+EXPORT_SYMBOL_GPL(st_sync_timeline_create);
+
+void st_sync_timeline_get(struct sync_timeline *tl)
+{
+	sync_timeline_get(tl);
+}
+EXPORT_SYMBOL_GPL(st_sync_timeline_get);
+
+void st_sync_timeline_put(struct sync_timeline *tl)
+{
+	sync_timeline_put(tl);
+}
+EXPORT_SYMBOL_GPL(st_sync_timeline_put);
+
+void st_sync_timeline_signal(struct sync_timeline *tl, unsigned int inc)
+{
+	sync_timeline_signal(tl, inc);
+}
+EXPORT_SYMBOL_GPL(st_sync_timeline_signal);
+
+struct dma_fence *
+st_sync_pt_create(struct sync_timeline *tl, unsigned int seqno)
+{
+	struct sync_pt *pt;
+
+	pt = sync_pt_create(tl, seqno);
+	if (!pt)
+		return NULL;
+
+	return &pt->base;
+}
+EXPORT_SYMBOL_GPL(st_sync_pt_create);
+#endif
diff --git a/drivers/dma-buf/sync_debug.h b/drivers/dma-buf/sync_debug.h
index 56589dae2159..546a11e564a7 100644
--- a/drivers/dma-buf/sync_debug.h
+++ b/drivers/dma-buf/sync_debug.h
@@ -72,4 +72,12 @@  void sync_timeline_debug_remove(struct sync_timeline *obj);
 void sync_file_debug_add(struct sync_file *fence);
 void sync_file_debug_remove(struct sync_file *fence);
 
+struct sync_timeline *st_sync_timeline_create(const char *name);
+void st_sync_timeline_get(struct sync_timeline *tl);
+void st_sync_timeline_put(struct sync_timeline *tl);
+void st_sync_timeline_signal(struct sync_timeline *tl, unsigned int inc);
+
+struct dma_fence *
+st_sync_pt_create(struct sync_timeline *tl, unsigned int seqno);
+
 #endif /* _LINUX_SYNC_H */