Re: [PATCH for-next 15/16] RDMA/rxe: Add workqueue support for tasks

[Leon Romanovsky <leon@kernel.org>]

On Mon, Oct 17, 2022 at 11:33:46PM -0500, Bob Pearson wrote:
> Add a third task type RXE_TASK_TYPE_WORKQUEUE to rxe_task.c.

Why do you need an extra type and not instead of RXE_TASK_TYPE_TASKLET?

> 
> Signed-off-by: Ian Ziemba <ian.ziemba@hpe.com>
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe.c      |  9 ++-
>  drivers/infiniband/sw/rxe/rxe_task.c | 84 ++++++++++++++++++++++++++++
>  drivers/infiniband/sw/rxe/rxe_task.h | 10 +++-
>  3 files changed, 101 insertions(+), 2 deletions(-)

<...>

> +static struct workqueue_struct *rxe_wq;
> +
> +int rxe_alloc_wq(void)
> +{
> +	rxe_wq = alloc_workqueue("rxe_wq", WQ_MEM_RECLAIM |
> +				WQ_HIGHPRI | WQ_CPU_INTENSIVE |
> +				WQ_SYSFS, WQ_MAX_ACTIVE);

Are you sure that all these flags can be justified? WQ_MEM_RECLAIM?

> +
> +	if (!rxe_wq)
> +		return -ENOMEM;
> +
> +	return 0;
> +}

<...>

> +static void work_sched(struct rxe_task *task)
> +{
> +	if (!task->valid)
> +		return;
> +
> +	queue_work(rxe_wq, &task->work);
> +}
> +
> +static void work_do_task(struct work_struct *work)
> +{
> +	struct rxe_task *task = container_of(work, typeof(*task), work);
> +
> +	if (!task->valid)
> +		return;

How can it be that submitted task is not valid? Especially without any
locking.

> +
> +	do_task(task);
> +}

Thanks

> +