| Message ID | 20180417191710.14855-1-longli@linuxonhyperv.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/6] cifs: smbd: Check for iov length on sending the last iov | expand |
> -----Original Message----- > From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf > Of Long Li > Sent: Tuesday, April 17, 2018 12:17 PM > To: Steve French <sfrench@samba.org>; linux-cifs@vger.kernel.org; samba- > technical@lists.samba.org; linux-kernel@vger.kernel.org; linux-rdma@vger.kernel.org > Cc: Long Li <longli@microsoft.com>; stable@vger.kernel.org > Subject: [Patch v2 1/6] cifs: smbd: Check for iov length on sending the last iov > > From: Long Li <longli@microsoft.com> > > When sending the last iov that breaks into smaller buffers to fit the > transfer size, it's necessary to check if this is the last iov. > > If this is the latest iov, stop and proceed to send pages. > > Signed-off-by: Long Li <longli@microsoft.com> > Cc: stable@vger.kernel.org Reviewed-by: Michael Kelley <mikelley@microsoft.com> But a question unrelated to this change arose during my review: At the beginning and end of smbd_send(), the field smbd_send_pending is incremented and decremented, respectively. The increment/decrement are not done as atomic operations. Is this code guaranteed to be single threaded? If not, the count could become corrupted, and smbd_destroy_rdma_work(), which waits for the count to become zero, could hang. A similar question applies to smbd_recv_pending in smbd_recv(). > --- > fs/cifs/smbdirect.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c > index 90e673c..b5c6c0d 100644 > --- a/fs/cifs/smbdirect.c > +++ b/fs/cifs/smbdirect.c > @@ -2197,6 +2197,8 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) > goto done; > } > i++; > + if (i == rqst->rq_nvec) > + break; > } > start = i; > buflen = 0; > -- > 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Subject: RE: [Patch v2 1/6] cifs: smbd: Check for iov length on sending the last > iov > > > -----Original Message----- > > From: linux-kernel-owner@vger.kernel.org > > <linux-kernel-owner@vger.kernel.org> On Behalf Of Long Li > > Sent: Tuesday, April 17, 2018 12:17 PM > > To: Steve French <sfrench@samba.org>; linux-cifs@vger.kernel.org; > > samba- technical@lists.samba.org; linux-kernel@vger.kernel.org; > > linux-rdma@vger.kernel.org > > Cc: Long Li <longli@microsoft.com>; stable@vger.kernel.org > > Subject: [Patch v2 1/6] cifs: smbd: Check for iov length on sending > > the last iov > > > > From: Long Li <longli@microsoft.com> > > > > When sending the last iov that breaks into smaller buffers to fit the > > transfer size, it's necessary to check if this is the last iov. > > > > If this is the latest iov, stop and proceed to send pages. > > > > Signed-off-by: Long Li <longli@microsoft.com> > > Cc: stable@vger.kernel.org > > Reviewed-by: Michael Kelley <mikelley@microsoft.com> > > But a question unrelated to this change arose during my review: At the > beginning and end of smbd_send(), the field smbd_send_pending is > incremented and decremented, respectively. The increment/decrement > are not done as atomic operations. Is this code guaranteed to be single > threaded? If not, the count could become corrupted, and > smbd_destroy_rdma_work(), which waits for the count to become zero, > could hang. A similar question applies to smbd_recv_pending in smbd_recv(). It is safe. The transport sending path is locked by TCP_Server_Info->srv_mutex. The transport receiving path is single threaded. > > > --- > > fs/cifs/smbdirect.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c index > > 90e673c..b5c6c0d 100644 > > --- a/fs/cifs/smbdirect.c > > +++ b/fs/cifs/smbdirect.c > > @@ -2197,6 +2197,8 @@ int smbd_send(struct smbd_connection *info, > struct smb_rqst *rqst) > > goto done; > > } > > i++; > > + if (i == rqst->rq_nvec) > > + break; > > } > > start = i; > > buflen = 0; > > -- > > 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c index 90e673c..b5c6c0d 100644 --- a/fs/cifs/smbdirect.c +++ b/fs/cifs/smbdirect.c @@ -2197,6 +2197,8 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } i++; + if (i == rqst->rq_nvec) + break; } start = i; buflen = 0;