On Wed, March 12, 2025 4:19 PM Zhu Yanjun <yanjun.zhu@xxxxxxxxx> wrote:
> 在 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@xxxxxxxxxxx>
> > ---
> > 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.
Thank you for the review.
I had used this "flags" like an enum variable rather than a flag here,
but the latter approach should be better for future extensions.
Daisuke
>
> Except the above, I am fine with this commit.
> Thanks a lot.
>
> Reviewed-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxx>
>
> 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;
> >
