> -----Original Message----- > From: Doug Ledford [mailto:dledford@xxxxxxxxxx] > Sent: Monday, February 04, 2019 4:56 PM > To: Dalessandro, Dennis <dennis.dalessandro@xxxxxxxxx>; jgg@xxxxxxxx > Cc: linux-rdma@xxxxxxxxxxxxxxx; Marciniszyn, Mike > <mike.marciniszyn@xxxxxxxxx>; Wan, Kaike <kaike.wan@xxxxxxxxx> > 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@xxxxxxxxxx> > GPG KeyID: B826A3330E572FDD > Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
