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

[Bob Pearson <rpearsonhpe@gmail.com>]

On 4/3/23 23:58, Leon Romanovsky wrote:
> 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

Leon,

Then drop the workqueue patch and apply the fix rxe_task.c patch and I will resubmit the workqueue patch.

Thanks

Bob