| Message ID | 20250418165948.241433-1-linux@treblig.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RDMA/rxe: Remove unused rxe_run_task | expand |
在 2025/4/18 18:59, linux@treblig.org 写道: > From: "Dr. David Alan Gilbert" <linux@treblig.org> > > rxe_run_task() has been unused since 2024's > commit 23bc06af547f ("RDMA/rxe: Don't call direct between tasks") > > Remove it. > Thanks a lot. Please add the Fixes tags. Fixes: 23bc06af547f ("RDMA/rxe: Don't call direct between tasks") And in the following comments, the function rxe_run_task is still mentioned. " 86 /* do_task is a wrapper for the three tasks (requester, 87 * completer, responder) and calls them in a loop until 88 * they return a non-zero value. It is called either 89 * directly by rxe_run_task or indirectly if rxe_sched_task 90 * schedules the task. They must call __reserve_if_idle to 91 * move the task to busy before calling or scheduling. 92 * The task can also be moved to drained or invalid 93 * by calls to rxe_cleanup_task or rxe_disable_task. 94 * In that case tasks which get here are not executed but 95 * just flushed. The tasks are designed to look to see if 96 * there is work to do and then do part of it before returning 97 * here with a return value of zero until all the work 98 * has been consumed then it returns a non-zero value. 99 * The number of times the task can be run is limited by 100 * max iterations so one task cannot hold the cpu forever. 101 * If the limit is hit and work remains the task is rescheduled. 102 */ " Not sure if you like to modify the above comments to remove rxe_run_task or not. Except the above, I am fine with this commit. Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev> Best Regards, Zhu Yanjun > Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org> > --- > drivers/infiniband/sw/rxe/rxe_task.c | 18 ------------------ > drivers/infiniband/sw/rxe/rxe_task.h | 2 -- > 2 files changed, 20 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c > index 80332638d9e3..9d02d847fd78 100644 > --- a/drivers/infiniband/sw/rxe/rxe_task.c > +++ b/drivers/infiniband/sw/rxe/rxe_task.c > @@ -234,24 +234,6 @@ void rxe_cleanup_task(struct rxe_task *task) > spin_unlock_irqrestore(&task->lock, flags); > } > > -/* run the task inline if it is currently idle > - * cannot call do_task holding the lock > - */ > -void rxe_run_task(struct rxe_task *task) > -{ > - unsigned long flags; > - bool run; > - > - WARN_ON(rxe_read(task->qp) <= 0); > - > - spin_lock_irqsave(&task->lock, flags); > - run = __reserve_if_idle(task); > - spin_unlock_irqrestore(&task->lock, flags); > - > - if (run) > - do_task(task); > -} > - > /* schedule the task to run later as a work queue entry. > * the queue_work call can be called holding > * the lock. > diff --git a/drivers/infiniband/sw/rxe/rxe_task.h b/drivers/infiniband/sw/rxe/rxe_task.h > index a63e258b3d66..a8c9a77b6027 100644 > --- a/drivers/infiniband/sw/rxe/rxe_task.h > +++ b/drivers/infiniband/sw/rxe/rxe_task.h > @@ -47,8 +47,6 @@ int rxe_init_task(struct rxe_task *task, struct rxe_qp *qp, > /* cleanup task */ > void rxe_cleanup_task(struct rxe_task *task); > > -void rxe_run_task(struct rxe_task *task); > - > void rxe_sched_task(struct rxe_task *task); > > /* keep a task from scheduling */
* Zhu Yanjun (yanjun.zhu@linux.dev) wrote: > 在 2025/4/18 18:59, linux@treblig.org 写道: > > From: "Dr. David Alan Gilbert" <linux@treblig.org> > > > > rxe_run_task() has been unused since 2024's > > commit 23bc06af547f ("RDMA/rxe: Don't call direct between tasks") > > > > Remove it. > > > Hi, > Thanks a lot. Please add the Fixes tags. > Fixes: 23bc06af547f ("RDMA/rxe: Don't call direct between tasks") Thanks for the review; I've tended to avoid the fixes tag because people use 'Fixes' to automatically pull in patches to stable or downstream kernels, and there is no need for them to do that for a cleanup patch. > And in the following comments, the function rxe_run_task is still mentioned. > " > 86 /* do_task is a wrapper for the three tasks (requester, > 87 * completer, responder) and calls them in a loop until > 88 * they return a non-zero value. It is called either > 89 * directly by rxe_run_task or indirectly if rxe_sched_task > 90 * schedules the task. They must call __reserve_if_idle to > 91 * move the task to busy before calling or scheduling. > 92 * The task can also be moved to drained or invalid > 93 * by calls to rxe_cleanup_task or rxe_disable_task. > 94 * In that case tasks which get here are not executed but > 95 * just flushed. The tasks are designed to look to see if > 96 * there is work to do and then do part of it before returning > 97 * here with a return value of zero until all the work > 98 * has been consumed then it returns a non-zero value. > 99 * The number of times the task can be run is limited by > 100 * max iterations so one task cannot hold the cpu forever. > 101 * If the limit is hit and work remains the task is rescheduled. > 102 */ > " > Not sure if you like to modify the above comments to remove rxe_run_task or > not. Would it be correct to just reword: > 88 * It is called either > 89 * directly by rxe_run_task or indirectly if rxe_sched_task > 90 * schedules the task. to: It is called indirectly when rxe_sched_task schedules the task. > Except the above, I am fine with this commit. Thanks! > Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev> Dave > Best Regards, > Zhu Yanjun > > Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org> > > --- > > drivers/infiniband/sw/rxe/rxe_task.c | 18 ------------------ > > drivers/infiniband/sw/rxe/rxe_task.h | 2 -- > > 2 files changed, 20 deletions(-) > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c > > index 80332638d9e3..9d02d847fd78 100644 > > --- a/drivers/infiniband/sw/rxe/rxe_task.c > > +++ b/drivers/infiniband/sw/rxe/rxe_task.c > > @@ -234,24 +234,6 @@ void rxe_cleanup_task(struct rxe_task *task) > > spin_unlock_irqrestore(&task->lock, flags); > > } > > -/* run the task inline if it is currently idle > > - * cannot call do_task holding the lock > > - */ > > -void rxe_run_task(struct rxe_task *task) > > -{ > > - unsigned long flags; > > - bool run; > > - > > - WARN_ON(rxe_read(task->qp) <= 0); > > - > > - spin_lock_irqsave(&task->lock, flags); > > - run = __reserve_if_idle(task); > > - spin_unlock_irqrestore(&task->lock, flags); > > - > > - if (run) > > - do_task(task); > > -} > > - > > /* schedule the task to run later as a work queue entry. > > * the queue_work call can be called holding > > * the lock. > > diff --git a/drivers/infiniband/sw/rxe/rxe_task.h b/drivers/infiniband/sw/rxe/rxe_task.h > > index a63e258b3d66..a8c9a77b6027 100644 > > --- a/drivers/infiniband/sw/rxe/rxe_task.h > > +++ b/drivers/infiniband/sw/rxe/rxe_task.h > > @@ -47,8 +47,6 @@ int rxe_init_task(struct rxe_task *task, struct rxe_qp *qp, > > /* cleanup task */ > > void rxe_cleanup_task(struct rxe_task *task); > > -void rxe_run_task(struct rxe_task *task); > > - > > void rxe_sched_task(struct rxe_task *task); > > /* keep a task from scheduling */ >
在 2025/4/19 2:22, Dr. David Alan Gilbert 写道: > > Hi, > >> Thanks a lot. Please add the Fixes tags. >> Fixes: 23bc06af547f ("RDMA/rxe: Don't call direct between tasks") > > Thanks for the review; I've tended to avoid the fixes tag because > people use 'Fixes' to automatically pull in patches to stable or > downstream kernels, and there is no need for them to do that for > a cleanup patch. > >> And in the following comments, the function rxe_run_task is still mentioned. >> " >> 86 /* do_task is a wrapper for the three tasks (requester, >> 87 * completer, responder) and calls them in a loop until >> 88 * they return a non-zero value. It is called either >> 89 * directly by rxe_run_task or indirectly if rxe_sched_task >> 90 * schedules the task. They must call __reserve_if_idle to >> 91 * move the task to busy before calling or scheduling. >> 92 * The task can also be moved to drained or invalid >> 93 * by calls to rxe_cleanup_task or rxe_disable_task. >> 94 * In that case tasks which get here are not executed but >> 95 * just flushed. The tasks are designed to look to see if >> 96 * there is work to do and then do part of it before returning >> 97 * here with a return value of zero until all the work >> 98 * has been consumed then it returns a non-zero value. >> 99 * The number of times the task can be run is limited by >> 100 * max iterations so one task cannot hold the cpu forever. >> 101 * If the limit is hit and work remains the task is rescheduled. >> 102 */ >> " >> Not sure if you like to modify the above comments to remove rxe_run_task or >> not. > > Would it be correct to just reword: >> 88 * It is called either >> 89 * directly by rxe_run_task or indirectly if rxe_sched_task >> 90 * schedules the task. > > to: > It is called indirectly when rxe_sched_task schedules the task. I am fine with it. Thanks a lot. Zhu Yanjun > >> Except the above, I am fine with this commit. > > Thanks! > >> Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev> > > Dave >
* Zhu Yanjun (yanjun.zhu@linux.dev) wrote: > 在 2025/4/19 2:22, Dr. David Alan Gilbert 写道: > > > > Hi, > > > > > Thanks a lot. Please add the Fixes tags. > > > Fixes: 23bc06af547f ("RDMA/rxe: Don't call direct between tasks") > > > > Thanks for the review; I've tended to avoid the fixes tag because > > people use 'Fixes' to automatically pull in patches to stable or > > downstream kernels, and there is no need for them to do that for > > a cleanup patch. > > > > > And in the following comments, the function rxe_run_task is still mentioned. > > > " > > > 86 /* do_task is a wrapper for the three tasks (requester, > > > 87 * completer, responder) and calls them in a loop until > > > 88 * they return a non-zero value. It is called either > > > 89 * directly by rxe_run_task or indirectly if rxe_sched_task > > > 90 * schedules the task. They must call __reserve_if_idle to > > > 91 * move the task to busy before calling or scheduling. > > > 92 * The task can also be moved to drained or invalid > > > 93 * by calls to rxe_cleanup_task or rxe_disable_task. > > > 94 * In that case tasks which get here are not executed but > > > 95 * just flushed. The tasks are designed to look to see if > > > 96 * there is work to do and then do part of it before returning > > > 97 * here with a return value of zero until all the work > > > 98 * has been consumed then it returns a non-zero value. > > > 99 * The number of times the task can be run is limited by > > > 100 * max iterations so one task cannot hold the cpu forever. > > > 101 * If the limit is hit and work remains the task is rescheduled. > > > 102 */ > > > " > > > Not sure if you like to modify the above comments to remove rxe_run_task or > > > not. > > > > Would it be correct to just reword: > > > 88 * It is called either > > > 89 * directly by rxe_run_task or indirectly if rxe_sched_task > > > 90 * schedules the task. > > > > to: > > It is called indirectly when rxe_sched_task schedules the task. > > I am fine with it. Thanks a lot. Thanks, v2 sent. Dave > Zhu Yanjun > > > > > > Except the above, I am fine with this commit. > > > > Thanks! > > > > > Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev> > > > > Dave > > >
diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c index 80332638d9e3..9d02d847fd78 100644 --- a/drivers/infiniband/sw/rxe/rxe_task.c +++ b/drivers/infiniband/sw/rxe/rxe_task.c @@ -234,24 +234,6 @@ void rxe_cleanup_task(struct rxe_task *task) spin_unlock_irqrestore(&task->lock, flags); } -/* run the task inline if it is currently idle - * cannot call do_task holding the lock - */ -void rxe_run_task(struct rxe_task *task) -{ - unsigned long flags; - bool run; - - WARN_ON(rxe_read(task->qp) <= 0); - - spin_lock_irqsave(&task->lock, flags); - run = __reserve_if_idle(task); - spin_unlock_irqrestore(&task->lock, flags); - - if (run) - do_task(task); -} - /* schedule the task to run later as a work queue entry. * the queue_work call can be called holding * the lock. diff --git a/drivers/infiniband/sw/rxe/rxe_task.h b/drivers/infiniband/sw/rxe/rxe_task.h index a63e258b3d66..a8c9a77b6027 100644 --- a/drivers/infiniband/sw/rxe/rxe_task.h +++ b/drivers/infiniband/sw/rxe/rxe_task.h @@ -47,8 +47,6 @@ int rxe_init_task(struct rxe_task *task, struct rxe_qp *qp, /* cleanup task */ void rxe_cleanup_task(struct rxe_task *task); -void rxe_run_task(struct rxe_task *task); - void rxe_sched_task(struct rxe_task *task); /* keep a task from scheduling */