| Message ID | 20250312065937.1787241-1-matsuda-daisuke@fujitsu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [for-next] RDMA/rxe: Improve readability of ODP pagefault interface | expand |
在 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; >
On Wed, March 12, 2025 4:19 PM Zhu Yanjun <yanjun.zhu@linux.dev> 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@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. 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@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; > >
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; 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;
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(-)