On Tue, Oct 30, 2018 at 07:31:18PM +0000, Mark Bloch wrote:
>
>
> On 10/30/2018 6:54 AM, Andrew Boyer wrote:
> > Signed-off-by: Andrew Boyer <andrew.boyer@xxxxxxxx>
> > ---
> > drivers/infiniband/sw/rxe/rxe_comp.c | 6 ++++++
> > drivers/infiniband/sw/rxe/rxe_hw_counters.c | 7 +++++--
> > drivers/infiniband/sw/rxe/rxe_hw_counters.h | 3 +++
> > drivers/infiniband/sw/rxe/rxe_net.c | 1 +
> > drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
> > 5 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
> > index 6cdc40ed8a9f..6dc99ca31b5f 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_comp.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_comp.c
> > @@ -428,6 +428,7 @@ static void make_send_cqe(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
> > */
> > static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
> > {
> > + struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
> > struct rxe_cqe cqe;
> >
> > if ((qp->sq_sig_type == IB_SIGNAL_ALL_WR) ||
> > @@ -440,6 +441,11 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
> > advance_consumer(qp->sq.queue);
> > }
> >
> > + if (wqe->wr.opcode == IB_WR_SEND ||
> > + wqe->wr.opcode == IB_WR_SEND_WITH_IMM ||
> > + wqe->wr.opcode == IB_WR_SEND_WITH_INV)
> > + rxe_counter_inc(rxe, RXE_CNT_RDMA_SEND);
> > +
> > /*
> > * we completed something so let req run again
> > * if it is trying to fence
> > diff --git a/drivers/infiniband/sw/rxe/rxe_hw_counters.c b/drivers/infiniband/sw/rxe/rxe_hw_counters.c
> > index 6aeb7a165e46..4a24895846d3 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_hw_counters.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_hw_counters.c
> > @@ -37,15 +37,18 @@ static const char * const rxe_counter_name[] = {
> > [RXE_CNT_SENT_PKTS] = "sent_pkts",
> > [RXE_CNT_RCVD_PKTS] = "rcvd_pkts",
> > [RXE_CNT_DUP_REQ] = "duplicate_request",
> > - [RXE_CNT_OUT_OF_SEQ_REQ] = "out_of_sequence",
> > + [RXE_CNT_OUT_OF_SEQ_REQ] = "out_of_seq_request",
> > [RXE_CNT_RCV_RNR] = "rcvd_rnr_err",
> > [RXE_CNT_SND_RNR] = "send_rnr_err",
> > [RXE_CNT_RCV_SEQ_ERR] = "rcvd_seq_err",
> > - [RXE_CNT_COMPLETER_SCHED] = "ack_deffered",
> > + [RXE_CNT_COMPLETER_SCHED] = "ack_deferred",
>
> We really can't change those two, you are changing a sysfs entry with this.
>
Mark,
In this specific case, I wouldn't be so strict, for one main reason,
most probably we won't break any user space applications with such change.
It is long-going debate what is actually included in famous "don't break
user space" narrative, but in this case, all users of RXE are using
upstream code and there are no any known to me tools which monitor
those RXE counters. It means that in this case, it is safe to rename.
In general, the rule is that kernel devs are allowed to make user
visible changes as long as they don't break user space applications
and every cycle you can find endless number of such examples, many of
them in fs/ part of kernel.
Thanks
Attachment:
signature.asc
Description: PGP signature
