[Intel-gfx] [PATCH 3/3] dma-buf/selftests: Add locking selftests for sw_sync
Chris Wilson
chris at chris-wilson.co.uk
Tue Jul 14 20:06:46 UTC 2020
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 Nieuwenhuize 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 <chris at chris-wilson.co.uk>
Cc: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
---
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
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..3a21e7717df7
--- /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 0;
+}
+
+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 0;
+}
+
+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 0;
+}
+
+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 (!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 0;
+}
+
+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 (!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 0;
+}
+
+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 (!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 0;
+}
+
+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 116dad6c7905..7c229f4a6b56 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -426,3 +426,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 fd073fc32329..7c96d9a2c02d 100644
--- a/drivers/dma-buf/sync_debug.h
+++ b/drivers/dma-buf/sync_debug.h
@@ -71,4 +71,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 */
--
2.20.1
More information about the Intel-gfx
mailing list