| Message ID | 20220311115247.23521-4-yangx.jy@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | RDMA/rxe: Add RDMA Atomic Write operation | expand |
On Fri, Mar 11, 2022 at 07:52:47PM +0800, Xiao Yang wrote: > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index abd1c5d3dc66..580b5cacec09 100644 > +++ b/include/rdma/ib_verbs.h > @@ -291,6 +291,8 @@ enum ib_device_cap_flags { > /* The device supports padding incoming writes to cacheline. */ > IB_DEVICE_PCI_WRITE_END_PADDING = (1ULL << 36), > IB_DEVICE_ALLOW_USER_UNREG = (1ULL << 37), > + /* Atomic write attributes */ > + IB_DEVICE_ATOMIC_WRITE = (1ULL << 40), > }; Can you make a patch to clean this up too? The right parts of it need to get moved to the uapi header and it should work similarly to the ib_uverbs_wr_opcode / ib_wr_opcode thing. You'll also need to do something about the 32 bit compatability that kbuild detected - I suppose this can't work on 32 bit platforms? So IS_ENABLED() it off or something? Jason
On 2022/3/16 2:53, Jason Gunthorpe wrote: > Can you make a patch to clean this up too? The right parts of it need > to get moved to the uapi header and it should work similarly to the > ib_uverbs_wr_opcode / ib_wr_opcode thing. > > You'll also need to do something about the 32 bit compatability that > kbuild detected - I suppose this can't work on 32 bit platforms? So > IS_ENABLED() it off or something? Hi Jason, Thanks for your reminder. I will do the cleanup and fix the issue detected by kbuild. Best Regards, Xiao Yang
On 2022/3/16 2:53, Jason Gunthorpe wrote: > You'll also need to do something about the 32 bit compatability that > kbuild detected - I suppose this can't work on 32 bit platforms? So > IS_ENABLED() it off or something? Hi Jason, Is it possible to fix the issue by atomic64_set_release()? If not, we may need to add a check for __native_word(*dst) and return an unsupported error when __native_word(*dst) is false. Best Regards, Xiao Yang
On Mon, Mar 21, 2022 at 03:55:01AM +0000, yangx.jy@fujitsu.com wrote: > On 2022/3/16 2:53, Jason Gunthorpe wrote: > > You'll also need to do something about the 32 bit compatability that > > kbuild detected - I suppose this can't work on 32 bit platforms? So > > IS_ENABLED() it off or something? > Hi Jason, > > Is it possible to fix the issue by atomic64_set_release()? No > If not, we may need to add a check for __native_word(*dst) and return an > unsupported error when __native_word(*dst) is false. The whole feature, including the cap bits should be turned off for 32 bit builds because it cannot possibly work Jason
On 2022/3/21 23:32, Jason Gunthorpe write: > On Mon, Mar 21, 2022 at 03:55:01AM +0000, yangx.jy@fujitsu.com wrote: >> On 2022/3/16 2:53, Jason Gunthorpe wrote: >>> You'll also need to do something about the 32 bit compatability that >>> kbuild detected - I suppose this can't work on 32 bit platforms? So >>> IS_ENABLED() it off or something? >> Hi Jason, >> >> Is it possible to fix the issue by atomic64_set_release()? > No > >> If not, we may need to add a check for __native_word(*dst) and return an >> unsupported error when __native_word(*dst) is false. > The whole feature, including the cap bits should be turned off for 32 > bit builds because it cannot possibly work Hi Jason, Is it ok to disable the whole atomic write by checking CONFIG_64BIT? Best Regards, Xiao Yang > > Jason
On Fri, Mar 25, 2022 at 11:44:53AM +0000, yangx.jy@fujitsu.com wrote: > On 2022/3/21 23:32, Jason Gunthorpe write: > > On Mon, Mar 21, 2022 at 03:55:01AM +0000, yangx.jy@fujitsu.com wrote: > >> On 2022/3/16 2:53, Jason Gunthorpe wrote: > >>> You'll also need to do something about the 32 bit compatability that > >>> kbuild detected - I suppose this can't work on 32 bit platforms? So > >>> IS_ENABLED() it off or something? > >> Hi Jason, > >> > >> Is it possible to fix the issue by atomic64_set_release()? > > No > > > >> If not, we may need to add a check for __native_word(*dst) and return an > >> unsupported error when __native_word(*dst) is false. > > The whole feature, including the cap bits should be turned off for 32 > > bit builds because it cannot possibly work > > Hi Jason, > > Is it ok to disable the whole atomic write by checking CONFIG_64BIT? It is not great, but there is not another choice I can see.. Jason
On 2022/3/25 21:22, Jason Gunthorpe wrote: > It is not great, but there is not another choice I can see.. Hi Jason, I plan to only disable the key places by the following change so that user cannot use the atomic write: ----------------------------------------------------------------------------------------------------------- diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c index e2d332a5af0a..7f83b5e39ace 100644 --- a/drivers/infiniband/sw/rxe/rxe_req.c +++ b/drivers/infiniband/sw/rxe/rxe_req.c @@ -238,8 +238,10 @@ static int next_opcode_rc(struct rxe_qp *qp, u32 opcode, int fits) return fits ? IB_OPCODE_RC_SEND_ONLY_WITH_INVALIDATE : IB_OPCODE_RC_SEND_FIRST; +#ifdef CONFIG_64BIT case IB_WR_RDMA_ATOMIC_WRITE: return IB_OPCODE_RC_RDMA_ATOMIC_WRITE; +#endif /* CONFIG_64BIT */ case IB_WR_REG_MR: case IB_WR_LOCAL_INV: diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c index ddc791b199e2..311ec523fb57 100644 --- a/drivers/infiniband/sw/rxe/rxe_resp.c +++ b/drivers/infiniband/sw/rxe/rxe_resp.c @@ -592,6 +592,7 @@ static enum resp_states process_atomic(struct rxe_qp *qp, return ret; } +#ifdef CONFIG_64BIT static enum resp_states process_atomic_write(struct rxe_qp *qp, struct rxe_pkt_info *pkt) { @@ -613,6 +614,7 @@ static enum resp_states process_atomic_write(struct rxe_qp *qp, return RESPST_NONE; } +#endif /* CONFIG_64BIT */ static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt, @@ -871,10 +873,12 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt) err = process_atomic(qp, pkt); if (err) return err; +#ifdef CONFIG_64BIT } else if (pkt->mask & RXE_ATOMIC_WRITE_MASK) { err = process_atomic_write(qp, pkt); if (err) return err; +#endif /* CONFIG_64BIT */ } else { /* Unreachable */ WARN_ON_ONCE(1); ----------------------------------------------------------------------------------------------------------- Best Regards, Xiao Yang > > Jason
On Mon, Mar 28, 2022 at 10:07:26AM +0000, yangx.jy@fujitsu.com wrote: > On 2022/3/25 21:22, Jason Gunthorpe wrote: > > It is not great, but there is not another choice I can see.. > > Hi Jason, > > I plan to only disable the key places by the following change so that > user cannot use the atomic write: Isn't there a cap flag too? Jason
On 2022/3/28 19:39, Jason Gunthorpe wrote: > On Mon, Mar 28, 2022 at 10:07:26AM +0000, yangx.jy@fujitsu.com wrote: >> On 2022/3/25 21:22, Jason Gunthorpe wrote: >>> It is not great, but there is not another choice I can see.. >> Hi Jason, >> >> I plan to only disable the key places by the following change so that >> user cannot use the atomic write: > Isn't there a cap flag too? Hi Jason, I will disable the atomic write cap flag as well, like this: ----------------------------------- diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h index 918270e34a35..88953f9c26e4 100644 --- a/drivers/infiniband/sw/rxe/rxe_param.h +++ b/drivers/infiniband/sw/rxe/rxe_param.h @@ -53,7 +53,12 @@ enum rxe_device_param { | IB_DEVICE_ALLOW_USER_UNREG | IB_DEVICE_MEM_WINDOW | IB_DEVICE_MEM_WINDOW_TYPE_2A +#ifdef CONFIG_64BIT + | IB_DEVICE_MEM_WINDOW_TYPE_2B + | IB_DEVICE_ATOMIC_WRITE, +#else | IB_DEVICE_MEM_WINDOW_TYPE_2B, +#endif /* CONFIG_64BIT */ RXE_MAX_SGE = 32, RXE_MAX_WQE_SIZE = sizeof(struct rxe_send_wqe) + sizeof(struct ib_sge) * RXE_MAX_SGE, ----------------------------------- BTW: I hope we can review and merge my cleanup patchset[1][2] first. So that I will update the third patch[3] based on it. ^_^ [1]: [PATCH v2 1/2] IB/uverbs: Move enum ib_raw_packet_caps to uapi [2]: [PATCH v2 2/2] IB/uverbs: Move part of enum ib_device_cap_flags to uapi [3]: [PATCH v3 3/3] RDMA/rxe: Add RDMA Atomic Write attribute for rxe device Best Regards, Xiao Yang > > Jason
On 2022/3/29 10:36, yangx.jy@fujitsu.com wrote: > On 2022/3/28 19:39, Jason Gunthorpe wrote: >> On Mon, Mar 28, 2022 at 10:07:26AM +0000, yangx.jy@fujitsu.com wrote: >>> On 2022/3/25 21:22, Jason Gunthorpe wrote: >>>> It is not great, but there is not another choice I can see.. >>> Hi Jason, >>> >>> I plan to only disable the key places by the following change so that >>> user cannot use the atomic write: >> Isn't there a cap flag too? > Hi Jason, > > I will disable the atomic write cap flag as well, like this: > > ----------------------------------- > > diff --git a/drivers/infiniband/sw/rxe/rxe_param.h > b/drivers/infiniband/sw/rxe/rxe_param.h > index 918270e34a35..88953f9c26e4 100644 > --- a/drivers/infiniband/sw/rxe/rxe_param.h > +++ b/drivers/infiniband/sw/rxe/rxe_param.h > @@ -53,7 +53,12 @@ enum rxe_device_param { > | IB_DEVICE_ALLOW_USER_UNREG > | IB_DEVICE_MEM_WINDOW > | IB_DEVICE_MEM_WINDOW_TYPE_2A > +#ifdef CONFIG_64BIT > + | IB_DEVICE_MEM_WINDOW_TYPE_2B > + | IB_DEVICE_ATOMIC_WRITE, > +#else > | IB_DEVICE_MEM_WINDOW_TYPE_2B, > +#endif /* CONFIG_64BIT */ > RXE_MAX_SGE = 32, > RXE_MAX_WQE_SIZE = sizeof(struct rxe_send_wqe) + > sizeof(struct ib_sge) * > RXE_MAX_SGE, > > ----------------------------------- > > BTW: > > I hope we can review and merge my cleanup patchset[1][2] first. So that > I will update the third patch[3] based on it. ^_^ > > [1]: [PATCH v2 1/2] IB/uverbs: Move enum ib_raw_packet_caps to uapi > > [2]: [PATCH v2 2/2] IB/uverbs: Move part of enum ib_device_cap_flags to uapi Sorry, the correct version is my v3 cleanup patchset [1]: [PATCH v3 1/2] IB/uverbs: Move enum ib_raw_packet_caps to uapi [2]: [PATCH v3 2/2] IB/uverbs: Move part of enum ib_device_cap_flags to uapi > > [3]: [PATCH v3 3/3] RDMA/rxe: Add RDMA Atomic Write attribute for rxe device > > Best Regards, > > Xiao Yang > >> Jason
diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h index 918270e34a35..6ae6c4079639 100644 --- a/drivers/infiniband/sw/rxe/rxe_param.h +++ b/drivers/infiniband/sw/rxe/rxe_param.h @@ -53,7 +53,8 @@ enum rxe_device_param { | IB_DEVICE_ALLOW_USER_UNREG | IB_DEVICE_MEM_WINDOW | IB_DEVICE_MEM_WINDOW_TYPE_2A - | IB_DEVICE_MEM_WINDOW_TYPE_2B, + | IB_DEVICE_MEM_WINDOW_TYPE_2B + | IB_DEVICE_ATOMIC_WRITE, RXE_MAX_SGE = 32, RXE_MAX_WQE_SIZE = sizeof(struct rxe_send_wqe) + sizeof(struct ib_sge) * RXE_MAX_SGE, diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index abd1c5d3dc66..580b5cacec09 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -291,6 +291,8 @@ enum ib_device_cap_flags { /* The device supports padding incoming writes to cacheline. */ IB_DEVICE_PCI_WRITE_END_PADDING = (1ULL << 36), IB_DEVICE_ALLOW_USER_UNREG = (1ULL << 37), + /* Atomic write attributes */ + IB_DEVICE_ATOMIC_WRITE = (1ULL << 40), }; enum ib_atomic_cap {
The attribute shows that rxe device supports RDMA Atomic Write operation. Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> --- drivers/infiniband/sw/rxe/rxe_param.h | 3 ++- include/rdma/ib_verbs.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-)