RE: [PATCH for-next v2 18/18] RDMA/rxe: Add parameters to control task type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Oct 22, 2022 5:01 AM Bob Pearson:
> 
> Add modparams to control the task types for req, comp, and resp
> tasks.
> 
> It is expected that the work queue version will take the place of
> the tasklet version once this patch series is accepted and moved
> upstream. However, for now it is convenient to keep the option of
> easily switching between the versions to help benchmarking and
> debugging.
> 
> Signed-off-by: Ian Ziemba <ian.ziemba@xxxxxxx>
> Signed-off-by: Bob Pearson <rpearsonhpe@xxxxxxxxx>
> ---
>  drivers/infiniband/sw/rxe/rxe_qp.c   | 6 +++---
>  drivers/infiniband/sw/rxe/rxe_task.c | 8 ++++++++
>  drivers/infiniband/sw/rxe/rxe_task.h | 4 ++++
>  3 files changed, 15 insertions(+), 3 deletions(-)

<...>

> diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c
> index 9b8c9d28ee46..4568c4a05e85 100644
> --- a/drivers/infiniband/sw/rxe/rxe_task.c
> +++ b/drivers/infiniband/sw/rxe/rxe_task.c
> @@ -6,6 +6,14 @@
> 
>  #include "rxe.h"
> 
> +int rxe_req_task_type = RXE_TASK_TYPE_TASKLET;
> +int rxe_comp_task_type = RXE_TASK_TYPE_TASKLET;
> +int rxe_resp_task_type = RXE_TASK_TYPE_TASKLET;

As the tasklet version is to be eliminated in near future, why
don't we make the workqueue version default now?


> +
> +module_param_named(req_task_type, rxe_req_task_type, int, 0664);
> +module_param_named(comp_task_type, rxe_comp_task_type, int, 0664);
> +module_param_named(resp_task_type, rxe_resp_task_type, int, 0664);

As I have commented to the 7th patch, users would not benefit from
specifying the 'inline' type directly. It is OK to have the mode internally
to keep the source code simple, but RXE_TASK_TYPE_INLINE should
not be exposed to users.

I suggest that these module parameters be bool type and task types
be like this:
=== rxe_task.h===
enum rxe_task_type {
        RXE_TASK_TYPE_WORKQUEUE = 0,
        RXE_TASK_TYPE_TASKLET   = 1,
        RXE_TASK_TYPE_INLINE    = 2,
};
=============
In this case, while we can preserve the 'inline' type internally,
we can prohibit users from specifying any value other than
'workqueue' or 'tasklet'; modprobe fails if non-boolean values
are passed.

If you still want to keep the parameters int type, then you need
to add some code to perform value check. We can specify an 
arbitrary int value with the current implementation.

Thanks,
Daisuke

> +
>  static struct workqueue_struct *rxe_wq;
> 
>  int rxe_alloc_wq(void)
> diff --git a/drivers/infiniband/sw/rxe/rxe_task.h b/drivers/infiniband/sw/rxe/rxe_task.h
> index d1156b935635..5a2ac7ada05b 100644
> --- a/drivers/infiniband/sw/rxe/rxe_task.h
> +++ b/drivers/infiniband/sw/rxe/rxe_task.h
> @@ -7,6 +7,10 @@
>  #ifndef RXE_TASK_H
>  #define RXE_TASK_H
> 
> +extern int rxe_req_task_type;
> +extern int rxe_comp_task_type;
> +extern int rxe_resp_task_type;
> +
>  struct rxe_task;
> 
>  struct rxe_task_ops {
> --
> 2.34.1





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux