Message ID | 1562736764-31752-2-git-send-email-santosh.shilimkar@oracle.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net,1/5] rds: fix reordering with composite message notification | expand |
On 2019/7/10 13:32, Santosh Shilimkar wrote: > RDS composite message(rdma + control) user notification needs to be > triggered once the full message is delivered and such a fix was > added as part of commit 941f8d55f6d61 ("RDS: RDMA: Fix the composite > message user notification"). But rds_send_remove_from_sock is missing > data part notify check and hence at times the user don't get > notification which isn't desirable. > > One way is to fix the rds_send_remove_from_sock to check of that case > but considering the ordering complexity with completion handler and > rdma + control messages are always dispatched back to back in same send > context, just delaying the signaled completion on rmda work request also > gets the desired behaviour. i.e Notifying application only after > RDMA + control message send completes. So patch updates the earlier > fix with this approach. The delay signaling completions of rdma op > till the control message send completes fix was done by Venkat > Venkatsubra in downstream kernel. > > Reviewed-and-tested-by: Zhu Yanjun <yanjun.zhu@oracle.com> Thanks. I am fine with this. Zhu Yanjun > Reviewed-by: Gerd Rausch <gerd.rausch@oracle.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@oracle.com> > --- > net/rds/ib_send.c | 29 +++++++++++++---------------- > net/rds/rdma.c | 10 ---------- > net/rds/rds.h | 1 - > net/rds/send.c | 4 +--- > 4 files changed, 14 insertions(+), 30 deletions(-) > > diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c > index 18f2341..dfe6237 100644 > --- a/net/rds/ib_send.c > +++ b/net/rds/ib_send.c > @@ -69,6 +69,16 @@ static void rds_ib_send_complete(struct rds_message *rm, > complete(rm, notify_status); > } > > +static void rds_ib_send_unmap_data(struct rds_ib_connection *ic, > + struct rm_data_op *op, > + int wc_status) > +{ > + if (op->op_nents) > + ib_dma_unmap_sg(ic->i_cm_id->device, > + op->op_sg, op->op_nents, > + DMA_TO_DEVICE); > +} > + > static void rds_ib_send_unmap_rdma(struct rds_ib_connection *ic, > struct rm_rdma_op *op, > int wc_status) > @@ -129,21 +139,6 @@ static void rds_ib_send_unmap_atomic(struct rds_ib_connection *ic, > rds_ib_stats_inc(s_ib_atomic_fadd); > } > > -static void rds_ib_send_unmap_data(struct rds_ib_connection *ic, > - struct rm_data_op *op, > - int wc_status) > -{ > - struct rds_message *rm = container_of(op, struct rds_message, data); > - > - if (op->op_nents) > - ib_dma_unmap_sg(ic->i_cm_id->device, > - op->op_sg, op->op_nents, > - DMA_TO_DEVICE); > - > - if (rm->rdma.op_active && rm->data.op_notify) > - rds_ib_send_unmap_rdma(ic, &rm->rdma, wc_status); > -} > - > /* > * Unmap the resources associated with a struct send_work. > * > @@ -902,7 +897,9 @@ int rds_ib_xmit_rdma(struct rds_connection *conn, struct rm_rdma_op *op) > send->s_queued = jiffies; > send->s_op = NULL; > > - nr_sig += rds_ib_set_wr_signal_state(ic, send, op->op_notify); > + if (!op->op_notify) > + nr_sig += rds_ib_set_wr_signal_state(ic, send, > + op->op_notify); > > send->s_wr.opcode = op->op_write ? IB_WR_RDMA_WRITE : IB_WR_RDMA_READ; > send->s_rdma_wr.remote_addr = remote_addr; > diff --git a/net/rds/rdma.c b/net/rds/rdma.c > index b340ed4..916f5ec 100644 > --- a/net/rds/rdma.c > +++ b/net/rds/rdma.c > @@ -641,16 +641,6 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, > } > op->op_notifier->n_user_token = args->user_token; > op->op_notifier->n_status = RDS_RDMA_SUCCESS; > - > - /* Enable rmda notification on data operation for composite > - * rds messages and make sure notification is enabled only > - * for the data operation which follows it so that application > - * gets notified only after full message gets delivered. > - */ > - if (rm->data.op_sg) { > - rm->rdma.op_notify = 0; > - rm->data.op_notify = !!(args->flags & RDS_RDMA_NOTIFY_ME); > - } > } > > /* The cookie contains the R_Key of the remote memory region, and > diff --git a/net/rds/rds.h b/net/rds/rds.h > index 0d8f67c..f0066d1 100644 > --- a/net/rds/rds.h > +++ b/net/rds/rds.h > @@ -476,7 +476,6 @@ struct rds_message { > } rdma; > struct rm_data_op { > unsigned int op_active:1; > - unsigned int op_notify:1; > unsigned int op_nents; > unsigned int op_count; > unsigned int op_dmasg; > diff --git a/net/rds/send.c b/net/rds/send.c > index 166dd57..031b1e9 100644 > --- a/net/rds/send.c > +++ b/net/rds/send.c > @@ -491,14 +491,12 @@ void rds_rdma_send_complete(struct rds_message *rm, int status) > struct rm_rdma_op *ro; > struct rds_notifier *notifier; > unsigned long flags; > - unsigned int notify = 0; > > spin_lock_irqsave(&rm->m_rs_lock, flags); > > - notify = rm->rdma.op_notify | rm->data.op_notify; > ro = &rm->rdma; > if (test_bit(RDS_MSG_ON_SOCK, &rm->m_flags) && > - ro->op_active && notify && ro->op_notifier) { > + ro->op_active && ro->op_notify && ro->op_notifier) { > notifier = ro->op_notifier; > rs = rm->m_rs; > sock_hold(rds_rs_to_sk(rs));
diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c index 18f2341..dfe6237 100644 --- a/net/rds/ib_send.c +++ b/net/rds/ib_send.c @@ -69,6 +69,16 @@ static void rds_ib_send_complete(struct rds_message *rm, complete(rm, notify_status); } +static void rds_ib_send_unmap_data(struct rds_ib_connection *ic, + struct rm_data_op *op, + int wc_status) +{ + if (op->op_nents) + ib_dma_unmap_sg(ic->i_cm_id->device, + op->op_sg, op->op_nents, + DMA_TO_DEVICE); +} + static void rds_ib_send_unmap_rdma(struct rds_ib_connection *ic, struct rm_rdma_op *op, int wc_status) @@ -129,21 +139,6 @@ static void rds_ib_send_unmap_atomic(struct rds_ib_connection *ic, rds_ib_stats_inc(s_ib_atomic_fadd); } -static void rds_ib_send_unmap_data(struct rds_ib_connection *ic, - struct rm_data_op *op, - int wc_status) -{ - struct rds_message *rm = container_of(op, struct rds_message, data); - - if (op->op_nents) - ib_dma_unmap_sg(ic->i_cm_id->device, - op->op_sg, op->op_nents, - DMA_TO_DEVICE); - - if (rm->rdma.op_active && rm->data.op_notify) - rds_ib_send_unmap_rdma(ic, &rm->rdma, wc_status); -} - /* * Unmap the resources associated with a struct send_work. * @@ -902,7 +897,9 @@ int rds_ib_xmit_rdma(struct rds_connection *conn, struct rm_rdma_op *op) send->s_queued = jiffies; send->s_op = NULL; - nr_sig += rds_ib_set_wr_signal_state(ic, send, op->op_notify); + if (!op->op_notify) + nr_sig += rds_ib_set_wr_signal_state(ic, send, + op->op_notify); send->s_wr.opcode = op->op_write ? IB_WR_RDMA_WRITE : IB_WR_RDMA_READ; send->s_rdma_wr.remote_addr = remote_addr; diff --git a/net/rds/rdma.c b/net/rds/rdma.c index b340ed4..916f5ec 100644 --- a/net/rds/rdma.c +++ b/net/rds/rdma.c @@ -641,16 +641,6 @@ int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm, } op->op_notifier->n_user_token = args->user_token; op->op_notifier->n_status = RDS_RDMA_SUCCESS; - - /* Enable rmda notification on data operation for composite - * rds messages and make sure notification is enabled only - * for the data operation which follows it so that application - * gets notified only after full message gets delivered. - */ - if (rm->data.op_sg) { - rm->rdma.op_notify = 0; - rm->data.op_notify = !!(args->flags & RDS_RDMA_NOTIFY_ME); - } } /* The cookie contains the R_Key of the remote memory region, and diff --git a/net/rds/rds.h b/net/rds/rds.h index 0d8f67c..f0066d1 100644 --- a/net/rds/rds.h +++ b/net/rds/rds.h @@ -476,7 +476,6 @@ struct rds_message { } rdma; struct rm_data_op { unsigned int op_active:1; - unsigned int op_notify:1; unsigned int op_nents; unsigned int op_count; unsigned int op_dmasg; diff --git a/net/rds/send.c b/net/rds/send.c index 166dd57..031b1e9 100644 --- a/net/rds/send.c +++ b/net/rds/send.c @@ -491,14 +491,12 @@ void rds_rdma_send_complete(struct rds_message *rm, int status) struct rm_rdma_op *ro; struct rds_notifier *notifier; unsigned long flags; - unsigned int notify = 0; spin_lock_irqsave(&rm->m_rs_lock, flags); - notify = rm->rdma.op_notify | rm->data.op_notify; ro = &rm->rdma; if (test_bit(RDS_MSG_ON_SOCK, &rm->m_flags) && - ro->op_active && notify && ro->op_notifier) { + ro->op_active && ro->op_notify && ro->op_notifier) { notifier = ro->op_notifier; rs = rm->m_rs; sock_hold(rds_rs_to_sk(rs));