From: Zhu Yanjun <yanjun.zhu@intel.com>
To: zyjzyj2000@gmail.com, jgg@ziepe.ca, leon@kernel.org,
linux-rdma@vger.kernel.org, rpearsonhpe@gmail.com,
matsuda-daisuke@fujitsu.com, bvanassche@acm.org,
shinichiro.kawasaki@wdc.com, linux-scsi@vger.kernel.org
Cc: Zhu Yanjun <yanjun.zhu@linux.dev>
Subject: [PATCH 1/1] Revert "RDMA/rxe: Add workqueue support for rxe tasks"
Date: Fri, 22 Sep 2023 12:32:31 -0400 [thread overview]
Message-ID: <20230922163231.2237811-1-yanjun.zhu@intel.com> (raw)
From: Zhu Yanjun <yanjun.zhu@linux.dev>
This reverts commit 9b4b7c1f9f54120940e243251e2b1407767b3381.
This commit replaces tasklet with workqueue. But this results
in occasionally pocess hang at the blktests test case srp/002.
After the discussion in the link[1], this commit is reverted.
Link: https://lore.kernel.org/linux-rdma/e8b76fae-780a-470e-8ec4-c6b650793d10@leemhuis.info/T/#t [1]
Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
CC: rpearsonhpe@gmail.com
CC: matsuda-daisuke@fujitsu.com
CC: bvanassche@acm.org
CC: shinichiro.kawasaki@wdc.com
---
drivers/infiniband/sw/rxe/rxe.c | 9 +--
drivers/infiniband/sw/rxe/rxe_task.c | 110 ++++++++++++---------------
drivers/infiniband/sw/rxe/rxe_task.h | 6 +-
3 files changed, 49 insertions(+), 76 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index 54c723a6edda..7a7e713de52d 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -212,15 +212,9 @@ static int __init rxe_module_init(void)
{
int err;
- err = rxe_alloc_wq();
- if (err)
- return err;
-
err = rxe_net_init();
- if (err) {
- rxe_destroy_wq();
+ if (err)
return err;
- }
rdma_link_register(&rxe_link_ops);
pr_info("loaded\n");
@@ -232,7 +226,6 @@ static void __exit rxe_module_exit(void)
rdma_link_unregister(&rxe_link_ops);
ib_unregister_driver(RDMA_DRIVER_RXE);
rxe_net_exit();
- rxe_destroy_wq();
pr_info("unloaded\n");
}
diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c
index 1501120d4f52..fb9a6bc8e620 100644
--- a/drivers/infiniband/sw/rxe/rxe_task.c
+++ b/drivers/infiniband/sw/rxe/rxe_task.c
@@ -6,24 +6,8 @@
#include "rxe.h"
-static struct workqueue_struct *rxe_wq;
-
-int rxe_alloc_wq(void)
-{
- rxe_wq = alloc_workqueue("rxe_wq", WQ_UNBOUND, WQ_MAX_ACTIVE);
- if (!rxe_wq)
- return -ENOMEM;
-
- return 0;
-}
-
-void rxe_destroy_wq(void)
-{
- destroy_workqueue(rxe_wq);
-}
-
/* Check if task is idle i.e. not running, not scheduled in
- * work queue and not draining. If so move to busy to
+ * tasklet queue and not draining. If so move to busy to
* reserve a slot in do_task() by setting to busy and taking
* a qp reference to cover the gap from now until the task finishes.
* state will move out of busy if task returns a non zero value
@@ -37,6 +21,9 @@ static bool __reserve_if_idle(struct rxe_task *task)
{
WARN_ON(rxe_read(task->qp) <= 0);
+ if (task->tasklet.state & BIT(TASKLET_STATE_SCHED))
+ return false;
+
if (task->state == TASK_STATE_IDLE) {
rxe_get(task->qp);
task->state = TASK_STATE_BUSY;
@@ -51,7 +38,7 @@ static bool __reserve_if_idle(struct rxe_task *task)
}
/* check if task is idle or drained and not currently
- * scheduled in the work queue. This routine is
+ * scheduled in the tasklet queue. This routine is
* called by rxe_cleanup_task or rxe_disable_task to
* see if the queue is empty.
* Context: caller should hold task->lock.
@@ -59,7 +46,7 @@ static bool __reserve_if_idle(struct rxe_task *task)
*/
static bool __is_done(struct rxe_task *task)
{
- if (work_pending(&task->work))
+ if (task->tasklet.state & BIT(TASKLET_STATE_SCHED))
return false;
if (task->state == TASK_STATE_IDLE ||
@@ -90,23 +77,23 @@ static bool is_done(struct rxe_task *task)
* schedules the task. They must call __reserve_if_idle to
* move the task to busy before calling or scheduling.
* The task can also be moved to drained or invalid
- * by calls to rxe_cleanup_task or rxe_disable_task.
+ * by calls to rxe-cleanup_task or rxe_disable_task.
* In that case tasks which get here are not executed but
* just flushed. The tasks are designed to look to see if
- * there is work to do and then do part of it before returning
+ * there is work to do and do part of it before returning
* here with a return value of zero until all the work
- * has been consumed then it returns a non-zero value.
+ * has been consumed then it retuens a non-zero value.
* The number of times the task can be run is limited by
* max iterations so one task cannot hold the cpu forever.
- * If the limit is hit and work remains the task is rescheduled.
*/
-static void do_task(struct rxe_task *task)
+static void do_task(struct tasklet_struct *t)
{
+ int cont;
+ int ret;
+ struct rxe_task *task = from_tasklet(task, t, tasklet);
unsigned int iterations;
unsigned long flags;
int resched = 0;
- int cont;
- int ret;
WARN_ON(rxe_read(task->qp) <= 0);
@@ -128,22 +115,25 @@ static void do_task(struct rxe_task *task)
} while (ret == 0 && iterations-- > 0);
spin_lock_irqsave(&task->lock, flags);
- /* we're not done yet but we ran out of iterations.
- * yield the cpu and reschedule the task
- */
- if (!ret) {
- task->state = TASK_STATE_IDLE;
- resched = 1;
- goto exit;
- }
-
switch (task->state) {
case TASK_STATE_BUSY:
- task->state = TASK_STATE_IDLE;
+ if (ret) {
+ task->state = TASK_STATE_IDLE;
+ } else {
+ /* This can happen if the client
+ * can add work faster than the
+ * tasklet can finish it.
+ * Reschedule the tasklet and exit
+ * the loop to give up the cpu
+ */
+ task->state = TASK_STATE_IDLE;
+ resched = 1;
+ }
break;
- /* someone tried to schedule the task while we
- * were running, keep going
+ /* someone tried to run the task since the last time we called
+ * func, so we will call one more time regardless of the
+ * return value
*/
case TASK_STATE_ARMED:
task->state = TASK_STATE_BUSY;
@@ -151,24 +141,22 @@ static void do_task(struct rxe_task *task)
break;
case TASK_STATE_DRAINING:
- task->state = TASK_STATE_DRAINED;
+ if (ret)
+ task->state = TASK_STATE_DRAINED;
+ else
+ cont = 1;
break;
default:
WARN_ON(1);
- rxe_dbg_qp(task->qp, "unexpected task state = %d",
- task->state);
- task->state = TASK_STATE_IDLE;
+ rxe_info_qp(task->qp, "unexpected task state = %d", task->state);
}
-exit:
if (!cont) {
task->num_done++;
if (WARN_ON(task->num_done != task->num_sched))
- rxe_dbg_qp(
- task->qp,
- "%ld tasks scheduled, %ld tasks done",
- task->num_sched, task->num_done);
+ rxe_err_qp(task->qp, "%ld tasks scheduled, %ld tasks done",
+ task->num_sched, task->num_done);
}
spin_unlock_irqrestore(&task->lock, flags);
} while (cont);
@@ -181,12 +169,6 @@ static void do_task(struct rxe_task *task)
rxe_put(task->qp);
}
-/* wrapper around do_task to fix argument for work queue */
-static void do_work(struct work_struct *work)
-{
- do_task(container_of(work, struct rxe_task, work));
-}
-
int rxe_init_task(struct rxe_task *task, struct rxe_qp *qp,
int (*func)(struct rxe_qp *))
{
@@ -194,9 +176,11 @@ int rxe_init_task(struct rxe_task *task, struct rxe_qp *qp,
task->qp = qp;
task->func = func;
+
+ tasklet_setup(&task->tasklet, do_task);
+
task->state = TASK_STATE_IDLE;
spin_lock_init(&task->lock);
- INIT_WORK(&task->work, do_work);
return 0;
}
@@ -229,6 +213,8 @@ void rxe_cleanup_task(struct rxe_task *task)
while (!is_done(task))
cond_resched();
+ tasklet_kill(&task->tasklet);
+
spin_lock_irqsave(&task->lock, flags);
task->state = TASK_STATE_INVALID;
spin_unlock_irqrestore(&task->lock, flags);
@@ -240,7 +226,7 @@ void rxe_cleanup_task(struct rxe_task *task)
void rxe_run_task(struct rxe_task *task)
{
unsigned long flags;
- bool run;
+ int run;
WARN_ON(rxe_read(task->qp) <= 0);
@@ -249,11 +235,11 @@ void rxe_run_task(struct rxe_task *task)
spin_unlock_irqrestore(&task->lock, flags);
if (run)
- do_task(task);
+ do_task(&task->tasklet);
}
-/* schedule the task to run later as a work queue entry.
- * the queue_work call can be called holding
+/* schedule the task to run later as a tasklet.
+ * the tasklet)schedule call can be called holding
* the lock.
*/
void rxe_sched_task(struct rxe_task *task)
@@ -264,7 +250,7 @@ void rxe_sched_task(struct rxe_task *task)
spin_lock_irqsave(&task->lock, flags);
if (__reserve_if_idle(task))
- queue_work(rxe_wq, &task->work);
+ tasklet_schedule(&task->tasklet);
spin_unlock_irqrestore(&task->lock, flags);
}
@@ -291,9 +277,7 @@ void rxe_disable_task(struct rxe_task *task)
while (!is_done(task))
cond_resched();
- spin_lock_irqsave(&task->lock, flags);
- task->state = TASK_STATE_DRAINED;
- spin_unlock_irqrestore(&task->lock, flags);
+ tasklet_disable(&task->tasklet);
}
void rxe_enable_task(struct rxe_task *task)
@@ -307,7 +291,7 @@ void rxe_enable_task(struct rxe_task *task)
spin_unlock_irqrestore(&task->lock, flags);
return;
}
-
task->state = TASK_STATE_IDLE;
+ tasklet_enable(&task->tasklet);
spin_unlock_irqrestore(&task->lock, flags);
}
diff --git a/drivers/infiniband/sw/rxe/rxe_task.h b/drivers/infiniband/sw/rxe/rxe_task.h
index a63e258b3d66..facb7c8e3729 100644
--- a/drivers/infiniband/sw/rxe/rxe_task.h
+++ b/drivers/infiniband/sw/rxe/rxe_task.h
@@ -22,7 +22,7 @@ enum {
* called again.
*/
struct rxe_task {
- struct work_struct work;
+ struct tasklet_struct tasklet;
int state;
spinlock_t lock;
struct rxe_qp *qp;
@@ -32,10 +32,6 @@ struct rxe_task {
long num_done;
};
-int rxe_alloc_wq(void);
-
-void rxe_destroy_wq(void);
-
/*
* init rxe_task structure
* qp => parameter to pass to func
--
2.40.1
next reply other threads:[~2023-09-22 16:32 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 16:32 Zhu Yanjun [this message]
2023-09-22 16:42 ` [PATCH 1/1] Revert "RDMA/rxe: Add workqueue support for rxe tasks" Bart Van Assche
2023-09-26 9:43 ` Leon Romanovsky
2023-09-26 9:43 ` Leon Romanovsky
2023-09-26 14:06 ` Leon Romanovsky
2023-09-26 17:05 ` Bart Van Assche
2023-09-26 18:34 ` Bob Pearson
2023-09-26 20:24 ` Bart Van Assche
2023-09-27 0:08 ` Rain River
2023-09-27 16:36 ` Bob Pearson
2023-09-27 16:51 ` Bob Pearson
2023-10-01 6:30 ` Leon Romanovsky
2023-10-04 17:44 ` Bart Van Assche
2023-10-04 3:41 ` Zhu Yanjun
2023-10-04 17:43 ` Bart Van Assche
2023-10-04 18:38 ` Jason Gunthorpe
2023-10-05 9:25 ` Zhu Yanjun
2023-10-05 14:21 ` Jason Gunthorpe
2023-10-05 14:50 ` Bart Van Assche
2023-10-05 15:56 ` Jason Gunthorpe
2023-10-06 15:58 ` Bob Pearson
2023-10-07 0:35 ` Zhu Yanjun
2023-10-08 16:01 ` Zhu Yanjun
2023-10-08 17:09 ` Leon Romanovsky
2023-10-10 4:53 ` Daisuke Matsuda (Fujitsu)
2023-10-10 16:09 ` Jason Gunthorpe
2023-10-10 21:29 ` Bart Van Assche
2023-10-11 15:51 ` Jason Gunthorpe
2023-10-11 20:14 ` Bart Van Assche
2023-10-11 23:12 ` Jason Gunthorpe
2023-10-12 11:49 ` Zhu Yanjun
2023-10-12 15:38 ` Bob Pearson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230922163231.2237811-1-yanjun.zhu@intel.com \
--to=yanjun.zhu@intel.com \
--cc=bvanassche@acm.org \
--cc=dsg6rd66tyiei32zaxs6ddv5ebefr5vtxjwz6d2ewqrcwisogl@ge7jzan7dg5u \
--cc=jgg@ziepe.ca \
--cc=leon@kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=matsuda-daisuke@fujitsu.com \
--cc=rpearsonhpe@gmail.com \
--cc=shinichiro.kawasaki@wdc.com \
--cc=yanjun.zhu@linux.dev \
--cc=zyjzyj2000@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).