dmatest: don't use set_freezable_with_signal()
Commit 981ed70d8e (dmatest: make dmatest threads freezable) made
dmatest kthread use set_freezable_with_signal(); however, the
interface is scheduled to be removed in the next merge window.
The problem is that unlike userland tasks there's no default place
which handles signal pending state and it isn't clear who owns and/or
is responsible for clearing TIF_SIGPENDING. For example, in the
current code, try_to_freeze() clears TIF_SIGPENDING but it isn't sure
whether it actually owns the TIF_SIGPENDING nor is it race-free -
ie. the task may continue to run with TIF_SIGPENDING set after the
freezable section.
Unfortunately, we don't have wait_for_completion_freezable_timeout().
This patch open codes it and uses wait_event_freezable_timeout()
instead and removes timeout reloading - wait_event_freezable_timeout()
won't return across freezing events (currently racy but fix scheduled)
and timer doesn't decrement while the task is in freezer. Although
this does lose timer-reset-over-freezing, given that timeout is
supposed to be long enough and failure to finish inside is considered
irrecoverable, I don't think this is worth the complexity.
While at it, move completion to outer scope and explain that we're
ignoring dangling pointer problem after timeout. This should give
slightly better chance at avoiding oops after timeout.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index eb1d864..2b8661b 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -214,9 +214,18 @@
return error_count;
}
-static void dmatest_callback(void *completion)
+/* poor man's completion - we want to use wait_event_freezable() on it */
+struct dmatest_done {
+ bool done;
+ wait_queue_head_t *wait;
+};
+
+static void dmatest_callback(void *arg)
{
- complete(completion);
+ struct dmatest_done *done = arg;
+
+ done->done = true;
+ wake_up_all(done->wait);
}
/*
@@ -235,7 +244,9 @@
*/
static int dmatest_func(void *data)
{
+ DECLARE_WAIT_QUEUE_HEAD_ONSTACK(done_wait);
struct dmatest_thread *thread = data;
+ struct dmatest_done done = { .wait = &done_wait };
struct dma_chan *chan;
const char *thread_name;
unsigned int src_off, dst_off, len;
@@ -252,7 +263,7 @@
int i;
thread_name = current->comm;
- set_freezable_with_signal();
+ set_freezable();
ret = -ENOMEM;
@@ -306,9 +317,6 @@
struct dma_async_tx_descriptor *tx = NULL;
dma_addr_t dma_srcs[src_cnt];
dma_addr_t dma_dsts[dst_cnt];
- struct completion cmp;
- unsigned long start, tmo, end = 0 /* compiler... */;
- bool reload = true;
u8 align = 0;
total_tests++;
@@ -391,9 +399,9 @@
continue;
}
- init_completion(&cmp);
+ done.done = false;
tx->callback = dmatest_callback;
- tx->callback_param = &cmp;
+ tx->callback_param = &done;
cookie = tx->tx_submit(tx);
if (dma_submit_error(cookie)) {
@@ -407,20 +415,20 @@
}
dma_async_issue_pending(chan);
- do {
- start = jiffies;
- if (reload)
- end = start + msecs_to_jiffies(timeout);
- else if (end <= start)
- end = start + 1;
- tmo = wait_for_completion_interruptible_timeout(&cmp,
- end - start);
- reload = try_to_freeze();
- } while (tmo == -ERESTARTSYS);
+ wait_event_freezable_timeout(done_wait, done.done,
+ msecs_to_jiffies(timeout));
status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
- if (tmo == 0) {
+ if (!done.done) {
+ /*
+ * We're leaving the timed out dma operation with
+ * dangling pointer to done_wait. To make this
+ * correct, we'll need to allocate wait_done for
+ * each test iteration and perform "who's gonna
+ * free it this time?" dancing. For now, just
+ * leave it dangling.
+ */
pr_warning("%s: #%u: test timed out\n",
thread_name, total_tests - 1);
failed_tests++;