| Message ID | 1308405706-3467-1-git-send-email-steved@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Jun 18, 2011, at 10:01 AM, Steve Dickson wrote: > With non-blocking connections, do not skip records when receiving > the streams since entire value messages can be ignored which > in cause the entire stream to become out of sync. > > For example, two mounts simultaneously send two unmaps > commands. The first one is read, then the second thrown > away due to skipping the record. Skipping this record > will cause XDR error later in processing of the stream. I notice that xdrrec_skiprecord() also does a __xdrrec_getrec() on nonblocking connections. In the current code, then, __xdrrec_getrec() is likely invoked twice in svc_vc_recv() on non-blocking connections. After this patch, it is only invoked once. Invoking __xdrrec_getrec() just once in both the blocking and non-blocking cases feels more correct to me, but I don't have a deep understanding of this code. So this patch seems like the right thing to do. > Signed-off-by: Steve Dickson <steved@redhat.com> > --- > src/svc_vc.c | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/src/svc_vc.c b/src/svc_vc.c > index aaaf2d7..87406f1 100644 > --- a/src/svc_vc.c > +++ b/src/svc_vc.c > @@ -610,7 +610,11 @@ svc_vc_recv(xprt, msg) > } > > xdrs->x_op = XDR_DECODE; > - (void)xdrrec_skiprecord(xdrs); > + /* > + * No need skip records with nonblocking connections > + */ > + if (cd->nonblock == FALSE) > + (void)xdrrec_skiprecord(xdrs); cd->nonblock is already checked just before this code block. As a minor optimization, I'd suggest reordering slightly so cd->nonblock is checked only once: xdrs->x_op = XDR_DECODE; if (cd->nonblock) { if (!__xdrrec_getrec(xdrs, &cd->strm_stat, TRUE)) return FALSE; } else (void)xdrrec_skiprecord(xdrs); As long as __xdrrec_getrec() doesn't care about the value of xdrs->x_op, this should be OK to do. > if (xdr_callmsg(xdrs, msg)) { > cd->x_id = msg->rm_xid; > return (TRUE);
diff --git a/src/svc_vc.c b/src/svc_vc.c index aaaf2d7..87406f1 100644 --- a/src/svc_vc.c +++ b/src/svc_vc.c @@ -610,7 +610,11 @@ svc_vc_recv(xprt, msg) } xdrs->x_op = XDR_DECODE; - (void)xdrrec_skiprecord(xdrs); + /* + * No need skip records with nonblocking connections + */ + if (cd->nonblock == FALSE) + (void)xdrrec_skiprecord(xdrs); if (xdr_callmsg(xdrs, msg)) { cd->x_id = msg->rm_xid; return (TRUE);
With non-blocking connections, do not skip records when receiving the streams since entire value messages can be ignored which in cause the entire stream to become out of sync. For example, two mounts simultaneously send two unmaps commands. The first one is read, then the second thrown away due to skipping the record. Skipping this record will cause XDR error later in processing of the stream. Signed-off-by: Steve Dickson <steved@redhat.com> --- src/svc_vc.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)