Re: [PATCH] Do not skip records with nonblocking connections (take 2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
> 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);

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux