> -----Original Message----- > From: Tom Talpey > Sent: Monday, August 14, 2017 1:44 PM > To: Long Li <longli@xxxxxxxxxxxxx>; Steve French <sfrench@xxxxxxxxx>; > linux-cifs@xxxxxxxxxxxxxxx; samba-technical@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx > Subject: RE: [[PATCH v1] 18/37] [CIFS] SMBD: Implement API for upper layer > to send data > > > -----Original Message----- > > From: linux-cifs-owner@xxxxxxxxxxxxxxx [mailto:linux-cifs- > > owner@xxxxxxxxxxxxxxx] On Behalf Of Long Li > > Sent: Wednesday, August 2, 2017 4:10 PM > > To: Steve French <sfrench@xxxxxxxxx>; linux-cifs@xxxxxxxxxxxxxxx; > > samba- technical@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > > Cc: Long Li <longli@xxxxxxxxxxxxx> > > Subject: [[PATCH v1] 18/37] [CIFS] SMBD: Implement API for upper layer > > to send data > > > > +/* > > + * Write data to transport > > + * Each rqst is transported as a SMBDirect payload > > + * rqst: the data to write > > + * return value: 0 if successfully write, otherwise error code */ > > +int cifs_rdma_write(struct cifs_rdma_info *info, struct smb_rqst > > +*rqst) { > > !!! > This is a VERY confusing name. It is not sending an RDMA Write, which will > confuse any RDMA-enlightened reader. It's performing an RDMA Send, so > that name is perhaps one possibility. > > > + if (info->transport_status != CIFS_RDMA_CONNECTED) { > > + log_cifs_write("disconnected returning -EIO\n"); > > + return -EIO; > > + } > > Isn't this optimizing the error case? There's no guarantee it's still connected > by the time the following request construction occurs. Why not just proceed > without the check? > > > + /* Strip the first 4 bytes MS-SMB2 section 2.1 > > + * they are used only for TCP transport */ > > + iov[0].iov_base = (char*)rqst->rq_iov[0].iov_base + 4; > > + iov[0].iov_len = rqst->rq_iov[0].iov_len - 4; > > + buflen += iov[0].iov_len; > > Ok, that layering choice in the cifs.ko client code needs to be corrected. After > all, it will need to be RDMA-aware to build the SMB3 read/write channel > structures. > And, the code in cifs_post_send_data() is allocating and building a structure > that could have been accounted for much earlier, avoiding the extra > overhead. > > That change could happen later, the hack is mostly ok for now. But > something needs to be said in a comment. Will address those in v2. > > Tom. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
