| Message ID | 20190124032400.3371.74628.stgit@scvm10.sc.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | IB/hfi1: Add TID RDMA Read | expand |
On Wed, 2019-01-23 at 19:31 -0800, Dennis Dalessandro wrote: > +/* > + * Handle the KDETH eflags for TID RDMA READ response. > + * > + * Return true if the last packet for a segment has been received and it is > + * time to process the response normally; otherwise, return true. > + */ > +static bool handle_read_kdeth_eflags(struct hfi1_ctxtdata *rcd, > + struct hfi1_packet *packet, u8 rcv_type, > + u8 rte, u32 psn, u32 ibpsn) > +{ This function is called by the code below with both an rcu_read_lock and a spin_lock_irqsave on qp->r_lock. In most places in this patchset, you've been good about calling out required locking. In this case, it seems you know this function must be called with at least the qp->r_lock held because instead of spin_lock_irqsave, you use plain spin_lock when taking the qp->s_lock in this function. You should make this locking requirement explicit like you did elsewhere.
> -----Original Message----- > From: Doug Ledford [mailto:dledford@redhat.com] > Sent: Monday, February 04, 2019 4:56 PM > To: Dalessandro, Dennis <dennis.dalessandro@intel.com>; jgg@ziepe.ca > Cc: linux-rdma@vger.kernel.org; Marciniszyn, Mike > <mike.marciniszyn@intel.com>; Wan, Kaike <kaike.wan@intel.com> > Subject: Re: [PATCH for-next 10/17] IB/hfi1: Add functions to receive TID > RDMA READ response > > On Wed, 2019-01-23 at 19:31 -0800, Dennis Dalessandro wrote: > > +/* > > + * Handle the KDETH eflags for TID RDMA READ response. > > + * > > + * Return true if the last packet for a segment has been received and > > +it is > > + * time to process the response normally; otherwise, return true. > > + */ > > +static bool handle_read_kdeth_eflags(struct hfi1_ctxtdata *rcd, > > + struct hfi1_packet *packet, u8 rcv_type, > > + u8 rte, u32 psn, u32 ibpsn) { > > This function is called by the code below with both an rcu_read_lock and a > spin_lock_irqsave on qp->r_lock. In most places in this patchset, you've been > good about calling out required locking. In this case, it seems you know this > function must be called with at least the qp->r_lock held because instead of > spin_lock_irqsave, you use plain spin_lock when taking the qp->s_lock in this > function. You should make this locking requirement explicit like you did > elsewhere. I will add __must_hold() to the function and also add comments. Kaike > > -- > Doug Ledford <dledford@redhat.com> > GPG KeyID: B826A3330E572FDD > Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
On Wed, 2019-01-23 at 19:29 -0800, Dennis Dalessandro wrote: > This is the series for adding TID RDMA read. Kaike put in a lot of effort into > making this more consumable for review so special thanks to him. > > Allocating resources and tracing are separated out followed by patches which > build up the read request. Then we have the patches to receive incoming TID RDMA > read requests and handle integration with the RC protocol. > > See the cover letter of the original posting for more of a detailed overview > of TID. > > https://www.spinics.net/lists/linux-rdma/msg66611.html > > --- > > Kaike Wan (17): > IB/hfi: Move RC functions into a header file > IB/hfi1: TID RDMA flow allocation > IB/hfi1: TID RDMA RcvArray programming and TID allocation > IB/hfi1: Add the counter n_tidwait > IB/hfi1: Add static trace for flow and TID management functions > IB/hfi1: Add functions to build TID RDMA READ request > IB/hfi1: Set PbcInsertHcrc for TID RDMA packets > IB/hfi1: Add functions to receive TID RDMA READ request > IB/hfi1: Add a function to build TID RDMA READ response > IB/hfi1: Add functions to receive TID RDMA READ response > IB/hfi1: Add TID RDMA handlers > IB/hfi1: Add functions for restarting TID RDMA READ request > IB/hfi1: Increment the retry timeout value for TID RDMA READ request > IB/hfi1: Integrate TID RDMA READ protocol into RC protocol > IB/hfi1: Add interlock between a TID RDMA request and other requests > IB/hfi1: Enable TID RDMA READ protocol > IB/hfi1: Add static trace for TID RDMA READ protocol Thanks, series with two V2 patches applied.
This is the series for adding TID RDMA read. Kaike put in a lot of effort into making this more consumable for review so special thanks to him. Allocating resources and tracing are separated out followed by patches which build up the read request. Then we have the patches to receive incoming TID RDMA read requests and handle integration with the RC protocol. See the cover letter of the original posting for more of a detailed overview of TID. https://www.spinics.net/lists/linux-rdma/msg66611.html --- Kaike Wan (17): IB/hfi: Move RC functions into a header file IB/hfi1: TID RDMA flow allocation IB/hfi1: TID RDMA RcvArray programming and TID allocation IB/hfi1: Add the counter n_tidwait IB/hfi1: Add static trace for flow and TID management functions IB/hfi1: Add functions to build TID RDMA READ request IB/hfi1: Set PbcInsertHcrc for TID RDMA packets IB/hfi1: Add functions to receive TID RDMA READ request IB/hfi1: Add a function to build TID RDMA READ response IB/hfi1: Add functions to receive TID RDMA READ response IB/hfi1: Add TID RDMA handlers IB/hfi1: Add functions for restarting TID RDMA READ request IB/hfi1: Increment the retry timeout value for TID RDMA READ request IB/hfi1: Integrate TID RDMA READ protocol into RC protocol IB/hfi1: Add interlock between a TID RDMA request and other requests IB/hfi1: Enable TID RDMA READ protocol IB/hfi1: Add static trace for TID RDMA READ protocol drivers/infiniband/hw/hfi1/chip.c | 2 drivers/infiniband/hw/hfi1/chip.h | 1 drivers/infiniband/hw/hfi1/common.h | 4 drivers/infiniband/hw/hfi1/driver.c | 58 - drivers/infiniband/hw/hfi1/hfi.h | 19 drivers/infiniband/hw/hfi1/init.c | 10 drivers/infiniband/hw/hfi1/qp.c | 5 drivers/infiniband/hw/hfi1/qp.h | 2 drivers/infiniband/hw/hfi1/rc.c | 542 +++++- drivers/infiniband/hw/hfi1/rc.h | 50 + drivers/infiniband/hw/hfi1/tid_rdma.c | 2714 +++++++++++++++++++++++++++++ drivers/infiniband/hw/hfi1/tid_rdma.h | 196 ++ drivers/infiniband/hw/hfi1/trace.c | 52 + drivers/infiniband/hw/hfi1/trace_ibhdrs.h | 2 drivers/infiniband/hw/hfi1/trace_rc.h | 48 + drivers/infiniband/hw/hfi1/trace_tid.h | 758 ++++++++ drivers/infiniband/hw/hfi1/trace_tx.h | 12 drivers/infiniband/hw/hfi1/user_exp_rcv.h | 1 drivers/infiniband/hw/hfi1/verbs.c | 176 ++ drivers/infiniband/hw/hfi1/verbs.h | 61 + drivers/infiniband/hw/qib/qib_rc.c | 7 drivers/infiniband/sw/rdmavt/qp.c | 13 drivers/infiniband/sw/rdmavt/rc.c | 13 include/rdma/ib_hdrs.h | 9 include/rdma/rdma_vt.h | 12 include/rdma/rdmavt_qp.h | 18 include/rdma/tid_rdma_defs.h | 52 + 27 files changed, 4664 insertions(+), 173 deletions(-) create mode 100644 drivers/infiniband/hw/hfi1/rc.h create mode 100644 include/rdma/tid_rdma_defs.h -- -Denny