linux-kernel - Re: [PATCH] RDMA/rxe: Remove unused rxe_run

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aALsrZxAqhwxDD7d@gallifrey>
Date: Sat, 19 Apr 2025 00:22:05 +0000
From: "Dr. David Alan Gilbert" <linux@...blig.org>
To: Zhu Yanjun <yanjun.zhu@...ux.dev>
Cc: zyjzyj2000@...il.com, jgg@...pe.ca, leon@...nel.org,
	linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] RDMA/rxe: Remove unused rxe_run_task

* Zhu Yanjun (yanjun.zhu@...ux.dev) wrote:
> 在 2025/4/18 18:59, linux@...blig.org 写道:
> > From: "Dr. David Alan Gilbert" <linux@...blig.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@...ux.dev>

Dave

> Best Regards,
> Zhu Yanjun
> > Signed-off-by: Dr. David Alan Gilbert <linux@...blig.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 */
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/