Re: [PATCH] RDMA/rxe: Fix error in rxe_task.c

[Leon Romanovsky <leon@kernel.org>]

On Mon, Apr 03, 2023 at 03:38:13PM -0500, Bob Pearson wrote:
> On 3/29/23 14:33, Bob Pearson wrote:
> > In a previous patch TASKLET_STATE_SCHED was used as a bit but it
> > is a bit position instead. This patch corrects that error.
> > 
> > Reported-by: Dan Carpenter <error27@gmail.com>
> > Link: https://lore.kernel.org/linux-rdma/8a054b78-6d50-4bc6-8d8a-83f85fbdb82f@kili.mountain/
> > Fixes: d94671632572 ("RDMA/rxe: Rewrite rxe_task.c")
> > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> > ---
> >  drivers/infiniband/sw/rxe/rxe_task.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c
> > index fea9a517c8d9..fb9a6bc8e620 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_task.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_task.c
> > @@ -21,7 +21,7 @@ static bool __reserve_if_idle(struct rxe_task *task)
> >  {
> >  	WARN_ON(rxe_read(task->qp) <= 0);
> >  
> > -	if (task->tasklet.state & TASKLET_STATE_SCHED)
> > +	if (task->tasklet.state & BIT(TASKLET_STATE_SCHED))
> >  		return false;
> >  
> >  	if (task->state == TASK_STATE_IDLE) {
> > @@ -46,7 +46,7 @@ static bool __reserve_if_idle(struct rxe_task *task)
> >   */
> >  static bool __is_done(struct rxe_task *task)
> >  {
> > -	if (task->tasklet.state & TASKLET_STATE_SCHED)
> > +	if (task->tasklet.state & BIT(TASKLET_STATE_SCHED))
> >  		return false;
> >  
> >  	if (task->state == TASK_STATE_IDLE ||
> 
> This patch fixes a bug in rxe_task.c introduced by the earlier patch (d94671632572 RDMA/rxe: Rewrite rxe_task.c)
> which is in for-next. The bug actually has minimal effects because TASKLET_STATE_SCHED is zero and in testing
> so far it doesn't seem to make a difference.
> 
> There is a second patch currently in patchworks ([for-next,v6] RDMA/rxe: Add workqueue support for tasks
> [for-next,v6] RDMA/rxe: Add workqueue support for tasks 	- - - 	--- 	2023-03-02 	Bob Pearson New)
> which is ahead of this one and replaces the tasklet implementation by work queues. This second patch replaces the
> lines lines containing the error with a workqueue specific equivalent.
> 
> There are two ways forward here. We could fix the tasklet version by applying this patch first or ignore the
> error and apply the workqueue patch first. My desire is to get rid of tasklets altogether so I prefer the
> second choice. If we choose the first choice then we need to reorder the two patches in patchworks and
> rebase the workqueue patch to match the fixed tasklet code.
> 
> Please suggest how you would like me to proceed.

I prefer to have small fix first as it is unclear when "RDMA/rxe: Add workqueue support for tasks"
will be merged.
https://lore.kernel.org/all/20230330053933.GJ831478@unreal

Thanks

> 
> Bob