Re: [PATCH for-next] RDMA/rxe: Improve readability of ODP pagefault interface

[Zhu Yanjun <yanjun.zhu@linux.dev>]

在 2025/3/12 7:59, Daisuke Matsuda 写道:
> Use a meaningful constant explicitly instead of hard-coding a literal.
> 
> Signed-off-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com>
> ---
>   drivers/infiniband/sw/rxe/rxe_odp.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_odp.c b/drivers/infiniband/sw/rxe/rxe_odp.c
> index a82e5011360c..94f7bbe14981 100644
> --- a/drivers/infiniband/sw/rxe/rxe_odp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_odp.c
> @@ -37,8 +37,9 @@ const struct mmu_interval_notifier_ops rxe_mn_ops = {
>   	.invalidate = rxe_ib_invalidate_range,
>   };
>   
> -#define RXE_PAGEFAULT_RDONLY BIT(1)
> -#define RXE_PAGEFAULT_SNAPSHOT BIT(2)
> +#define RXE_PAGEFAULT_DEFAULT 0
> +#define RXE_PAGEFAULT_RDONLY BIT(0)
> +#define RXE_PAGEFAULT_SNAPSHOT BIT(1)
>   static int rxe_odp_do_pagefault_and_lock(struct rxe_mr *mr, u64 user_va, int bcnt, u32 flags)
>   {
>   	struct ib_umem_odp *umem_odp = to_ib_umem_odp(mr->umem);
> @@ -222,7 +223,7 @@ int rxe_odp_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length,
>   		    enum rxe_mr_copy_dir dir)
>   {
>   	struct ib_umem_odp *umem_odp = to_ib_umem_odp(mr->umem);
> -	u32 flags = 0;
> +	u32 flags = RXE_PAGEFAULT_DEFAULT;
>   	int err;
>   
>   	if (length == 0)
> @@ -236,7 +237,7 @@ int rxe_odp_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length,
>   		break;
>   
>   	case RXE_FROM_MR_OBJ:
> -		flags = RXE_PAGEFAULT_RDONLY;
> +		flags |= RXE_PAGEFAULT_RDONLY;

The above "|=" is different from the original "=". I am not sure if it 
is correct or not. But in this function, because flags is set 0 at 
first. Thus, the result is the same.

Except the above, I am fine with this commit.
Thanks a lot.

Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>

Zhu Yanjun

>   		break;
>   
>   	default:
> @@ -312,7 +313,8 @@ int rxe_odp_atomic_op(struct rxe_mr *mr, u64 iova, int opcode,
>   	struct ib_umem_odp *umem_odp = to_ib_umem_odp(mr->umem);
>   	int err;
>   
> -	err = rxe_odp_map_range_and_lock(mr, iova, sizeof(char), 0);
> +	err = rxe_odp_map_range_and_lock(mr, iova, sizeof(char),
> +					 RXE_PAGEFAULT_DEFAULT);
>   	if (err < 0)
>   		return err;
>